Re: [patch -mm 20/20 RFC] chardev: GPIO for SCx200 & PC-8736x: addsysfs-GPIO interface

From: Randy.Dunlap
Date: Tue Jun 20 2006 - 23:48:30 EST


On Sat, 17 Jun 2006 12:42:28 -0600 Jim Cromie wrote:

> Ok,
> heres the brand-spanking-new proto-sysfs-gpio interface,
> preceded by some pseudo/proto-Documentation.
>
>
> We need a standard rep for GPIO in sysfs, so heres a strawman.
> Strike a match, lets have a campfire!
>
> Essentially, this seeks to describe the directory of
> 'device-attribute-files' that are populated by a driver
>
> All device-attr-files are named as <prefix>_<id>_<suffix>
>
> in LM-sensors:
> - prefix sensor-type: in(volts), temp, fan, etc. (no trailing '_')
> - id usually single integer
> - suffix the sensor attribute in question.
>
>
> GPIO-sysfs Prefix Names.
>
> Basically, GPIO hardware design appears to have 2 top-level factors;
> pin features, and pin-to-port grouping. These are mapped onto
> filename prefixes & suffixes.
>
> All GPIOs (Ive seen) are organized as 1+ ports of 8-32 bits. The
> bits' attributes are addressable individually, but are also accessible
> as a group via the port_* files. If you change a bit-attribute, that
> change will also exhibit in the port attr too.

Drop one of "also" and "too". (redundant)

> IOW, we have bit_*, port_*. They are interconnected at the hardware
> level, and (I think) there is no need for inter-locks between the
> sysfs handlers for bit_ and port_ (except for shadow regs, but I
> digress)

add period ('.')

> In fact, it might be nice to have the option of not creating the bit_*
> sysfs-device-files. For apps where user-code is doing its own
> bit-masking, the kernel could avoid some unused overheads. OTOH, this
> might be silly, premature, overoptimization. This would be controlled
> - assuming its worthwhile - by a modparam; 'nobits' or 'portsonly'
>
>
> GPIO Architectures
>
> GPIO pins have lots of hardware / architectural / naming-convention
> variations, which makes this harder. My simplifying assumption is
> that drivers should reflect the hardware capabilities directly (or
> very nearly so), and push the abstraction to user-space (at least in
> part). Obviously this needs

needs completing....

> Drivers should create sysfs 'files' only for attributes that are
> pertinent for the hardware being driven. Ths way, the absense or
s/Ths/This/
s/absense/absence/

> presense of files communicates functionality, as does their
s/presense/presence/

> readonlyness. (these 'behaviors' may be different than lm-sensors)

read-only-ness IMO.
s/these/These/
s/than/from/
add a period at end.

> Forex:

Use "For example:" or "E.g.:".

> if a pin is input only, it shouldnt have an _output_enabled attr.
> if a pin is output only, it shouldnt have an _output_enabled attr.

s/output_enabled/input_enabled/

> This way, `ls` tells you that a particular port/bit cannot possibly
> drive a value out of the chip.
>
> - one of several different values (otherwize why show it ?
> After all, you dont to be told that PI=3.14159...)

Some lines were rearranged ??

> - changed. if a pin cannot be output, theres nothing to enable, and
> showing the attribute is confusing.
>
> OTOH, a readonly _output_enabled would also convey info.

Confusing lines...

> yield the same, but not as
> visibly (ls vs ls -l)
>
> So, Im somewhat ambivalent here, looking for input....

You are doing pretty well here, with the exception of the
aversion to tic marks.

> User-Space
>
> Following LM-sensors approach, a user-side library would add the
> niceties:
>
> - provide any equivalences needed by users
> ie bit_x_tristate = ! bit_x_output_enabled.

I strongly prefer "i.e." to "ie".

> - sub-port allocation and management.
> support for 3+3+2 bit sub-ports on an 8 bit port would be nice
>
> I suspect that a sophisticated programmer would be able to add a
> sub-port allocation facility w/in the driver. I cannot,

s/,/./

> GPIO Pin Features
>
> As outlined above, pin features are represented as _<suffixes>
>
> 1st: there are several alternative naming schemes:
>
> - name-as-verb _output_enable (conveys an 'action')
> - name-as-state _output_enabled (conveys a 'current state')
> - feature-name _output (a knob to turn)
> - feature+state _output+(currval) (currval in name is bad idea)
>
> 1,2 are quite close. Ive done 2.

I can't tell which choices above are 1, 2, etc.

> FWIW, heres the pin attributes of my GPIOs, as expressed in the syslog
> by the legacy drivers: (these are

??

> [15510.384000] pc8736x_gpio.0: io16: 0x0004 TS OD PUE EDGE LO io:0/1
> [15510.564000] pc8736x_gpio.0: io17: 0x0004 TS OD PUE EDGE LO io:1/1
> [15510.744000] pc8736x_gpio.0: io18: 0x0004 TS OD PUE EDGE LO io:1/1
> [15510.928000] pc8736x_gpio.0: io19: 0x0004 TS OD PUE EDGE LO io:1/1

More confusing interspersed lines.

> # whether output-drive is on/off
> _output_enable # 1 or 0,
> _tristate # ! _output_enable, logically linked.
>
> Now, theres no need to have both of these; if there were, they would
> have to be intrinsically linked (logically opposite values).
>
> IOW, drivers should name the file as one of possible states of the
> feature, which ever best describes it, and not expose it 2x.
>
> To the extent that we need support for '_tristate' version of a
> '_output_enabled' sysfs-file, user-space (libraries) should provide
> that support.
>
> # output circuit configuration
> _opendrain # only 1 transistor, can sink current from pin
> _totem # has 2nd transistor, can drive pin hi.
> _pushpull # alias for _totempole
>
> Ive chosen _totem as the attribute name
>
> _pullup_enabled # pin tied to power via resistor.
> _pullup_off # duh
> _pullup_no # how many aliases ?
>
> _debounce # present if supported, 0 if off, 1 if on.
>
> It kinda works, but the pullup is a bit ugly, and all the aliases
> suggest some semantic difficulty/mismatch/incompleteness, but adding
> them all definitely creates clutter and has reached diminished
> incremental value.
>
>
> If hardware doesnt support a feature, like _opendrain, it:
>
> - sets _pushpull to 1, readonly ?
> - sets _opendrain to 0, readonly ?
> OR never creates _opendrain ?
>
> Doing either of these works to communicate the feature-set to
> user-space, but not creating _opendrain when pin doesnt do it means
> that the file's presense communicates this; IOW, user issues 'ls', not

s/presense/presence/

> 'ls -l' to find out.
>
> (continuing strawman)
>
> _value # read the pin
> (no-suffix) # alias for _value
>
> _current # the value 'driven' by the pin (last written)
>
> And here we can see some potential (user) difficulties;
> under some conditions,
> - read-value = current-value
>
> but not on these:
> - pin is input-only/tristate - (current is irrelevant, except as 'state')
> - pin is over-driven by attached circuit
> -- pin cannot sink/supply sufficient current
>
> Detecting these situations is both hardware and circuit dependent, and
> properly belongs in user-space. It sounds a lot like what lm-sensors
> does already.
>
> For the 2 drivers Ive 'experienced', pin control was via device-file,
> with this command-set. Presumably the correspondence with the sysfs
> strawman above is obvious.
>
> case 'O': output enabled
> case 'o': output disabled
> case 'T': output is push pull
> case 't': output is open drain
> case 'P': pull up enabled
> case 'p': pull up disabled
>
>
> Port Organization.
>
> My *vast* experience (with 1.5 GPIO architectures) suggests that all
> chips organize their GPIOs into one or more ports. Each port supports
> reading and writing all bits simultaneously.
>
> Some hardware also supports reading/writing pin-properties like
> output-enable in a single-word (todo-research). Drivers for these
> hardwares could/should create attributes for each pin-property that is
> accessible as a bit-vector.
>
> Further, port (and pin) capabilities generally vary by port; hardware
> will typically put a full set of features on 1 port, and less on
> others, expecting a designer to allocate functions to pins
> accordingly. Forex, on the pc8736x chip, port 0 can issue interrupts,

not "Forex".

> so those pins should have extra properties.
>
> These capabilities must be cleanly representable in any worthwhile
> sysfs/GPIO model (and we continue to test this strawman)..
>
>
> Port-names and Pin-names
>
> # prefixes (note the trailing _)
> port_[0..P]_
> bit_[P]_[0..bits-per-word]_
>
> Getting past the port/bit names, these files are populated by the
> driver according to the device. For the 2 drivers Ive touched, heres
> the table:
>
> driver: ports bits-per-port
> scx200_gpio 1 32
> pc8736x_gpio 4 8
>
>
> Strawman tie-together:
>
> bit_0_0_output_enable # shows current output-drive of port 0 bit 0
> bit_0_0_value
> bit_0_0 # 2 reads of same bit
>
> # lessee what happens :->

Let's see ...

> port_0_value_bin # 1-4 bytes typically returned (depending on device)
> port_0_value_hex # converted to human readable, always printable
> port_0_value #
>
> port_1_output_enable # read/write vector of enable bits to port
> port_1_<suffix_set> #
>
>
> The driver should know which properties are readable/writable in a
> bit-vector basis, and expose those sysfs-attributes only. Thus the
> presense of the port_N_value* attrs implies that all the bits in that
> port are readable at once.
>
> If the driver doesnt expose forex: port_1_output_enable, user-space is

s/forex/e.g./

> free to loop over each bit, in essence 'emulating' the port-wide
> operation.
>
>
> RESTATING - whats above kinda hangs together.
>
> NEXT - muddles
>
> pin_XY_output_state # one-of( 'output_enable', 'tristate')
>
> This might be convenient for some situations, but probably is needless
> complication / obfuscation.
>
>
> pin_XY_state_bin # binary state reader
>
> This is intended an 'escape-valve' for things that are turn out to be

s/an/as an/
s/that are/that/

> cumbersome with the above. This is probably tantamount to an IOCTL,
> so might be a hugely bad idea.
>
>
> pin_XY_interrupt_enable #
> pin_XY_interrupt_trigger_edge
> pin_XY_interrupt_trigger_level
> pin_XY_interrupt_trigger_edge_rising
> pin_XY_interrupt_trigger_edge_falling
> pin_XY_interrupt_trigger_level_hi
> pin_XY_interrupt_trigger_level_lo
>
> Well - thats a big one - Do we expose any of this ?
> - the ability to enable / disable / control hardware interrupt
> - or is that insane meddling in such affairs ?
>
> We cant afterall allow mapping of the actual interrupt handler, that
> does sound insane (unless hugely carefull)

s/carefull/careful/

> With the new genirq architecture, things are apparently more
> orthogonal, which suggests there might be something to control by
> means of attributes such as above.
>
> Further, any of the above attributes could readily be RO; they would
> convey info thats at least useful, even if 'control' is too exposing.
>
> Forex, somewhere during boot, the APIC is setup to handle interrupts;

s/Forex/E.g./

> at this point its probably known what the configured state of all
> interrrupts is, and this info could be exposed here. Whether that has
> sufficient value is unclear, and certainly not for v.submit-1
>
>
> when a pin is level-triggered (presumably this can be
> determined early in the boot process, as soon as APIC etc would be
>

eh?

>
> setup), the _edge_* attributes vanish, and the _level_{hi,lo} attrs
> are set 1/0, and RO.
>
>
> OK - IM DONE.
>
> Please be liberal with feedback -
> - this is wrong
> - poorly explained -
> - correct ... - boil this down - reduce to a guiding statement
> - strip out conjectures

I can't tell which parts of this are the documented interface
and which parts are you just thinking in bits/bytes, so I made
corrections to all of it...

I would say that you need to make a proposal and see what feedback
(if any) you get on it.

And I'm not in the right group of people to tell you how GPIO
pins should be represented.


[deleted some source files]

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/