[Skiboot] [PATCH] HDAT: Add IPMI sensor data under /bmc node
Vasant Hegde
hegdevasant at linux.vnet.ibm.com
Mon Jul 3 19:57:09 AEST 2017
On 06/26/2017 11:27 AM, Joel Stanley wrote:
> On Fri, Jun 23, 2017 at 9:09 PM, Vasant Hegde
> <hegdevasant at linux.vnet.ibm.com> wrote:
>> Add IPMI sensor data under /bmc node.
>
> This doesn't add any tests.
>
> How did you test it?
>
> Which machines support this feature?
Joel,
In P8 BMC system, hostboot provided these sensors details. In P9 they will be
passing this information via HDAT and we will build the device tree. I think
hostboot side code merged recently.
This feature is supported on all P9 BMC machines.
>
>>
>> CC: Joel Stanley <joel at jms.id.au>
>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> ---
>> hdata/fsp.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>> hdata/spira.c | 1 +
>> hdata/spira.h | 24 +++++++++++++++++++++++-
>> 3 files changed, 74 insertions(+), 2 deletions(-)
>>
>> diff --git a/hdata/fsp.c b/hdata/fsp.c
>> index 6953d97..b941ee2 100644
>> --- a/hdata/fsp.c
>> +++ b/hdata/fsp.c
>> @@ -280,6 +280,53 @@ static void add_uart(const struct spss_iopath *iopath, struct dt_node *lpc)
>> be32_to_cpu(iopath->lpc.uart_baud));
>> }
>>
>> +static void add_ipmi_sensors(struct dt_node *bmc_node)
>> +{
>> + int i;
>> + const void *hdif_sensor;
>> + struct dt_node *sensors_node, *sensor_node;
>> + const struct ipmi_sensors *ipmi_sensors;
>> +
>> + hdif_sensor = get_hdif(&spira.ntuples.ipmi_sensor, IPMI_SENSORS_HDIF_SIG);
>
> What is hdif?
>
> (Oh, I just noticed that this is FSP stuff. I guess that's why it
> makes no sense to me?)
Yes :-) HDIF is part of HDAT specification, not FSP specific anymore.
>
>> + if (!hdif_sensor) {
>> + prlog(PR_DEBUG, "SENSORS: Invalid data\n");
>> + return;
>> + }
>> +
>> + ipmi_sensors = HDIF_get_idata(hdif_sensor, IPMI_SENSORS_IDATA_SENSORS, NULL);
>> + if (!ipmi_sensors) {
>> + prlog(PR_DEBUG, "SENSORS: bad data\n");
>> + return;
>> + }
>> +
>> + sensors_node = dt_new(bmc_node, "sensors");
>> + assert(sensors_node);
>> +
>> + dt_add_property_cells(sensors_node, "#address-cells", 1);
>> + dt_add_property_cells(sensors_node, "#size-cells", 0);
>> +
>> + for (i = 0; i < be32_to_cpu(ipmi_sensors->count); i++) {
>> + if(dt_find_by_name_addr(sensors_node, "sensor",
>
> if (condition)
>
>> + ipmi_sensors->data[i].id)) {
>> + prlog(PR_WARNING, "SENSORS: Duplicate sensor ID : %x\n",
>> + ipmi_sensors->data[i].id);
>
> What causes a duplicate sensor ID?
FW !
>
> What can the user do with this information?
No use for end user. But it kind of tells us the we have a bug in FW .
>
>> + continue;
>> + }
>> +
>> + /* We support only < MAX_IPMI_SENSORS sensors */
>> + if (ipmi_sensors->data[i].type > 0xfe)
>
> use MAX_IPMI_SENSORS.
>
> ie.
>
> if (!(ipmi_sensors->data[i].type < MAX_IPMI_SENSORS))
Agreed. I should have used that macro . Will fix in v2.
>
>> + continue;
>> +
>> + sensor_node = dt_new_addr(sensors_node, "sensor",
>> + ipmi_sensors->data[i].id);
>> + assert(sensor_node);
>> + dt_add_property_string(sensor_node, "compatible", "ibm,ipmi-sensor");
>> + dt_add_property_cells(sensor_node, "reg", ipmi_sensors->data[i].id);
>> + dt_add_property_cells(sensor_node, "ipmi-sensor-type",
>> + ipmi_sensors->data[i].type);
>> + }
>> +}
>> +
>> static void bmc_create_node(const struct HDIF_common_hdr *sp)
>> {
>> struct dt_node *bmc_node;
>> @@ -297,7 +344,9 @@ static void bmc_create_node(const struct HDIF_common_hdr *sp)
>> dt_add_property_cells(bmc_node, "#address-cells", 1);
>> dt_add_property_cells(bmc_node, "#size-cells", 0);
>>
>> - /* TODO: add sensor info under /bmc */
>> + /* Add sensor info under /bmc */
>> + add_ipmi_sensors(bmc_node);
>> +
>> sp_impl = HDIF_get_idata(sp, SPSS_IDATA_SP_IMPL, &size);
>> if (CHECK_SPPTR(sp_impl) && (size > 8)) {
>> dt_add_property_strings(bmc_node, "compatible", sp_impl->sp_family);
>> diff --git a/hdata/spira.c b/hdata/spira.c
>> index fd7f351..601c1b3 100644
>> --- a/hdata/spira.c
>> +++ b/hdata/spira.c
>> @@ -1234,6 +1234,7 @@ static void fixup_spira(void)
>> spira.ntuples.pcia = spiras->ntuples.pcia;
>> spira.ntuples.proc_chip = spiras->ntuples.proc_chip;
>> spira.ntuples.hs_data = spiras->ntuples.hs_data;
>> + spira.ntuples.ipmi_sensor = spiras->ntuples.ipmi_sensor;
>> }
>>
>> int parse_hdat(bool is_opal)
>> diff --git a/hdata/spira.h b/hdata/spira.h
>> index bb7ad3e..f141b72 100644
>> --- a/hdata/spira.h
>> +++ b/hdata/spira.h
>> @@ -68,6 +68,7 @@ struct spira_ntuples {
>> struct spira_ntuple pcia; /* 0x2e0 */
>> struct spira_ntuple proc_chip; /* 0x300 */
>> struct spira_ntuple hs_data; /* 0x320 */
>> + struct spira_ntuple ipmi_sensor; /* 0x360 */
>> };
>>
>> struct spira {
>> @@ -81,7 +82,7 @@ struct spira {
>> *
>> * According to FSP engineers, this is an okay thing to do.
>> */
>> - u8 reserved[0xc0];
>> + u8 reserved[0xa0];
>> } __packed __align(0x100);
>>
>> extern struct spira spira;
>> @@ -1063,6 +1064,27 @@ struct sppcrd_chip_tod {
>> /* Idata index 0 : System attribute data */
>> #define HSERV_IDATA_SYS_ATTR 0
>>
>> +/* IPMI sensors mapping data */
>> +#define IPMI_SENSORS_HDIF_SIG "FRUSE "
>
> What does FRUSE mean?
>
> What is it used for?
Again its part of spec, we use FRUSE to identify the section.
>
>> +
>> +/* Idata index 0 : Sensor mapping data */
>> +#define IPMI_SENSORS_IDATA_SENSORS 0
>> +
>> +struct ipmi_sensors_data {
>> + __be32 slca_index;
>> + uint8_t type;
>> + uint8_t id;
>> + __be16 reserved;
>> +};
>
> Perhaps do this so you don't get padding:
>
> struct ipmi_sensors_data {
> __be32 slca_index;
> uint8_t type;
> uint8_t id;
> uint8_t reserved[2];
> };
>
> And add a packed annotation so that we don't get holes.
Agree. I missed to add packed annotation.
Thank!
-Vasant
More information about the Skiboot
mailing list