Hello Marek,
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.
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.
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
