Thanks for looking. Agreed. This changes leaves /proc/acpi/ibm/fan entirely untouched (and actually behave as expected). That's by design. > 1. Use the "hwmon" sysfs interface to expose each fan separately, and > control them separately. > > 1a. it is quite acceptable to control them in group by default, and have > a module parameter to select grouped, or separate behavior. Agree as well. I would add, though, that that would need a more involved refactor, since there would be two different ways to control the fans now. > Also, "fail-safe" for this is to have the two fans on automatic, and to > enable both fans. I thought I hadn't touched that part. Lemme double check. > [...] later improve on the patch to > expose the second fan separately (provided safe group behavior is > maintained, see above). Yep. -- Lars On Thursday, April 30, 2020, 2:40:57 PM PDT, Henrique de Moraes Holschuh <hmh@xxxxxxxxxx> wrote: On Wed, 29 Apr 2020, Stefan Assmann wrote: > On 29.04.20 02:20, larsh@xxxxxxxxxx wrote: > > Do you have a use case for that behavior? > > > > The previous patch broke the /proc interface, didn't not work with the current version of thinkfan > > (but a a version with multi-fan support is in the works), and it had hard to track internal mutable state. > > > > The proposed change is clean on all these fronts. > > > > I'm not a fan of surprising the user with unnecessarily complex behavior (but perhaps this can be added as an option in the future.) > > I concur to keep the patch as is. Any additional functionality could be > added later on, if deemed necessary. Yes, but let's avoid changing exposed APIs as much as possible... Anyway, the correct interface *when exposing both fans* is: 1. Use the "hwmon" sysfs interface to expose each fan separately, and control them separately. 1a. it is quite acceptable to control them in group by default, and have a module parameter to select grouped, or separate behavior. 2. /proc/acpi/ibm/fan shall control both of them at the same time, and report data from the first fan. THIS INTERFACE IS FROZEN, LET'S NOT MESS WITH IT. Also, "fail-safe" for this is to have the two fans on automatic, and to enable both fans. All of that said, *it is very much acceptable* to merge a first set of patches that controls both fans simultaneously and exposes the fan group as if it were just the main fan, and later improve on the patch to expose the second fan separately (provided safe group behavior is maintained, see above). -- Henrique Holschuh
