[lm-sensors] [PATCH v5 1/2] Use "request_muxed_region" in it87 watchdog drivers

Mike Waychison mikew at google.com
Thu Apr 14 21:00:49 CEST 2011


On Wed, Apr 13, 2011 at 4:29 AM, Alan Cox <alan at lxorguk.ukuu.org.uk> wrote:
> On Wed, 13 Apr 2011 09:03:00 +0200
> Wim Van Sebroeck <wim at iguana.be> wrote:
>
>> Hi Nat,
>>
>> > +   (void) request_muxed_region(REG, 2, NAME);
>>
>> Why do we need to typecast this? Can't we just use
>> +     request_muxed_region(REG, 2, NAME);
>
> We really ought to use
>
>        if ()
>
> in theory the request can fail if someone has hogged the resource and not
> muxed it. I'm not clear what the right thing to do in that case is -
> given it should never happen I guess log an error and bail out but that's
> a rather bigger change and perhaps should be a follow up patch ?
>

request_muxed_region() is sorta gross:  it's essentially acting like a
lock, meant to be used for short periods of time, but it can fail if
someone else decides it should.

Would it make more sense to have drivers that need to use the current
request_muxed_region() be able to force a region into a mux-only state
at driver init?  That would lead to much less contorted code to handle
the off-chance that the request_muxed_region() fails.

Ie:

Driver init:

if (!request_muxed_region(base, size, DRV_NAME))
   goto cleanup_driver_init_failed;

Driver cleanup

release_muxed_region(base, size); /* returns void */

And then within drivers:

use_muxed_region(base, size); /* sleeps until region is usable --
returns void */
/* Do stuff */
unuse_muxed_region(base, size); /* returns void */



I realize the above example re-uses the 'request_muxed_region()' name,
but at least this would be much more consistent with how
request_region is used in other drivers.




More information about the lm-sensors mailing list