[Skiboot] [PATCH] HDAT: Add IPMI sensor data under /bmc node

[Joel Stanley]

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?

>
> 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?)

> +       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?

What can the user do with this information?

> +                       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))

> +                       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?

> +
> +/* 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.

> +
> +struct ipmi_sensors {
> +       __be32  count;
> +       struct ipmi_sensors_data data[];
> +};
> +
> +/* Idata index 1 : LED - sensors ID mapping data */
> +#define IPMI_SENSORS_IDATA_LED         1
> +
>  static inline const char *cpu_state(u32 flags)
>  {
>         switch ((flags & CPU_ID_VERIFY_MASK) >> CPU_ID_VERIFY_SHIFT) {
> --
> 2.9.3
>