[lm-sensors] [PATCH] Make all it87 drivers SMP safe.
natg at google.com
Wed Apr 6 04:50:47 CEST 2011
Thank you for spotting the fact the everything goes south if you
disable "watchdog". I am working on a solution. Looks like the ideal
place to store it87_io_lock.c will be drivers/misc and the IT87_LOCK
config will be placed before the MISC_DEVICES entry in
drivers/misc/Kconfig file. This will be similar to the
SENSORS_LIS3LV02D entry in that Kconfig file.
Now going back to the partitioning do I send this change out as
a multi patch set consisting of 4 parts something as below:
patch 0 has a description
patch 1 has only the lock and related files
patch 2 has drivers/watchdog changes
patch 3 has drives/hwmon changes
On Tue, Apr 5, 2011 at 5:43 PM, Guenter Roeck
<guenter.roeck at ericsson.com> wrote:
> On Tue, Apr 05, 2011 at 08:13:50PM -0400, Natarajan Gurumoorthy wrote:
> > Guenter,
> > How would you partition it out? Are you suggesting that we do
> > the following:
> > Patch1:
> > drivers/hwmon/Kconfig | 1 +
> > drivers/hwmon/it87.c | 14 ++++++++++++-
> > Patch2:
> > drivers/watchdog/Kconfig | 12 +++++++++++
> > drivers/watchdog/Makefile | 1 +
> > drivers/watchdog/it8712f_wdt.c | 10 ++++----
> > drivers/watchdog/it87_lock.c | 27 +++++++++++++++++++++++++
> > drivers/watchdog/it87_wdt.c | 42 ++++++---------------------------------
> > Patch3:
> > include/linux/it87_lock.h | 28 ++++++++++++++++++++++++++
> No, not really. The include file is part of the locking code, and the sequence is wrong.
> I personally would introduce the lock in the 1st patch.
> This would affect
> The second patch would update the watchdog driver, affecting
> and the last patch would update the hwmon driver.
> Others may argue that patch 1 and 2 (introducing the lock and updating
> the watchdog driver) should be in a single patch, since the lock alone
> does not do anything without being used. This is a matter of opinion
> and really depends on the maintainer of the watchdog subsystem.
> Note that your patch has practical problems. If I disable WATCHDOG but enable
> the IT87 hwmon driver, I get:
> warning: (SENSORS_IT87) selects IT87_LOCK which has unmet direct dependencies (WATCHDOG)
> during configuration, and undefined references to it87_io_lock when linking.
> So it looks like you might want to consider moving the locking code to a location
> outside the watchdog code.
Nat Gurumoorthy AB6SJ
More information about the lm-sensors