linux-kernel - Re: [RESEND PATCH] thermal: rockchip: enable hwmon

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: <20191213173119.GA4231@roeck-us.net>
Date:   Fri, 13 Dec 2019 09:31:19 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Amit Kucheria <amit.kucheria@...durent.com>
Cc:     schaecsn@....net, jdelvare@...e.com,
        Heiko Stuebner <heiko@...ech.de>,
        Linux PM list <linux-pm@...r.kernel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Eduardo Valentin <edubezval@...il.com>,
        linux-rockchip@...ts.infradead.org,
        Zhang Rui <rui.zhang@...el.com>,
        lakml <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RESEND PATCH] thermal: rockchip: enable hwmon

On Fri, Dec 13, 2019 at 10:09:49AM +0530, Amit Kucheria wrote:
> Fix Guenter's email.
> 
> On Fri, Dec 13, 2019 at 10:08 AM Amit Kucheria
> <amit.kucheria@...durent.com> wrote:
> >
> > Hi Stefan,
> >
> > On Fri, Dec 13, 2019 at 4:59 AM Stefan Schaeckeler <schaecsn@....net> wrote:
> > >
> > > Hello Amit,
> > >
> > > > On Thu, Dec 12, 2019 at 11:47 AM Stefan Schaeckeler <schaecsn@....net> wrote:
> > > > >
> > > > > By default, of-based thermal drivers do not enable hwmon.
> > > > > Explicitly enable hwmon for both, the soc and gpu temperature
> > > > > sensor.
> > > >
> > > > Is there any reason you need to expose this in hwmon?
> > >
> > > Why hwmon:
> > >
> > > The soc embedds temperature sensors and hwmon is the standard way to expose
> > > sensors.
> >
> > Let me rephrase - is there something in the hwmon subsystem that is
> > needed that isn't provided by the thermal subsystem inside
> > /sys/class/thermal?
> >

Doesn't the sentence below answer that question ?

> > > Sensors exposed by hwmon are automagically found by userland clients. Users
> > > want to run sensors(1) and expect them to show up.
> > >
> >
> > That is a good point. In which case, I wonder if we should just fix
> > this in of-thermal.c instead of requiring individual drivers to do
> > write boilerplate code. I'm thinking of a flag that the driver could
> > set to enable the thermal_hwmon interface for of-thermal drivers.

It seems to me that would be outside the scope of this patch.

> >
> > > Why in rockchip_thermal.c:
> > >
> > > drivers/thermal/ provides a high-level hwmon api in thermal_hwmon.[hc] which is
> > > used by at least these thermal drivers: rcar_gen3_thermal.c, rcar_thermal.c,
> > > st/stm_thermal.c, and broadcom/bcm2835_thermal.c. I want to hook up
> > > rockchip_thermal.c exactly the same way.
> > >
> > > Apparently, other architectures hook up the cpu temperature sensors to hwmon
> > > elsewhere. Most seem to do this in hwmon/, e.g. hwmon/coretemp.c. These drivers
> > > are written from scratch. Utilizing thermal_hwmon.[ch] for chips which have
> > > already drivers in drivers/thermal/ seems to be more elegant.
> > >

There should either be a hwmon driver with a bridge to thermal, or a
thermal driver with a bridge to hwmon, but not both. A couple of
existing drivers implement both, but that should really be fixed.

Guenter