Jean Delvare wrote:
> <I wrote, snipped>
>
>The real problem here is that this driver should NOT belong to the i2c
>subsystem at all. It drives Super-I/O chips, not I2C chips. The problem
>is that the current kernel code doesn't allow for hardware monitoring
>drivers not to rely on the i2c subsystem. There are plans to change that
>soon, but it promises to take some time.
>
>
more background later ?
>That being said, you will notice that the pc87360 driver only accesses
>the Super-I/O space when you load it. The semaphores protect the
>hardware monitoring I/O ranges, NOT the Super-I/O space, this is
>different. The Super-I/O space is not protected (which is not exactly
>fine, but happens to work as long as the only accesses are from the init
>section of the driver).
>
>
>
ok - thats clearer.
in sc200_gpio, reconfiguring the pins is done by writing to the
device-files, so
is exposed to user-space, and can be done anytime. Presuming I keep the
same
interface (I planned to do so), pc87366_gpio would also. Not that this
changes
anything.
>So you could add and export a semaphore protecting the Super-I/O range,
>but this would make it impossible for anyone to use your GPIO driver
>without loading the hardware monitoring driver. Better would be to have
>a common driver for the Super-I/O access, which both the GPIO and the
>hardware monitoring drivers would rely on.
>
hmm. Ive already worked up a patch which moved your locks into a
sio-lock module,
which EXPORT_SYMBOL'd them, and extern'd them in your module, and mine.
Im not sending it, cuz as you pointed out, your locks arent protecting
what I thought they were.
BTW - why semaphores, and not read-write locks ? (parrotting a question
asked
of me on kernelnewbies, where I posted the patch for comments)
this approach seems a good middle ground - not too complex, not too distant
from what Ive already done. So, let me outline a 1st-look approach,
please shoot it if hideous:
struct semaphore* sio_chip_present ( unsigned command_addr,
unsigned dev_id_addr,
unsigned dev_id_value )
returns 0 if using command-addr & command_addr +1,
the device-id value is not found at the device id address.
otherwize
kallocs a lock for that command-addr
or returns existing one if already created
( locks can be allocated on heap, yes ? )
thereafter, users try each combo of 3 args they might expect
( 2 combos for pc87366: [2e,4e] and e9,
10 combos for pc87360: [2e,4e ] X [ e1, e8, e4, e5, e9 ] )
If they get a non-null addr returned, they should lock each access with it.
ie, they should lock, use, unlock.
pls substitute $lock for semaphore as appropriate.
Well - I went and tried that - it seemed worth a strawman at least.
(which is what it is, attached). In particular, it doesnt attempt to:
1. test for previously fetched locks. In perl Id use a hash for this,
are there
hash functions in the kernel I should use ? A hash is arguably overkill,
its realistic to think that there are < 10 sio devices in any system, which
is easy to test linearly. Still, I dont like arbitrary limits like that.
2. have any drop_lock to match the get-lock done by sio_chip_present.
I need new/better names for these..
Im starting to work on a hopefully-working version, but am sending this
in case now to see if its soo blooody ugly that it needs to die now.
>Yet another approach would be
>to have a generic Super-I/O subsystem which would take care of the
>problem for all Super-I/O chips.
>
I gather from the '66 pdf that sio cmd&data are either at 2e&2f or 4e&4F.
the sc-1100 has its own sio, at either 2e&2f or 15c&15d. My board,
which has both chips, uses 2e for the '66, so (I presume) must use 15c
for the sc-1100.
>Evgeniy Polyakov is working on this
>(although in a much more complex way I would have suggested). I don't
>know the current status though.
>
>
>
the kernel-connector stuff right ? It made it into 12-rcX-mmY,
but I only know what I read on LWN.
Have you outlined your simpler counter-proposal ?
I didnt find it at gmane.org, searching for
'superio' by 'delvare'
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.i2c.siolockmgr
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20050622/a5c7efb8/attachment.pl
