[Skiboot] [RFC V2] sensors: occ: Add support for OCC inband sensors
Cédric Le Goater
clg at kaod.org
Wed Mar 22 20:40:06 AEDT 2017
On 03/21/2017 11:27 AM, Shilpasri G Bhat wrote:
> Hi Cedric,
>
> On 03/21/2017 01:20 PM, Cédric Le Goater wrote:
>> Hello Shilpasri,
>>
>> It looks good to me. I just have a few questions below.
>>
>>
>> On 03/18/2017 07:54 PM, Shilpasri G Bhat wrote:
>>> Add support to parse and export OCC inband sensors which are copied
>>> by OCC to main memory in P9. Each OCC writes three buffers which
>>> includes one names buffer for sensor meta data and two buffers for
>>> sensor readings. While OCC writes to one buffer the sensor values
>>> can be read from the other buffer. The sensors are updated every
>>> 100ms.
>>>
>>> This patch adds power and temperature sensors to /ibm,opal/sensors
>>> device-tree node which can be exported by the ibmpowernv-hwmon driver
>>> in Linux.
>>
>> You could expose others sensors in the device tree also even if the
>> Linux driver does not support them yet and then work on the extension.
>> I suppose voltage would be interesting. What else ?
>>
>
> There are sensors of type current like CURVDD, CURVDN which are processor Vdd
> and Vdn current values. These can be exposed as HWMON sensors.
>
> Yes voltage sensors can be exposed.
>
>>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat at linux.vnet.ibm.com>
>>> ---
>>> Changes from V1:
>>> - Populate sensors to device tree instead of parsing in kernel
>>> - Read sensor value in OPAL instead of kernel via opal_sensor_read()
>>>
>>> core/init.c | 1 +
>>> core/sensor.c | 4 +
>>> hw/Makefile.inc | 2 +-
>>> hw/occ-sensor.c | 445 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/sensor.h | 1 +
>>> include/skiboot.h | 4 +
>>> 6 files changed, 456 insertions(+), 1 deletion(-)
>>> create mode 100644 hw/occ-sensor.c
>>>
>>> diff --git a/core/init.c b/core/init.c
>>> index 983ead5..dab76ea 100644
>>> --- a/core/init.c
>>> +++ b/core/init.c
>>> @@ -501,6 +501,7 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
>>> * as possible to avoid delay.
>>> */
>>> occ_pstates_init();
>>> + occ_sensors_init();
>>>
>>> /* Use nvram bootargs over device tree */
>>> cmdline = nvram_query("bootargs");
>>> diff --git a/core/sensor.c b/core/sensor.c
>>> index cc5341c..b0d3c5e 100644
>>> --- a/core/sensor.c
>>> +++ b/core/sensor.c
>>> @@ -29,6 +29,10 @@ static int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
>>> switch (sensor_get_family(sensor_hndl)) {
>>> case SENSOR_DTS:
>>> return dts_sensor_read(sensor_hndl, sensor_data);
>>> + case SENSOR_OCC:
>>> + return occ_sensor_read(sensor_hndl, sensor_data);
>>> + default:
>>> + break;
>>> }
>>>
>>> if (platform.sensor_read)
>>> diff --git a/hw/Makefile.inc b/hw/Makefile.inc
>>> index d87f85e..194097f 100644
>>> --- a/hw/Makefile.inc
>>> +++ b/hw/Makefile.inc
>>> @@ -1,7 +1,7 @@
>>> # -*-Makefile-*-
>>> SUBDIRS += hw
>>> HW_OBJS = xscom.o chiptod.o gx.o cec.o lpc.o lpc-uart.o psi.o
>>> -HW_OBJS += homer.o slw.o occ.o fsi-master.o centaur.o
>>> +HW_OBJS += homer.o slw.o occ.o fsi-master.o centaur.o occ-sensor.o
>>> HW_OBJS += nx.o nx-rng.o nx-crypto.o nx-842.o
>>> HW_OBJS += p7ioc.o p7ioc-inits.o p7ioc-phb.o
>>> HW_OBJS += phb3.o sfc-ctrl.o fake-rtc.o bt.o p8-i2c.o prd.o
>>> diff --git a/hw/occ-sensor.c b/hw/occ-sensor.c
>>> new file mode 100644
>>> index 0000000..04aeb2a
>>> --- /dev/null
>>> +++ b/hw/occ-sensor.c
>>> @@ -0,0 +1,445 @@
>>> +/* Copyright 2017 IBM Corp.
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at
>>> + *
>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>>> + * implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + */
>>> +
>>> +#include <skiboot.h>
>>> +#include <opal.h>
>>> +#include <chip.h>
>>> +#include <sensor.h>
>>> +#include <device.h>
>>> +
>>> +/*
>>> + * OCC Sensor Data
>>> + *
>>> + * OCC sensor data will use BAR2 (OCC Common is per physical drawer).
>>> + * Starting address is at offset 0x00580000 from BAR2 base address.
>>> + * Maximum size is 1.5MB.
>>> + *
>>> + * -------------------------------------------------------------------------
>>> + * | Start (Offset from | End | Size |Description |
>>> + * | BAR2 base address) | | | |
>>> + * -------------------------------------------------------------------------
>>> + * | 0x00580000 | 0x005A57FF |150kB |OCC 0 Sensor Data Block|
>>> + * | 0x005A5800 | 0x005CAFFF |150kB |OCC 1 Sensor Data Block|
>>> + * | : | : | : | : |
>>> + * | 0x00686800 | 0x006ABFFF |150kB |OCC 7 Sensor Data Block|
>>> + * | 0x006AC000 | 0x006FFFFF |336kB |Reserved |
>>> + * -------------------------------------------------------------------------
>>> + *
>>> + *
>>> + * OCC N Sensor Data Block Layout (150kB)
>>> + *
>>> + * The sensor data block layout is the same for each OCC N. It contains
>>> + * sensor-header-block, sensor-names buffer, sensor-readings-ping buffer and
>>> + * sensor-readings-pong buffer.
>>> + *
>>> + * ----------------------------------------------------------------------------
>>> + * | Start (Offset from OCC | End | Size |Description |
>>> + * | N Sensor Data Block) | | | |
>>> + * ----------------------------------------------------------------------------
>>> + * | 0x00000000 | 0x000003FF |1kB |Sensor Data Header Block |
>>> + * | 0x00000400 | 0x0000CBFF |50kB |Sensor Names |
>>> + * | 0x0000CC00 | 0x0000DBFF |4kB |Reserved |
>>> + * | 0x0000DC00 | 0x00017BFF |40kB |Sensor Readings ping buffer|
>>> + * | 0x00017C00 | 0x00018BFF |4kB |Reserved |
>>> + * | 0x00018C00 | 0x00022BFF |40kB |Sensor Readings pong buffer|
>>> + * | 0x00022C00 | 0x000257FF |11kB |Reserved |
>>> + * ----------------------------------------------------------------------------
>>> + *
>>> + * Sensor Data Header Block : This is written once by the OCC during
>>> + * initialization after a load or reset. Layout is defined in 'struct
>>> + * occ_sensor_data_header'
>>> + *
>>> + * Sensor Names : This is written once by the OCC during initialization after a
>>> + * load or reset. It contains static information for each sensor. The number of
>>> + * sensors, format version and length of each sensor is defined in
>>> + * 'Sensor Data Header Block'. Format of each sensor name is defined in
>>> + * 'struct occ_sensor_name'. The first sensor starts at offset 0 followed
>>> + * immediately by the next sensor.
>>> + *
>>> + * Sensor Readings Ping/Pong Buffer:
>>> + * There are two 40kB buffers to store the sensor readings. One buffer that
>>> + * is currently being updated by the OCC and one that is available to be read.
>>> + * Each of these buffers will be of the same format. The number of sensors and
>>> + * the format version of the ping and pong buffers is defined in the
>>> + * 'Sensor Data Header Block'.
>>> + *
>>> + * Each sensor within the ping and pong buffers may be of a different format
>>> + * and length. For each sensor the length and format is determined by its
>>> + * 'struct occ_sensor_name.structure_type' in the Sensor Names buffer.
>>> + *
>>> + * --------------------------------------------------------------------------
>>> + * | Offset | Byte0 | Byte1 | Byte2 | Byte3 | Byte4 | Byte5 | Byte6 | Byte7 |
>>> + * --------------------------------------------------------------------------
>>> + * | 0x0000 |Valid | Reserved |
>>> + * | |(0x01) | |
>>> + * --------------------------------------------------------------------------
>>> + * | 0x0008 | Sensor Readings |
>>> + * --------------------------------------------------------------------------
>>> + * | : | : |
>>> + * --------------------------------------------------------------------------
>>> + * | 0xA000 | End of Data |
>>> + * --------------------------------------------------------------------------
>>> + *
>>> + */
>>> +
>>> +#define MAX_OCCS 8
>>> +#define MAX_CHARS_SENSOR_NAME 16
>>> +#define MAX_CHARS_SENSOR_UNIT 4
>>> +
>>> +#define OCC_SENSOR_DATA_BLOCK_OFFSET 0x00580000
>>> +#define OCC_SENSOR_DATA_BLOCK_SIZE 0x00025800
>>> +
>>> +enum occ_sensor_type {
>>> + OCC_SENSOR_TYPE_GENERIC = 0x0001,
>>> + OCC_SENSOR_TYPE_CURRENT = 0x0002,
>>> + OCC_SENSOR_TYPE_VOLTAGE = 0x0004,
>>> + OCC_SENSOR_TYPE_TEMPERATURE = 0x0008,
>>> + OCC_SENSOR_TYPE_UTILIZATION = 0x0010,
>>> + OCC_SENSOR_TYPE_TIME = 0x0020,
>>> + OCC_SENSOR_TYPE_FREQUENCY = 0x0040,
>>> + OCC_SENSOR_TYPE_POWER = 0x0080,
>>> + OCC_SENSOR_TYPE_PERFORMANCE = 0x0200,
>>> +};
>>> +
>>> +enum occ_sensor_location {
>>> + OCC_SENSOR_LOC_SYSTEM = 0x0001,
>>> + OCC_SENSOR_LOC_PROCESSOR = 0x0002,
>>> + OCC_SENSOR_LOC_PARTITION = 0x0004,
>>> + OCC_SENSOR_LOC_MEMORY = 0x0008,
>>> + OCC_SENSOR_LOC_VRM = 0x0010,
>>> + OCC_SENSOR_LOC_OCC = 0x0020,
>>> + OCC_SENSOR_LOC_CORE = 0x0040,
>>> + OCC_SENSOR_LOC_QUAD = 0x0080,
>>> + OCC_SENSOR_LOC_GPU = 0x0100,
>>> +};
>>> +
>>> +enum sensor_struct_type {
>>> + OCC_SENSOR_READING_FULL = 0x01,
>>> + OCC_SENSOR_READING_COUNTER = 0x02,
>>> +};
>>> +
>>> +/**
>>> + * struct occ_sensor_data_header - Sensor Data Header Block
>>> + * @valid: When the value is 0x01 it indicates
>>> + * that this header block and the sensor
>>> + * names buffer are ready
>>> + * @version: Format version of this block
>>> + * @nr_sensors: Number of sensors in names, ping and
>>> + * pong buffer
>>> + * @sensor_reading_version: Format version of the Ping/Pong buffer
>>> + * @sensor_names_offset: Offset to the location of names buffer
>>> + * @sensor_names_version: Format version of names buffer
>>> + * @sensor_names_length: Length of each sensor in names buffer
>>> + * @sensor_reading_ping_offset: Offset to the location of Ping buffer
>>> + * @sensor_reading_pong_offset: Offset to the location of Pong buffer
>>> + * @pad/reserved: Unused data
>>> + */
>>> +struct occ_sensor_data_header {
>>> + u8 valid;
>>> + u8 version;
>>> + u16 nr_sensors;
>>> + u8 sensor_reading_version;
>>> + u8 pad[3];
>>> + u32 sensor_names_offset;
>>> + u8 sensor_names_version;
>>> + u8 sensor_name_length;
>>> + u16 reserved;
>>> + u32 sensor_reading_ping_offset;
>>> + u32 sensor_reading_pong_offset;
>>> +} __packed;
>>> +
>>> +/**
>>> + * struct occ_sensor_name - Format of Sensor Name
>>> + * @name: Sensor name
>>> + * @units: Sensor units of measurement
>>> + * @gsid: Global sensor id (OCC)
>>> + * @freq: Update frequency
>>> + * @scale_factor: Scaling factor
>>> + * @type: Sensor type as defined in
>>> + * 'enum occ_sensor_type'
>>> + * @location: Sensor location as defined in
>>> + * 'enum occ_sensor_location'
>>> + * @structure_type: Indicates type of data structure used
>>> + * for the sensor readings in the ping and
>>> + * pong buffers for this sensor as defined
>>> + * in 'enum sensor_struct_type'
>>> + * @reading_offset: Offset from the start of the ping/pong
>>> + * reading buffers for this sensor
>>> + * @sensor_data: Sensor specific info
>>> + * @pad: Padding to fit the size of 48 bytes.
>>> + */
>>> +struct occ_sensor_name {
>>> + char name[MAX_CHARS_SENSOR_NAME];
>>> + char units[MAX_CHARS_SENSOR_UNIT];
>>> + u16 gsid;
>>> + u32 freq;
>>> + u32 scale_factor;
>>> + u16 type;
>>> + u16 location;
>>> + u8 structure_type;
>>> + u32 reading_offset;
>>> + u8 sensor_data;
>>> + u8 pad[8];
>>> +} __packed;
>>> +
>>> +/**
>>> + * struct occ_sensor_record - Sensor Reading Full
>>> + * @gsid: Global sensor id (OCC)
>>> + * @timestamp: Time base counter value while updating
>>> + * the sensor
>>> + * @sample: Latest sample of this sensor
>>> + * @sample_min: Minimum value since last OCC reset
>>> + * @sample_max: Maximum value since last OCC reset
>>> + * @CSM_min: Minimum value since last reset request
>>> + * by CSM (CORAL)
>>> + * @CSM_max: Maximum value since last reset request
>>> + * by CSM (CORAL)
>>> + * @profiler_min: Minimum value since last reset request
>>> + * by profiler (CORAL)
>>> + * @profiler_max: Maximum value since last reset request
>>> + * by profiler (CORAL)
>>> + * @job_scheduler_min: Minimum value since last reset request
>>> + * by job scheduler(CORAL)
>>> + * @job_scheduler_max: Maximum value since last reset request
>>> + * by job scheduler (CORAL)
>>> + * @accumulator: Accumulator for this sensor
>>> + * @update_tag: Count of the number of ticks that have
>>> + * passed between updates
>>> + * @pad: Padding to fit the size of 48 bytes
>>> + */
>>> +struct occ_sensor_record {
>>> + u16 gsid;
>>> + u64 timestamp;
>>> + u16 sample;
>>> + u16 sample_min;
>>> + u16 sample_max;
>>> + u16 CSM_min;
>>> + u16 CSM_max;
>>> + u16 profiler_min;
>>> + u16 profiler_max;
>>> + u16 job_scheduler_min;
>>> + u16 job_scheduler_max;
>>> + u64 accumulator;
>>> + u32 update_tag;
>>> + u8 pad[8];
>>> +} __packed;
>>> +
>>> +/**
>>> + * struct occ_sensor_counter - Sensor Reading Counter
>>> + * @gsid: Global sensor id (OCC)
>>> + * @timestamp: Time base counter value while updating
>>> + * the sensor
>>> + * @accumulator: Accumulator/Counter
>>> + * @sample: Latest sample of this sensor (0/1)
>>> + * @pad: Padding to fit the size of 24 bytes
>>> + */
>>> +struct occ_sensor_counter {
>>> + u16 gsid;
>>> + u64 timestamp;
>>> + u64 accumulator;
>>> + u8 sample;
>>> + u8 pad[5];
>>> +} __packed;
>>> +
>>> +struct occ_sensor_info {
>>> + u32 offset;
>>> + u8 occ_num;
>>> +} *sdr;
>>
>> Is this an array mapping OCC sensor addresses with the skiboot sensor
>> handler ?
>
> Yes this array is used to map sensor handlers to sensor address offset which is
> relative to the starting address of sensor reading buffer.
>
>>
>>> +static u64 occ_sensor_base;
>>> +
>>> +static inline
>>> +struct occ_sensor_data_header *get_sensor_header_block(int occ_num)
>>> +{
>>> + return (struct occ_sensor_data_header *)
>>> + (occ_sensor_base + occ_num * OCC_SENSOR_DATA_BLOCK_SIZE);
>>> +}
>>> +
>>> +static inline u32 sensor_handler(int sensor_id)
>>> +{
>>> + return sensor_make_handler(SENSOR_OCC, 0, sensor_id, 0);
>>> +}
>>>
>>
>> So you don't plan to have attributes ? like fault ? I guess you don't
>> need the sensor class but may be you could hijack it to store the OCC
>> number. Just an idea. You don't have to unless this is a way to get rid
>> of sdr[]
>
> Yes. I was planning to post next series with attributes like min, max for each
> sensor.
ok. sounds good.
> Yup I can use class to store OCC number. That way I dont have to store them for
> each sdr[handler].
if we could get rid of the array with some masking/shifting
magic that would be nice.
>>
>>> +int occ_sensor_read(u32 handle, u32 *data)
>>> +{
>>> + struct occ_sensor_data_header *hb;
>>> + struct occ_sensor_record *sping, *spong;
>>> + struct occ_sensor_record *sensor;
>>> + u8 *ping, *pong;
>>> + u16 id = sensor_get_rid(handle);
>>> + u8 occ_num;
>>> +
>>> + occ_num = sdr[id].occ_num;
>>> + hb = get_sensor_header_block(occ_num);
>>> + ping = (u8 *)((u64)hb + hb->sensor_reading_ping_offset);
>>> + pong = (u8 *)((u64)hb + hb->sensor_reading_pong_offset);
>>> + sping = (struct occ_sensor_record *)((u64)ping + sdr[id].offset);
>>> + spong = (struct occ_sensor_record *)((u64)pong + sdr[id].offset);
>>> +
>>> + /* Check which buffer is valid and read the data from that.
>>> + * Ping Pong Action
>>> + * 0 0 Return with error
>>> + * 0 1 Read Pong
>>> + * 1 0 Read Ping
>>> + * 1 1 Read the buffer with latest timestamp
>>> + */
>>> + if (*ping && *pong) {
>>> + if (sping->timestamp > spong->timestamp)
>>> + sensor = sping;
>>> + else
>>> + sensor = spong;
>>> +
>>> + } else if (*ping && !*pong) {
>>> + sensor = sping;
>>> + } else if (!*ping && *pong) {
>>> + sensor = spong;
>>> + } else if (!*ping && !*pong) {
>>> + prlog(PR_DEBUG, "OCC: Both ping and pong sensor buffers are invalid\n");
>>> + return OPAL_BUSY;
>>> + }
>>> +
>>> + *data = sensor->sample;
>>> + return 0;
>>> +}
>>> +
>>> +void occ_sensors_init(void)
>>> +{
>>> + struct proc_chip *chip;
>>> + struct occ_sensor_data_header *hb;
>>> + struct occ_sensor_name *md;
>>> + int occ_to_chipid[MAX_OCCS];
>>
>> MAX_CHIPS may be ? because you loop on chips.
>
> Yes will change.
>
>>
>>> + int nr_sensors = 0, nr_occs = 0;
>>> + int occ_num, i;
>>> + u16 id = 0;
>>> +
>>> + /* OCC inband sensors is only supported in P9 */
>>> + if (proc_gen != proc_gen_p9)
>>> + return;
>>> +
>>> + /* Sensors are copied to BAR2 OCC Common Area */
>>> + chip = next_chip(NULL);
>>> + if (!chip->occ_common_base) {
>>> + prerror("OCC: Unassigned OCC Common Area. No sensors found\n");
>>> + return;
>>> + }
>>> +
>>> + occ_sensor_base = chip->occ_common_base + OCC_SENSOR_DATA_BLOCK_OFFSET;
>>> +
>>> + for_each_chip(chip)
>>> + occ_to_chipid[nr_occs++] = chip->id;
>>
>> I would put the section below in its own occ_sensor_sanity() routine.
>
> Okay will do.
>
>>
>>> + /* Sanity check of the Sensor Data Header Block */
>>> + for (occ_num = 0; occ_num < nr_occs; occ_num++) {
>>> + hb = get_sensor_header_block(occ_num);
>>> + if (hb->valid != 0x01) {
>>> + prerror("OCC: Sensor data invalid\n");
>>> + return;
>>> + }
>>> +
>>> + if (hb->version != 0x01) {
>>> + prerror("OCC: Unsupported sensor header block version %d\n",
>>> + hb->version);
>>> + return;
>>> + }
>>> +
>>> + if (hb->sensor_reading_version != 0x01) {
>>> + prerror("OCC: Unsupported sensor record format %d\n",
>>> + hb->sensor_reading_version);
>>> + return;
>>> + }
>>> +
>>> + if (hb->sensor_names_version != 0x01) {
>>> + prerror("OCC: Unsupported sensor info format %d\n",
>>> + hb->sensor_names_version);
>>> + return;
>>> + }
>>> +
>>> + if (hb->sensor_name_length != sizeof(struct occ_sensor_name)) {
>>> + prerror("OCC: Unsupported sensor info record length %d\n",
>>> + hb->sensor_name_length);
>>> + return;
>>> + }
>>> +
>>> + if (!hb->nr_sensors) {
>>> + prerror("OCC: No sensors found for chip %d\n",
>>> + occ_to_chipid[occ_num]);
>>> + continue;
>>> + }
>>> +
>>> + if (!hb->sensor_names_offset ||
>>> + !hb->sensor_reading_ping_offset ||
>>> + !hb->sensor_reading_pong_offset) {
>>> + prerror("OCC: Invalid sensor buffer pointers\n");
>>> + return;
>>> + }
>>> + }
>>> +
>>> + for (occ_num = 0; occ_num < nr_occs; occ_num++) {
>>> + hb = get_sensor_header_block(occ_num);
>>> +
>>> + md = (struct occ_sensor_name *)((uint64_t)hb +
>>> + hb->sensor_names_offset);
>>
>> I would add an helper for the above cast.
>
> Okay will do.
>
>>
>>> + for (i = 0; i < hb->nr_sensors; i++) {
>>> + if (md[i].type == OCC_SENSOR_TYPE_TEMPERATURE ||
>>> + md[i].type == OCC_SENSOR_TYPE_POWER)
>>> + nr_sensors++;
>>> + }
>>> + }
>>> +
>>> + sdr = malloc(sizeof(*sdr) * nr_sensors);
>>> + assert(sdr);
>>> +
>>> + for (occ_num = 0; occ_num < nr_occs; occ_num++) {
>>> + hb = get_sensor_header_block(occ_num);
>>> +
>>> + md = (struct occ_sensor_name *)((uint64_t)hb +
>>> + hb->sensor_names_offset);
>>> +
>>> + for (i = 0; i < hb->nr_sensors; i++) {
>>> + char name[MAX_CHARS_SENSOR_NAME * 2];
>>
>> why '* 2' ?
>
> I am prefixing chip id in the sensor name for chip sensors.
> char name[MAX_CHARS_SENSOR_NAME + 5] should do instead.
>
>>> + struct dt_node *node;
>>> + u32 handler;
>>> +
>>> + if (md[i].type != OCC_SENSOR_TYPE_TEMPERATURE &&
>>> + md[i].type != OCC_SENSOR_TYPE_POWER)
>>> + continue;
>>> +
>>> + sdr[id].offset = md[i].reading_offset;
>>> + sdr[id].occ_num = occ_num;
>>
>> could md[i].reading_offset fit in a u16 ?
>
> No both are defined u32.
hmm, so we cannot get rid of sdr ?
>>
>>> + handler = sensor_handler(id);
>>> + if (md[i].location == OCC_SENSOR_LOC_SYSTEM)
>>> + snprintf(name, sizeof(name), "%s", md[i].name);
>>> + else
>>> + snprintf(name, sizeof(name), "P%d_%s",
>>> + occ_to_chipid[occ_num], md[i].name);
>>
>> could you give us a sample ouput of these device tree names please ?
>
> P0_TEMPC3 at c0000a {
> label = "P0_TEMPC3";
> compatible = "ibm,opal-sensor";
> sensor-data = <0xc0000a>;
> sensor-type = "temp";
> phandle = <0x10000142>;
> };
>
> PWRSYS at c00000 {
> label = "PWRSYS";
> compatible = "ibm,opal-sensor";
> sensor-data = <0xc00000>;
> sensor-type = "power";
> phandle = <0x10000138>;
> };
>
> PWRGPU at c00004 {
> label = "PWRGPU";
> compatible = "ibm,opal-sensor";
> sensor-data = <0xc00004>;
> sensor-type = "power";
> phandle = <0x1000013c>;
> };
>
> P0_TEMPDIMM6 at c00006 {
> label = "P0_TEMPDIMM6";
> compatible = "ibm,opal-sensor";
> sensor-data = <0xc00006>;
> sensor-type = "temp";
> phandle = <0x1000013e>;
> };
>
> PWRSTORE at c00003 {
> label = "PWRSTORE";
> compatible = "ibm,opal-sensor";
> sensor-data = <0xc00003>;
> sensor-type = "power";
> phandle = <0x1000013b>;
> };
>
the device node should have a nicer name : "dimm-temp", "power-gpu".
I don't think we want to expose the OCC literal there, but we
can keep it in some other property. 'label' will reach user space,
so it should also be well constructed.
Thanks,
C.
>>
>>> + node = dt_new_addr(sensor_node, name, handler);
>>> + dt_add_property_cells(node, "sensor-data", handler);
>>> + dt_add_property_string(node, "compatible",
>>> + "ibm,opal-sensor");
>>> + dt_add_property_string(node, "label", name);
>>
>> This literal name will reach user space. It should be understandable and
>> not necessarily the same as the device tree node name.
>
> Yeah agree. Will add appropriate device-tree node names.
>
>>
>>> + if (md[i].type == OCC_SENSOR_TYPE_POWER)
>>
>> use a switch even if there is only two supported types.
>>
>
> Okay will do.
>
> Thanks and Regards,
> Shilpa
>
>> Thanks,
>>
>> C.
>>
>>> + dt_add_property_string(node, "sensor-type",
>>> + "power");
>>> + else
>>> + dt_add_property_string(node, "sensor-type",
>>> + "temp");
>>> + id++;
>>> + }
>>> + }
>>> +}
>>> diff --git a/include/sensor.h b/include/sensor.h
>>> index 7eb3fa5..445a6bc 100644
>>> --- a/include/sensor.h
>>> +++ b/include/sensor.h
>>> @@ -53,6 +53,7 @@
>>> */
>>> enum {
>>> SENSOR_FSP = 0,
>>> + SENSOR_OCC = 6,
>>> SENSOR_DTS = 7,
>>> };
>>>
>>> diff --git a/include/skiboot.h b/include/skiboot.h
>>> index bb0a7b5..a8e1534 100644
>>> --- a/include/skiboot.h
>>> +++ b/include/skiboot.h
>>> @@ -302,4 +302,8 @@ extern int fake_nvram_info(uint32_t *total_size);
>>> extern int fake_nvram_start_read(void *dst, uint32_t src, uint32_t len);
>>> extern int fake_nvram_write(uint32_t offset, void *src, uint32_t size);
>>>
>>> +/* OCC Inband Sensors */
>>> +extern void occ_sensors_init(void);
>>> +extern int occ_sensor_read(u32 handle, u32 *data);
>>> #endif /* __SKIBOOT_H */
>>>
>>
>
More information about the Skiboot
mailing list