LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..)
@ 2008-02-10 21:04 Andrew Paprocki
  2008-02-10 23:05 ` Alan Cox
  2008-02-11  8:40 ` Jean Delvare
  0 siblings, 2 replies; 4+ messages in thread
From: Andrew Paprocki @ 2008-02-10 21:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jorge Boncompte, Jean Delvare, Chris Gauthron

I'm interested in expanding the current support for the it87 Super I/O
chip to provide access to more of its capabilities. I started by
looking at two existing drivers which talk to different parts of the
chip. Maybe I'm missing something, but what guarantees that both of
these drivers won't attempt to talk to the chip at the same time?

The watchdog/it8712f_wdt.c driver has an internal spinlock, the
hwmon/it87.c driver doesn't, and I don't see how a lock could be
shared across both unless it is taken care of at a lower level that
I'm not aware of. You can see that code is essentially copied between
the two files to talk to the chip.

This brings me to a more general question regarding SuperI/O chips.
Since these chips touch many different parts of traditionally separate
driver areas, how should the drivers be structured so that they can
all talk to the chip? Should the low level communications routines for
the chip live in a library which all the drivers could use? Should all
of the created devices live inside one file? (e.g. platform/it87.c
instead of hwmon, watchdog, etc)

Thanks,
-Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..)
  2008-02-10 21:04 I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..) Andrew Paprocki
@ 2008-02-10 23:05 ` Alan Cox
  2008-02-10 23:32   ` Andrew Paprocki
  2008-02-11  8:40 ` Jean Delvare
  1 sibling, 1 reply; 4+ messages in thread
From: Alan Cox @ 2008-02-10 23:05 UTC (permalink / raw)
  To: Andrew Paprocki
  Cc: linux-kernel, Jorge Boncompte, Jean Delvare, Chris Gauthron

> Since these chips touch many different parts of traditionally separate
> driver areas, how should the drivers be structured so that they can
> all talk to the chip? Should the low level communications routines for
> the chip live in a library which all the drivers could use? 

Probably yes. And that if possible should manage all the locking. Thats
roughly how the majority of drivers do it. Some export the lock from base
code and inline the accessors depending how complex it is.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..)
  2008-02-10 23:05 ` Alan Cox
@ 2008-02-10 23:32   ` Andrew Paprocki
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Paprocki @ 2008-02-10 23:32 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-kernel, Jorge Boncompte, Jean Delvare

Does it make sense to use the drivers/mfd directory for Super I/O
chips then? Is there any problem with having other hwmon, watchdog,
etc depend on drivers/mfd? Should this kind of setup for platform
devices be documented in the driver-model?

-Andrew

On Feb 10, 2008 6:05 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> > Since these chips touch many different parts of traditionally separate
> > driver areas, how should the drivers be structured so that they can
> > all talk to the chip? Should the low level communications routines for
> > the chip live in a library which all the drivers could use?
>
> Probably yes. And that if possible should manage all the locking. Thats
> roughly how the majority of drivers do it. Some export the lock from base
> code and inline the accessors depending how complex it is.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..)
  2008-02-10 21:04 I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..) Andrew Paprocki
  2008-02-10 23:05 ` Alan Cox
@ 2008-02-11  8:40 ` Jean Delvare
  1 sibling, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2008-02-11  8:40 UTC (permalink / raw)
  To: Andrew Paprocki; +Cc: linux-kernel, Jorge Boncompte, Jim Cromie, Hans de Goede

Hi Andrew,

On Sun, 10 Feb 2008 16:04:25 -0500, Andrew Paprocki wrote:
> I'm interested in expanding the current support for the it87 Super I/O
> chip to provide access to more of its capabilities. I started by
> looking at two existing drivers which talk to different parts of the
> chip. Maybe I'm missing something, but what guarantees that both of
> these drivers won't attempt to talk to the chip at the same time?

Nothing guarantees it in general. However, at least most hwmon drivers
avoid accessing the Super-I/O configuration space after driver
initialization, which happens when the module is loaded. As module
loading is serialized, this limits the risk of concurrent access.

This is however a limitation I'd be happy to get rid of. It would be
nice to be able to access some configuration space registers at
run-time (e.g. VID readings).

> The watchdog/it8712f_wdt.c driver has an internal spinlock, the
> hwmon/it87.c driver doesn't, and I don't see how a lock could be
> shared across both unless it is taken care of at a lower level that
> I'm not aware of. You can see that code is essentially copied between
> the two files to talk to the chip.
> 
> This brings me to a more general question regarding SuperI/O chips.
> Since these chips touch many different parts of traditionally separate
> driver areas, how should the drivers be structured so that they can
> all talk to the chip? Should the low level communications routines for
> the chip live in a library which all the drivers could use? Should all
> of the created devices live inside one file? (e.g. platform/it87.c
> instead of hwmon, watchdog, etc)

You probably want to discuss this with Jim Cromie and Hans de Goede
(Cc'd.) They have been discussing a possible implementation in October
2007 on the lm-sensors list:

http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021561.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021562.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021563.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021564.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021565.html
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-October/021566.html

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-02-11  8:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-10 21:04 I/O collisions w/ hwmon/it87.c and watchdog/it8712f_wdt.c? (Super I/O chips in general..) Andrew Paprocki
2008-02-10 23:05 ` Alan Cox
2008-02-10 23:32   ` Andrew Paprocki
2008-02-11  8:40 ` Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).