On 6/23/25 9:53 PM, Uwe Kleine-König wrote:
Hello Marek,
Hello Uwe,
On Mon, Jun 23, 2025 at 07:30:33PM +0200, Marek Vasut wrote:
On 6/23/25 11:11 AM, Uwe Kleine-König wrote:
when I replied to v3 this v4 was already on the list which I missed. My
concern applies here, too, though.
On Sat, Jun 21, 2025 at 07:19:56PM +0200, Marek Vasut wrote:
+static void argon_fan_hat_i2c_shutdown(struct i2c_client *i2c)
+{
+ argon_fan_hat_write(i2c, 100);
+}
If you drop this, I'm willing to apply.
Dropping this would make the hardware which uses this device more
susceptible to thermal damage, e.g. in case it gets stuck during reboot and
does not boot Linux afterward. I don't want to risk such thermal damage.
We agree here. But the right place to address this is the pwm-fan
driver. A PWM is supposed to do exactly and only what its consumer wants
it to do (in the limits set by hardware). Officially a PWM driver
doesn't know the polarity of a fan, so `argon_fan_hat_write(i2c, 100)`
might fully enable or complete disable the fan. The fan-driver knows the
polarity. The PWM driver doesn't even know that it controls a fan. And
the next guy takes the argon device and controls a motor with it --- and
wonders that the vehicle gives full-speed at shutdown.
I suspect this cannot happen without non-standard hardware change of
this device, see the link which shows what this device is, it is an
integrated PWM+fan device:
Argon Fan HAT https://argon40.com/products/argon-fan-hat
So I hope we also agree that the pwm-fan driver (or an even more generic
place if possible that applies to all fan drivers) is the right layer to
fix this. And note that the pwm-fan driver already has such a decision
implemented, it's just the wrong one from your POV as it disables the
fan at shutdown. For me this is another confirmation that having a
shutdown callback in the PWM driver is wrong. The two affected drivers
shouldn't fight about what is the right policy.
I would fully agree with this argument for a generic PWM controller, but
this isn't one, this is a combined PWM+fan device.
The PWM driver is the last one that is being shut down, it is being shut
down after the pwm-fan driver. If the pwm-fan driver fails for whatever
reason, the PWM driver -- in this case driver for a combined PWM+fan
device -- should make sure that the hardware does not melt. So I would
argue that, for this specific device, the shutdown hook here is correct.
I would propose to keep the shutdown hook here, and extend the pwm-fan
driver to be aligned with the behavior of the shutdown hook here. Does
that work for you ?
