Hi, On 4/13/21 11:06 AM, Andy Shevchenko wrote: > > > On Tuesday, April 13, 2021, Hans de Goede <hdegoede@xxxxxxxxxx <mailto:hdegoede@xxxxxxxxxx>> 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? I don't think so, it adds 2 new sysfs files, all existing files keep working as is. The _label attributes are part of the standard hwmon API and any tools implementing that API will either ignore them (if they are ancient) or use them to set a better default label then "temp1" and "temp2", while still honoring any labels from config files with a higher priority then the kernel provided ones. > 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. The answer is no :) So no need to worry about this Regards, Hans > > > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx <mailto:hdegoede@xxxxxxxxxx>> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
