Hi Jim,
On Tue, 15 Jul 2008 01:56:00 -0600, Jim Cromie wrote:
> these are on my current todo / partly done
>
> - struct superio_common - stuff thats copied from struct superio-search
> to struct superio when reservation is taken
>
> - add a u8 flags to struct superio
> -- carries bit for tracking enter/exit state -
> should support warnings on situations like enter-on-entered,
> exit-on-exited
> - could carry a toggle bit for a union superio_exit { fnptr;
> exit_seq[X] } or somesuch.
>
> -- following another thread - we'd
> --- reserve region when reservation taken, after devid is validated,
> might help keep other semi-rogue drivers out.
Actually, _before_ devid is validated. You shouldn't make _any_ I/O
access without first requesting the region. You can always release the
region afterwards if you didn't find anything interesting.
>
> = sort out enter / exit
>
> As David noted in this thread, the ehf driver has an odd exit sequence,
>
> The combined combined superio_exit() function I have coded currently is
> really quite hideous,
> and has me thinking towards David's function-pointer idea.
>
> - the other alternative is that David's driver should include its own
> custom exit-fn, which
> he would use instead of the regular superio_exit(). It could use the
> superio_enter routine as-is iirc.
Please read my reply to David's concern, and why having a per-chip exit
sequence doesn't make that much sense. I would use the same exit
function as isadump and sensors-detect have been using for months, as
this appears to work fine.
> my stall points - (fishing for answers;)
>
> - enter / exit should be tracked by superio-locks, in flags bit.
> (recent idea, seems sound)
> - enter / exit should be vaguely discouraged in kernel-doc - due to
> expectations of isadump etc.
Which expectations? User-space should stay away from I/O ports the
kernel has requested. There's little point in trying to sort out
concurrent accesses which simply shouldn't happen in the first place.
>
>
> I guess I gotta stop overcomplicating things, and get busy on the patch-set.
>
>
> > More interesting questions IMHO are:
> > 1) is hwmon the correct subsystem to put the .c file in (somewhat
> > superfical
> > really, but we need a place to put the lock coordinator (and enable
> > / disable
> > its building).
>
> I put it here for proximity to the center-of-mass of the drivers I found.
That's not necessarily a bad initial location. We can have superio live
in drivers/hwmon at first, and once all hwmon drivers use it, move it
to a neutral ground and convert the other drivers in the kernel tree
(mainly watchdog drivers I think.)
> > 2) non hwmon drivers need to be edited for superio register use too,
> > and then
> > modified to use the lock coordinator if they touch the superio
> > registers.
> > This will also be a good exercise to see if our API is generic enough.
> >
> I guess there are drivers I missed ?
Best way I think is to grep for each super-I/O chip name.
> thanks again Hans for pulling me back into this, I'll commit to staying
> with it now.
> Should I infer from your comments that the code at the link incorporates
> your feedback ?
> They seem to have aquired some momentum recently 8-)
>
> Jean, you raised the idea of a sysfs interface.
> Does the October patchset look to fit with what youre thinking ?
I don't think your patch offered a sysfs interface? I'm not really
thinking of anything in particular anyway. We just want a way for
sensors-detect to identify supported Super-I/O chips without actually
accessing the I/O ports if a kernel driver already requested them.
> - the most simple I can think of :
> --expose the kzalloced array of superio-fields as readonly attributes.
> --values go non-zero when superio-port is registered by some user-driver
> --it would be nice to see client-drivers list on each slot, but that
> involves more code and mem
What we really need for sensors-detect is simply the value of registers
0x20 and 0x21, and values of registers 0x30, 0x60 and 0x61 for a
selected logical device. In what form we export these, needs to be
discussed.
> David,
> do you have a preference wrt fn-ptr ? and if so, a single fn with 1 arg
> : 0,1,2 for enter/exit/query ?
> do you have any use for a flag bit ? is there generality in it ?
>
> does anyone think s/superio/sio/g is appropriate ?
No. This was discussed before and I have always objected to the name
"sio". It's too short and could mean about anything (starting with
"synchronous I/O", as opposed to "asynchronous I/O" which is very often
spelled AIO.)
> anything else ?
--
Jean Delvare
