Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one_attribute

[Daniel Lezcano]

Hi Rui,


On 21/02/2023 07:22, Zhang, Rui wrote:
> On Mon, 2023-02-20 at 17:24 +0100, Daniel Lezcano wrote:
> > The driver hooks the thermal framework sysfs to add some driver
> > specific information. A debatable approach as that may belong the
> > device sysfs directory, not the thermal zone directory.
> >
> > As the driver is accessing the thermal internals, we should provide
> > at
> > least an API to the thermal framework to add an attribute to the
> > existing sysfs thermal zone entry.
> >
> > Before doing that and given the age of the driver (2008) may be it is
> > worth to double check if these attributes are really needed. So my
> > first proposal is to remove them if that does not hurt.
> >
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
>
> I don't have any device that uses this driver.
> Let's see what Sujith says.

Thanks for your answer.

I take the opportunity to ask you for the ACPI thermal additional sysfs 
entries.

The ACPI thermal driver adds a link:

/sys/class/thermal/thermal_zone0/device

which points to:

../../../LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00


And in this directory there is:

/sys/devices/LNXSYSTM:00/LNXSYBUS:01/LNXTHERM:00/thermal_zone

pointing to /sys/class/thermal/thermal_zone0


I was wondering if we have to keep it also? It is a cyclic description 
and we can have the several thermal zones having a device link pointing 
to the same location. So I'm not sure this is correct.

I can understand adding a link in the thermal zone pointing to the 
device could make sense, and that could be generalized to all the 
thermal zone creation, but the back pointer link seems strange.

Would it make sense to remove this second link ?

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog