Jean Delvare wrote:
>Hi Jim,
>
>
>
>>SuperIO devices typically hide many functional units behind 2 io-bus
>>addresses. These various units/devices will obviously have separate
>>drivers to control them, leading to the potential that 2 drivers
>>will clash over the 'port'.
>>
>>
>
>This is true. There have been a proposal from Evgeniy Poliakov in year
>2004 for a new superio subsystem which would have addressed the issue.
>Back then, it looked awfully complex to me. It seems to have vanished
>since, so the problem is still there.
>
>
>
>>As I see it, we need a place to put a lock for the sio port, ideally
>>without creating a dependency of one driver on another. That said,
>>it seems a bit like overkill to create a 3rd module which merely
>>holds the lock that both drivers use, and therefore depend upon.
>>IOW, this replaces one dependency for another.
>>
>>
>
>Direct dependencies aren't realistic, as you may have more than two
>drivers interested in one given Super-I/O device. Also, two devices
>from the same family may require a different set of drivers to cover
>all the functions, so you may end up with a complex network of
>dependencies. The only realistic solution is a centralized one, where
>one dedicated piece of code controls the access to the I/O ports, and
>other drivers kindly ask for the permission when they need to access it.
>
>
>
>>OTOH, an sio-lock manager which provides a lock for any sio port user
>>(that uses the helper) could be justified. There are a bunch of SuperIO
>>units in the hwmon/* world, so this seems like the right place to find
>>potential module clients.
>>
>>Any comments ?
>>
>>
>
>So far, we have been limiting the possible problems for hardware
>monitoring drivers by only accessing the Super-I/O ports during module
>initialization. This has some limitations (like VID pins being read
>only once, or some runtime configuration changes being impossible) but
>is sufficient in most cases.
>
>Implementing a centralized superio controller is not trivial. There are
>decisions to be made. Do we want a complete subsystem, or a simple
>helper? Do we want to provide a complete API to the specific LPC
>registers (logical device selection, activation register, I/O base
>registers, interrupt configuration, etc)? Or do we simply want a
>generic I/O ports multiplexer, with a very simple interface? Or both?
>Most Super-I/O chips follow the Intel LPC standard, living at 0x2e or
>0x4e, but I think some live at strange addresses, and/or do not follow
>the regular register mapping. How do we handle these?
>
>The detection part won't be easy either. While answering the question
>"is this Super-I/O device present at address X" is pretty simple
>(that's what individual drivers do at the moment), pre-detecting
>Super-I/O devices promises to be a difficult task. What ports do we
>want to probe? Given that probing these ports means writing to them, we
>better be careful. Also, each Super-I/O chip may have a different
>"access key", so we'd need to try all known keys. Once we're in, the
>different chips use different registers for identification. Register
>0x20 is the most commonly used, but some chips use other registers in
>the 0x21-0x2f range for a complete identification.
>
>Now, if anyone wants to take his/her chance with an experimental
>implementation, I'll be happy to review it. I'm fine with a simple
>implementation local to hardware monitoring drivers to start with.
>
>Thanks,
>
>
>
>
Curiously, I have one ;-) Its *unready* for serious review, but I can
outline it.
I went for a simple helper approach:
- provides an array of lock-structs (mod-param sets size), which are
handed out upon request.
- does no sio-lock search, expects drivers to request them specifically.
- but it does simplify the request. (implements the boilerplate)
- reserves locks for 1st requester of a (sio-port,device-id) tuple.
- 2nd requester shares that lock.
- ie its a singleton
- doesnt attempt to stop anti-social behavior (I wouldnt know how)
- drivers are expected to do their own lock/unlock-ing
driver chooses efficiency vs long-lock-time (maybe debug-mutexes
will help here)
- lock-struct is a lock, sio-addr, dev_id, and (TBD) cur_cmd, cur_val
- returning a lock-struct gives some type-based protection against
inadvertent misuse.
- sio_lock(), sio_unlock() further distinguish it.
- but user can still reach inside the lock-struct to the lock itself
- currently semaphore based, will be mutex.
Also provides sio_inb() and sio_outb(), which are correlates of s
uperio_inb() and superio_outb(). Currently, these functions attempt to
avoid
unnecessary outb()s to sioaddr (the command address), but thats likely
to be a
rarely used code-path, and I plan to rip it out, and maybe revisit it later.
So, the API:
+struct superio_gate {
+ struct semaphore sema; /* lock is here, for no-offset cast */
+ int sioaddr; /* this port's cmd-address */
+ int cur_cmd; /* of cmd-addr, avoid slow io-access */
+ int cur_val; /* of data-addr, avoid slow io-access */
+};
+
+struct superio_gate* get_sio_gate(int sioaddr, int devid_addr, int
devid_val);
+void sio_lock(struct superio_gate*);
+void sio_unlock(struct superio_gate*);
+int sio_inb(struct superio_gate*, int reg);
+void sio_outb(struct superio_gate*, int reg, int val );
+void sio_exit(struct superio_gate*);
users will do:
struct superio_gate* gate = get_sio_gate( sioaddr, devid_addr,
wanted_id );
and then either
sio_lock(gate);
or
lock_mutex(&gate->sema);
Once users have taken the lock, they can use
superio_inb(gate->sioaddr);
superio_inb(sioaddr); // if they know which was found
sio_inb(gate); //
inb(..)
wrt get_sio_gate(), Im thinking about get_oneof_sio_gates(), which would
accept null-terminated arrays for each arg (maybe not devid_addr, since
thats typically fixed across a family of chips - and the drivers that
can operate them),
and do the looping so clients dont have to.
So, with all that said, you can choose to defer a detailed review, since
Ill be
cleaning it up a bunch. I suspect we could chew on the API itself for a
while 1st.
tia
jimc
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.siolocks-alpha1
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060120/987a91f4/attachment.pl
