[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <780ce6e8-11fc-42be-b4a7-9cffbf811d78@t-8ch.de>
Date: Sat, 22 Mar 2025 16:23:05 +0100
From: Thomas Weißschuh <thomas@...ssschuh.net>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Sung-Chi Li <lschyi@...omium.org>, Jean Delvare <jdelvare@...e.com>,
Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>,
chrome-platform@...ts.linux.dev, linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] hwmon: (cros_ec) Add set and get target fan RPM
function
On 2025-03-22 07:12:48-0700, Guenter Roeck wrote:
> On 3/22/25 06:55, Thomas Weißschuh wrote:
> > On 2025-03-18 15:45:23+0800, Sung-Chi Li wrote:
> > > The ChromeOS embedded controller (EC) supports closed loop fan speed
> > > control, so add the fan target attribute under hwmon framework, such
> > > that kernel can expose reading and specifying the desired fan RPM for
> > > fans connected to the EC.
> > >
> > > When probing the cros_ec hwmon module, we also check the supported
> > > command version of setting target fan RPM. This commit implements the
> > > version 0 of getting the target fan RPM, which can only read the target
> > > RPM of the first fan. This commit also implements the version 1 of
> > > setting the target fan RPM to each fan respectively.
> > >
> > > Signed-off-by: Sung-Chi Li <lschyi@...omium.org>
> > > ---
> > > ChromeOS embedded controller (EC) supports closed-loop fan control. We
> > > anticipate to have the fan related control from the kernel side, so this
> > > series register the HWMON_F_TARGET attribute, and implement the read and
> > > write function for setting/reading the target fan RPM from the EC side.
> >
> > Should it be possible to switch back to automatic control?
> > I can't find anything in the hwmon ABI about it.
> > And neither in the CrOS EC source.
> >
> > Am I missing something?
> >
>
> Not sure I understand the context, but the fan control method is normally
> selected with pwmX_enable, which is defined as
>
> Fan speed control method:
>
> - 0: no fan speed control (i.e. fan at full speed)
> - 1: manual fan speed control enabled (using `pwmY`)
> - 2+: automatic fan speed control enabled
So far I associated pwmY_enable = 1 with the pwmY attribute.
Also controlling it through fanY_target does make sense though.
It could be clearer from the docs IMHO.
That also means that the patch under discussion needs to implement the
pwmY_enable attribute.
One more thing I have wondered about before:
Is pwmY always refering to the same thing as the matching fanY?
Thanks,
Thomas
Powered by blists - more mailing lists