linux-kernel - Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove add_one

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6b63db0832fadffa56ff85a1c4ca2b98d165df50.camel@intel.com>
Date:   Tue, 21 Feb 2023 12:58:59 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "rafael@...nel.org" <rafael@...nel.org>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>
CC:     "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Thomas, Sujith" <sujith.thomas@...el.com>,
        "amitk@...nel.org" <amitk@...nel.org>
Subject: Re: [PATCH RFC] thermal/drivers/intel_menlow: Remove
 add_one_attribute

Hi, Daniel,

On Tue, 2023-02-21 at 12:30 +0100, Daniel Lezcano wrote:
> 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@...aro.org>
> > 
> > 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.

I don't think so. So far, ACPI Thermal object and the generic thermal
zone device are 1:1 match.

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

That was required by some userpsace tool running on menlow, similar to
this one. But TBH, I don't recall the userspace details.

thanks,
rui