Re: New header for GPIO-like definitions

[Jason Thorpe <thorpej%me.com@localhost>]

> On Oct 26, 2020, at 8:58 AM, Julian Coleman <jdc%coris.org.uk@localhost> wrote:
> 
> Hi,
> 
> Thanks for the comments!
> 
>>>  #define GPIO_PIN_LED    0x01
>>>  #define GPIO_PIN_SENSOR 0x02
>>> 
>>> Does this seem reasonable, or is there a better way to do this?
> 
>> I don't really understand how this is different from in/out.
>> Presumably this is coming from some request from userspace originally,
>> where someone, perhaps in a config file, has told the system how a pin
>> is hooked up.

Sorry guys for chiming in a little late, but I have a keen interest in this area, and there are lots of ways this needs to be harmonized across the system, including the ability to have a generic way to specify that an interrupt source, for example, is tied to a GPIO pin (useful for things like i2c temp and light sensors that can generate interrupts when thresholds are crossed).

Might I suggest before we go to deep down the rabbit hole that you take a look at the GPIO FDT bindings?

	https://github.com/devicetree-org/devicetree-source/blob/master/Bindings/gpio/gpio.txt

Right now to use that sort of thing in the NetBSD kernel, you need to be an FDT'ized platform, but I have some experimental changes brewing in one of my hacked-up trees that attempts to make it more generic with the concepts being available on more platforms (and possibly backed by device's properties dictionary).

> 
> The definitions of pins are coming from hardware-specific properties.
> In the driver, I'd like to be able to handle requests based on what is
> connected to the pin.  For example, for LED's, attach them to the LED
> framework using led_attach(), and for sensors add them to envsys.
> 
> I wasn't planning to use this for userland, but it might be useful for
> a config file (as you mention).
> 
>> LED seems overly specific.  Presumably you care that the output does
>> something like "makes a light".  But I don't understand why your API
>> cares about light vs noise.  And I don't see an active high/low in your
>> proposal.   So I don't understand how this is different from just
>> "controllable binary output"
> 
> As above, I want to be able to route the pin to the correct internal
> subsystem in the GPIO driver.
> 
>> I am also not following SENSOR.    DO you just mean "reads if the logic
>> level at the pin is high or low".
>> 
>> I don't think you mean using i2c bitbang for a temp sensor.
> 
> Yes, just reading the logic level to display whether the "thing" connected
> is on or off.  A better name would be appreciated.  Maybe "INDICATOR", which
> would match the envsys name "ENVSYS_INDICATOR"?
> 
>> Perhaps you could step back and explain  the bigger picture and what's
>> awkward currently.  I don't doubt you that more is needed, but I am not
>> able to understand enough to discuss.
> 
> Hopefully, the above is enough, but maybe a code snippet would help (this
> snippet is only for LED's, but similar applies for other types).  In the
> hardware-specific driver, I add the pins to proplib:
> 
> 	add_gpio_pin(pins, "disk_fault", GPIO_PIN_LED,
> 	    0, GPIO_PIN_ACT_LOW, -1);
> 	...
> 	add_gpio_pin(pins, "power", GPIO_PIN_INPUT,
> 	    5, GPIO_PIN_ACT_LOW, -1);
> 	add_gpio_pin(pins, "key_normal", GPIO_PIN_SENSOR,
> 	    6, GPIO_PIN_ACT_LOW, -1);
> 
> where add_gpio_pin() has:
> 
> 	prop_dictionary_set_cstring(pin, "name", name);
> 	prop_dictionary_set_uint32(pin, "type", type);
> 	prop_dictionary_set_uint32(pin, "pin", num);
> 	prop_dictionary_set_bool(pin, "active_high", act);
> 
> Then, in the MD driver I have:
> 
> 	pin = prop_array_get(pins, i);
> 	prop_dictionary_get_uint32(pin, "type", &type);
> 	switch (type) {
> 		case GPIO_PIN_LED:
> 			...
> 			led_attach(n, l, pcf8574_get, pcf8574_set);
> 
> and because of the way that this chip works, I also need to know in advance
> which pins are input and which are output, to avoid inadvertently changing
> the input pins to output when writing to the chip.  For that, generic
> GPIO_PIN_IS_INPUT and GPIO_PIN_IS_OUTPUT definitions might be useful too.
> 
> Regards,
> 
> Julian

-- thorpej