Re: [PATCH RESEND] iwlwifi, Do not implement thermal zone unless ucode is loaded

From: Luca Coelho
Date: Wed Jul 13 2016 - 03:25:43 EST


On Wed, 2016-07-13 at 09:50 +0300, Kalle Valo wrote:
> Prarit Bhargava <prarit@xxxxxxxxxx> writes:
>
> > > We implement thermal zone because we do support it, but the
> > > problem is
> > > that we need the firmware to be loaded for that. So you can argue
> > > that
> > > we should register *later* when the firmware is loaded. But this
> > > is
> > > really not helping all that much because the firmware can also be
> > > stopped at any time. So you'd want us to register / unregister
> > > the
> > > thermal zone anytime the firmware is loaded / unloaded?
> >
> > You might have to do that.ÂÂI think that if the firmware enables a
> > feature then
> > the act of loading the firmware should run the code that enables
> > the feature.
> > IMO of course.
>
> But I suspect that the iwlwifi firmware is loaded during interface up
> (and unloaded during interface down) and in that case
> register/unregister would be happening all the time. That doesn't
> sound
> like a good idea. I would rather try to fix the thermal interface to
> handle the cases when the measurement is not available.

I totally agree with Emmanuel and Kalle. ÂWe should not change this.
ÂIt is a design decision to return an error when the interface is down,
this is very common with other subsystems as well. ÂThe userspace
should be able to handle errors and report something like "unavailable"
when this kind of error is returned.

I'm not sure EIO is the best we can have, but for me that's exactly
what it is. ÂThe thermal zone *is* there, but cannot be accessed
because the firmware is not available. ÂI'm okay to change it to EBUSY,
if that would help userspace, but I think that's a bit misleading. ÂThe
device is not busy, on the contrary, it's not even running at all.

Furthermore, I don't think this is "breaking userspace" in the sense of
being a regression. ÂThe userspace API has always been implemented with
the possibility of returning errors. ÂIt's not a good design if a
single device returning an error causes all the other devices to also
fail.

--
Cheers,
Luca.