[lm-sensors] [PATCH v5 1/2] Use "request_muxed_region" in it87 watchdog drivers
natg at google.com
Wed Apr 13 19:59:45 CEST 2011
Unfortunately you are right. After grepping though drivers/hwmon
and drivers/watchdog I realize the ugliness is fairly deep rooted. All
of the following drivers need to be cleaned up as far as the
"superio_enter" "superio_exit" operation is concerned if we want this
stuff to be SMP clean and allow multiple drivers to coexist.
dme1737.c:1935:static inline void dme1737_sio_enter(int sio_cip)
f71882fg.c:196:static inline int superio_enter(int base);
pc87360.c:1009: /* No superio_enter */
sch5627.c:118:static inline int superio_enter(int base)
smsc47b397.c:75:static inline void superio_enter(void)
vt1211.c:223:static inline void superio_enter(int sio_cip)
w83627hf.c:134:superio_enter(struct w83627hf_sio_data *sio)
Have I missed any other drivers that also need to be cleaned. Even if
I got my boss to sign up to clean this I can only test the it8712f
drivers and thats it.
On Wed, Apr 13, 2011 at 1:34 AM, Jean Delvare <khali at linux-fr.org> wrote:
> On Wed, 13 Apr 2011 01:05:33 -0700, Natarajan Gurumoorthy wrote:
>> Comments below
>> On Tue, Apr 12, 2011 at 11:50 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> > You shouldn't (void) this, there is a reason you get a warning
>> > otherwise! request_muxed_region can still fail if some other driver
>> > has done a none muxed request_region on the same region, and in that
>> > case you should not continue with accessing the io-ports.
>> There are 3 it87 drivers and they all have to do the exact same
>> sequence to put the chip into a mode where they can modify its state.
>> The sequences involve non atomic sequences that write locations 0x2e
>> and 0x2f. When they are done they write a different sequence to these
>> 2 locations. The entry routine is superio_enter and exit is
>> superio_exit. All the it87 drivers reserve these 2 locations before
>> they start manipulating the chip. This macro will hold off requestors
>> if the resource is busy because one of the other drivers is
>> manipulating the chip. Once the an it87 driver is done it calls
>> superio_exit which will release the reservation on those 2 locations
>> letting any other driver on the wait queue to now gain access two
>> Please read code in kernel/resource.c function "__request_region".
>> "request_muxed_region" turns on IORESOURCE_MUXED bit and that means
>> that only way an it87 driver will get back from a call to
>> "request_muxed_region" is when it gets hold of the region.
>> The scenario you mention above can never happen.
> Let me be straight clear, as apparently you have difficulties
> understanding Hans's simple request:
> You do not get to (void) the return of request_muxed_region(), period.
> This is _not_ negotiable.
> What other it87 drivers currently in the tree do or don't do is totally
> irrelevant. There can be new it87 drivers added later, there can be
> out-of-tree it87 drivers (including old copies of in-tree ones), and
> there can be non-it87 drivers accessing the I/O ports (or at least
> attempting to.) So the scenario mentioned by Hans can very well happen,
> and you have to deal with it.
> Jean Delvare
Nat Gurumoorthy AB6SJ
More information about the lm-sensors