2020-05-01 15:11:10

by Eddie James

[permalink] [raw]
Subject: [PATCH 0/3] occ: Add support for P10

The OCC in the P10 has a number of differences from the P9. Add some logic to
handle the differences in accessing the OCC from the service processor, and
support the new temperature sensor type.

Eddie James (3):
dt-bindings: fsi: Add P10 OCC device documentation
fsi: occ: Add support for P10
hwmon: (occ) Add new temperature sensor type

.../devicetree/bindings/fsi/ibm,p9-occ.txt | 12 +-
drivers/fsi/fsi-occ.c | 126 +++++++++++++-----
drivers/hwmon/occ/common.c | 75 +++++++++++
3 files changed, 173 insertions(+), 40 deletions(-)

--
2.24.0


2020-05-01 15:11:26

by Eddie James

[permalink] [raw]
Subject: [PATCH 3/3] hwmon: (occ) Add new temperature sensor type

The latest version of the On-Chip Controller (OCC) has a different
format for the temperature sensor data. Add a new temperature sensor
version to handle this data.

Signed-off-by: Eddie James <[email protected]>
---
drivers/hwmon/occ/common.c | 75 ++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index 30e18eb60da7..52af0e728232 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -41,6 +41,14 @@ struct temp_sensor_2 {
u8 value;
} __packed;

+struct temp_sensor_10 {
+ u32 sensor_id;
+ u8 fru_type;
+ u8 value;
+ u8 throttle;
+ u8 reserved;
+} __packed;
+
struct freq_sensor_1 {
u16 sensor_id;
u16 value;
@@ -307,6 +315,60 @@ static ssize_t occ_show_temp_2(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "%u\n", val);
}

+static ssize_t occ_show_temp_10(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int rc;
+ u32 val = 0;
+ struct temp_sensor_10 *temp;
+ struct occ *occ = dev_get_drvdata(dev);
+ struct occ_sensors *sensors = &occ->sensors;
+ struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+
+ rc = occ_update_response(occ);
+ if (rc)
+ return rc;
+
+ temp = ((struct temp_sensor_10 *)sensors->temp.data) + sattr->index;
+
+ switch (sattr->nr) {
+ case 0:
+ val = get_unaligned_be32(&temp->sensor_id);
+ break;
+ case 1:
+ val = temp->value;
+ if (val == OCC_TEMP_SENSOR_FAULT)
+ return -EREMOTEIO;
+
+ /*
+ * VRM doesn't return temperature, only alarm bit. This
+ * attribute maps to tempX_alarm instead of tempX_input for
+ * VRM
+ */
+ if (temp->fru_type != OCC_FRU_TYPE_VRM) {
+ /* sensor not ready */
+ if (val == 0)
+ return -EAGAIN;
+
+ val *= 1000;
+ }
+ break;
+ case 2:
+ val = temp->fru_type;
+ break;
+ case 3:
+ val = temp->value == OCC_TEMP_SENSOR_FAULT;
+ break;
+ case 4:
+ val = temp->throttle * 1000;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return snprintf(buf, PAGE_SIZE - 1, "%u\n", val);
+}
+
static ssize_t occ_show_freq_1(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -745,6 +807,10 @@ static int occ_setup_sensor_attrs(struct occ *occ)
num_attrs += (sensors->temp.num_sensors * 4);
show_temp = occ_show_temp_2;
break;
+ case 0x10:
+ num_attrs += (sensors->temp.num_sensors * 5);
+ show_temp = occ_show_temp_10;
+ break;
default:
sensors->temp.num_sensors = 0;
}
@@ -844,6 +910,15 @@ static int occ_setup_sensor_attrs(struct occ *occ)
attr->sensor = OCC_INIT_ATTR(attr->name, 0444,
show_temp, NULL, 3, i);
attr++;
+
+ if (sensors->temp.version == 0x10) {
+ snprintf(attr->name, sizeof(attr->name),
+ "temp%d_max", s);
+ attr->sensor = OCC_INIT_ATTR(attr->name, 0444,
+ show_temp, NULL,
+ 4, i);
+ attr++;
+ }
}
}

--
2.24.0

2020-05-06 15:58:56

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 3/3] hwmon: (occ) Add new temperature sensor type

On Fri, May 01, 2020 at 10:08:33AM -0500, Eddie James wrote:
> The latest version of the On-Chip Controller (OCC) has a different
> format for the temperature sensor data. Add a new temperature sensor
> version to handle this data.
>
> Signed-off-by: Eddie James <[email protected]>

For my reference:

Acked-by: Guenter Roeck <[email protected]>

I assume this depends on at least patch 2 of the series, so we'll have
to wait for that to be accepted.

Guenter

> ---
> drivers/hwmon/occ/common.c | 75 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index 30e18eb60da7..52af0e728232 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -41,6 +41,14 @@ struct temp_sensor_2 {
> u8 value;
> } __packed;
>
> +struct temp_sensor_10 {
> + u32 sensor_id;
> + u8 fru_type;
> + u8 value;
> + u8 throttle;
> + u8 reserved;
> +} __packed;
> +
> struct freq_sensor_1 {
> u16 sensor_id;
> u16 value;
> @@ -307,6 +315,60 @@ static ssize_t occ_show_temp_2(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "%u\n", val);
> }
>
> +static ssize_t occ_show_temp_10(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int rc;
> + u32 val = 0;
> + struct temp_sensor_10 *temp;
> + struct occ *occ = dev_get_drvdata(dev);
> + struct occ_sensors *sensors = &occ->sensors;
> + struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> +
> + rc = occ_update_response(occ);
> + if (rc)
> + return rc;
> +
> + temp = ((struct temp_sensor_10 *)sensors->temp.data) + sattr->index;
> +
> + switch (sattr->nr) {
> + case 0:
> + val = get_unaligned_be32(&temp->sensor_id);
> + break;
> + case 1:
> + val = temp->value;
> + if (val == OCC_TEMP_SENSOR_FAULT)
> + return -EREMOTEIO;
> +
> + /*
> + * VRM doesn't return temperature, only alarm bit. This
> + * attribute maps to tempX_alarm instead of tempX_input for
> + * VRM
> + */
> + if (temp->fru_type != OCC_FRU_TYPE_VRM) {
> + /* sensor not ready */
> + if (val == 0)
> + return -EAGAIN;
> +
> + val *= 1000;
> + }
> + break;
> + case 2:
> + val = temp->fru_type;
> + break;
> + case 3:
> + val = temp->value == OCC_TEMP_SENSOR_FAULT;
> + break;
> + case 4:
> + val = temp->throttle * 1000;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", val);
> +}
> +
> static ssize_t occ_show_freq_1(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -745,6 +807,10 @@ static int occ_setup_sensor_attrs(struct occ *occ)
> num_attrs += (sensors->temp.num_sensors * 4);
> show_temp = occ_show_temp_2;
> break;
> + case 0x10:
> + num_attrs += (sensors->temp.num_sensors * 5);
> + show_temp = occ_show_temp_10;
> + break;
> default:
> sensors->temp.num_sensors = 0;
> }
> @@ -844,6 +910,15 @@ static int occ_setup_sensor_attrs(struct occ *occ)
> attr->sensor = OCC_INIT_ATTR(attr->name, 0444,
> show_temp, NULL, 3, i);
> attr++;
> +
> + if (sensors->temp.version == 0x10) {
> + snprintf(attr->name, sizeof(attr->name),
> + "temp%d_max", s);
> + attr->sensor = OCC_INIT_ATTR(attr->name, 0444,
> + show_temp, NULL,
> + 4, i);
> + attr++;
> + }
> }
> }
>

2020-05-06 16:04:46

by Eddie James

[permalink] [raw]
Subject: Re: [PATCH 3/3] hwmon: (occ) Add new temperature sensor type


On 5/6/20 10:57 AM, Guenter Roeck wrote:
> On Fri, May 01, 2020 at 10:08:33AM -0500, Eddie James wrote:
>> The latest version of the On-Chip Controller (OCC) has a different
>> format for the temperature sensor data. Add a new temperature sensor
>> version to handle this data.
>>
>> Signed-off-by: Eddie James <[email protected]>
> For my reference:
>
> Acked-by: Guenter Roeck <[email protected]>
>
> I assume this depends on at least patch 2 of the series, so we'll have
> to wait for that to be accepted.


Thanks Guenter. Yes that's correct.

Eddie


>
> Guenter
>
>> ---
>> drivers/hwmon/occ/common.c | 75 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 75 insertions(+)
>>
>> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
>> index 30e18eb60da7..52af0e728232 100644
>> --- a/drivers/hwmon/occ/common.c
>> +++ b/drivers/hwmon/occ/common.c
>> @@ -41,6 +41,14 @@ struct temp_sensor_2 {
>> u8 value;
>> } __packed;
>>
>> +struct temp_sensor_10 {
>> + u32 sensor_id;
>> + u8 fru_type;
>> + u8 value;
>> + u8 throttle;
>> + u8 reserved;
>> +} __packed;
>> +
>> struct freq_sensor_1 {
>> u16 sensor_id;
>> u16 value;
>> @@ -307,6 +315,60 @@ static ssize_t occ_show_temp_2(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "%u\n", val);
>> }
>>
>> +static ssize_t occ_show_temp_10(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + int rc;
>> + u32 val = 0;
>> + struct temp_sensor_10 *temp;
>> + struct occ *occ = dev_get_drvdata(dev);
>> + struct occ_sensors *sensors = &occ->sensors;
>> + struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +
>> + rc = occ_update_response(occ);
>> + if (rc)
>> + return rc;
>> +
>> + temp = ((struct temp_sensor_10 *)sensors->temp.data) + sattr->index;
>> +
>> + switch (sattr->nr) {
>> + case 0:
>> + val = get_unaligned_be32(&temp->sensor_id);
>> + break;
>> + case 1:
>> + val = temp->value;
>> + if (val == OCC_TEMP_SENSOR_FAULT)
>> + return -EREMOTEIO;
>> +
>> + /*
>> + * VRM doesn't return temperature, only alarm bit. This
>> + * attribute maps to tempX_alarm instead of tempX_input for
>> + * VRM
>> + */
>> + if (temp->fru_type != OCC_FRU_TYPE_VRM) {
>> + /* sensor not ready */
>> + if (val == 0)
>> + return -EAGAIN;
>> +
>> + val *= 1000;
>> + }
>> + break;
>> + case 2:
>> + val = temp->fru_type;
>> + break;
>> + case 3:
>> + val = temp->value == OCC_TEMP_SENSOR_FAULT;
>> + break;
>> + case 4:
>> + val = temp->throttle * 1000;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", val);
>> +}
>> +
>> static ssize_t occ_show_freq_1(struct device *dev,
>> struct device_attribute *attr, char *buf)
>> {
>> @@ -745,6 +807,10 @@ static int occ_setup_sensor_attrs(struct occ *occ)
>> num_attrs += (sensors->temp.num_sensors * 4);
>> show_temp = occ_show_temp_2;
>> break;
>> + case 0x10:
>> + num_attrs += (sensors->temp.num_sensors * 5);
>> + show_temp = occ_show_temp_10;
>> + break;
>> default:
>> sensors->temp.num_sensors = 0;
>> }
>> @@ -844,6 +910,15 @@ static int occ_setup_sensor_attrs(struct occ *occ)
>> attr->sensor = OCC_INIT_ATTR(attr->name, 0444,
>> show_temp, NULL, 3, i);
>> attr++;
>> +
>> + if (sensors->temp.version == 0x10) {
>> + snprintf(attr->name, sizeof(attr->name),
>> + "temp%d_max", s);
>> + attr->sensor = OCC_INIT_ATTR(attr->name, 0444,
>> + show_temp, NULL,
>> + 4, i);
>> + attr++;
>> + }
>> }
>> }
>>