On Tuesday, April 13, 2021, Hans de Goede <hdego...@redhat.com> wrote:
> On EC version 3, the first 2 temperature sensors are always CPU and GPU
> add labels for these.
>
> This changes e.g. the "sensors" command output on a X1C8 from:
>
> thinkpad-isa-0000
> Adapter: ISA adapter
> fan1: 2694 RPM
> temp1: +42.0°C
> temp2: N/A
> temp3: +33.0°C
> temp4: +0.0°C
> temp5: +35.0°C
> temp6: +42.0°C
> temp7: +42.0°C
> temp8: N/A
>
> into:
>
> thinkpad-isa-0000
> Adapter: ISA adapter
> fan1: 2694 RPM
> CPU: +42.0°C
> GPU: N/A
> temp3: +33.0°C
> temp4: +0.0°C
> temp5: +35.0°C
> temp6: +42.0°C
> temp7: +42.0°C
> temp8: N/A
>
>
Is it an ABI change? Any updates to the documentation?
If the answer is yes, I would rather do one of the following approach:
1) enable labels by user request (sysfs knob)
2) just add additional two lines
3) add an additional column for labels (or completely another file)
I have understand that there are cons of each of them. I dunno which one is
the best.
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 72 ++++++++++++++++++----------
> 1 file changed, 47 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/
> thinkpad_acpi.c
> index ec98089d98c9..dd60c9397d35 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6296,6 +6296,8 @@ struct ibm_thermal_sensors_struct {
> };
>
> static enum thermal_access_mode thermal_read_mode;
> +static const struct attribute_group *thermal_attr_group;
> +static bool thermal_use_labels;
>
> /* idx is zero-based */
> static int thermal_get_sensor(int idx, s32 *value)
> @@ -6478,6 +6480,28 @@ static const struct attribute_group
> thermal_temp_input8_group = {
> #undef THERMAL_SENSOR_ATTR_TEMP
> #undef THERMAL_ATTRS
>
> +static ssize_t temp1_label_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "CPU\n");
> +}
> +static DEVICE_ATTR_RO(temp1_label);
> +
> +static ssize_t temp2_label_show(struct device *dev, struct
> device_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "GPU\n");
> +}
> +static DEVICE_ATTR_RO(temp2_label);
> +
> +static struct attribute *temp_label_attributes[] = {
> + &dev_attr_temp1_label.attr,
> + &dev_attr_temp2_label.attr,
> + NULL
> +};
> +
> +static const struct attribute_group temp_label_attr_group = {
> + .attrs = temp_label_attributes,
> +};
> +
> /* ---------------------------------------------------------------------
> */
>
> static int __init thermal_init(struct ibm_init_struct *iibm)
> @@ -6533,12 +6557,14 @@ static int __init thermal_init(struct
> ibm_init_struct *iibm)
> thermal_read_mode = TPACPI_THERMAL_NONE;
> }
> } else {
> - if (ver >= 3)
> + if (ver >= 3) {
> thermal_read_mode = TPACPI_THERMAL_TPEC_8;
> - else
> + thermal_use_labels = true;
> + } else {
> thermal_read_mode =
> (ta2 != 0) ?
> TPACPI_THERMAL_TPEC_16 :
> TPACPI_THERMAL_TPEC_8;
> + }
> }
> } else if (acpi_tmp7) {
> if (tpacpi_is_ibm() &&
> @@ -6560,44 +6586,40 @@ static int __init thermal_init(struct
> ibm_init_struct *iibm)
>
> switch (thermal_read_mode) {
> case TPACPI_THERMAL_TPEC_16:
> - res = sysfs_create_group(&tpacpi_hwmon->kobj,
> - &thermal_temp_input16_group);
> - if (res)
> - return res;
> + thermal_attr_group = &thermal_temp_input16_group;
> break;
> case TPACPI_THERMAL_TPEC_8:
> case TPACPI_THERMAL_ACPI_TMP07:
> case TPACPI_THERMAL_ACPI_UPDT:
> - res = sysfs_create_group(&tpacpi_hwmon->kobj,
> - &thermal_temp_input8_group);
> - if (res)
> - return res;
> + thermal_attr_group = &thermal_temp_input8_group;
> break;
> case TPACPI_THERMAL_NONE:
> default:
> return 1;
> }
>
> + res = sysfs_create_group(&tpacpi_hwmon->kobj, thermal_attr_group);
> + if (res)
> + return res;
> +
> + if (thermal_use_labels) {
> + res = sysfs_create_group(&tpacpi_hwmon->kobj,
> &temp_label_attr_group);
> + if (res) {
> + sysfs_remove_group(&tpacpi_hwmon->kobj,
> thermal_attr_group);
> + return res;
> + }
> + }
> +
> return 0;
> }
>
> static void thermal_exit(void)
> {
> - switch (thermal_read_mode) {
> - case TPACPI_THERMAL_TPEC_16:
> - sysfs_remove_group(&tpacpi_hwmon->kobj,
> - &thermal_temp_input16_group);
> - break;
> - case TPACPI_THERMAL_TPEC_8:
> - case TPACPI_THERMAL_ACPI_TMP07:
> - case TPACPI_THERMAL_ACPI_UPDT:
> - sysfs_remove_group(&tpacpi_hwmon->kobj,
> - &thermal_temp_input8_group);
> - break;
> - case TPACPI_THERMAL_NONE:
> - default:
> - break;
> - }
> + if (thermal_attr_group)
> + sysfs_remove_group(&tpacpi_hwmon->kobj,
> thermal_attr_group);
> +
> + if (thermal_use_labels)
> + sysfs_remove_group(&tpacpi_hwmon->kobj,
> &temp_label_attr_group);
> }
>
> static int thermal_read(struct seq_file *m)
> --
> 2.31.1
>
>
--
With Best Regards,
Andy Shevchenko
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
