Guenter,
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
drivers/misc/Kconfig
drivers/misc/Makefile
include/linux/it87_lock.h
drivers/misc/it87_lock.c
patch 2 has drivers/watchdog changes
drivers/watchdog/Kconfig
drivers/watchdog/it8712f_wdt.c
drivers/watchdog/it87_wdt.c
patch 3 has drives/hwmon changes
drivers/hwmon/Kconfig
drivers/hwmon/it87.c
Regards
Nat
On Tue, Apr 5, 2011 at 5:43 PM, Guenter Roeck
<guenter.roeck@xxxxxxxxxxxx> 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
> drivers/watchdog/it87_lock.c
> include/linux/it87_lock.h
> drivers/watchdog/Makefile
> drivers/watchdog/Kconfig
>
> The second patch would update the watchdog driver, affecting
> drivers/watchdog/Kconfig
> drivers/watchdog/it8712f_wdt.c
>
> and the last patch would update the hwmon driver.
> drivers/hwmon/Kconfig
> drivers/hwmon/it87.c
>
> 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.
>
> Thanks,
> Guenter
--
Regards
Nat Gurumoorthy AB6SJ
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
