Re: gpiobus: setting output value while in input mode
On Thu, 2019-10-24 at 19:01 +0300, Andriy Gapon wrote: > Also, if we universally implement GPIO_PIN_PRESET we still need to answer the > question. Because some consumer might still try to change an input, either by > mistake or for some reason, and we need a rule on how to handle that. Well, yeah, I guess just to zoom in on the core question of "what should happen if you try to set the state of a pin configured as input?" the answer would be "the controller should return ENODEV" and you could make a good case that it should do so regardless of the hardware's capabilities. Actually, for hardware that lets you set the output state while configured for input, where the drivers currently leverage that feature, we could both set the hardware and return ENODEV, and existing code like that in gpioiic will still work. But doing that also would require examining every existing driver and probably changing many of them. I'm not afraid of this aspect of any change we decide on... it's about 30 drivers, all of which will need minor changes. -- Ian ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: gpiobus: setting output value while in input mode
On 24/10/2019 18:38, Ian Lepore wrote:
> On Thu, 2019-10-24 at 17:04 +0300, Andriy Gapon wrote:
>> For a lack of a more specific mailing list (or my not being aware of it),
>> asking
>> here.
>>
>> gpioiic, a very simple driver, has this code:
>> ===
>> static void
>> gpioiic_setsda(device_t dev, int val)
>> {
>> struct gpioiic_softc*sc = device_get_softc(dev);
>>
>> if (val == 0) {
>> GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, sc->sda_pin, 0);
>> GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
>> GPIO_PIN_OUTPUT);
>> } else {
>> GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
>> GPIO_PIN_INPUT);
>> }
>> }
>> ===
>>
>> The interesting case is val == 0.
>>
>> I know that there are GPIO controllers where input and output values are
>> represented by different bits, so it is possible to set a future output value
>> while the pin is in input mode.
>> At the same time, there are controllers where there is a single bit per pin
>> and
>> while the pin is input mode the bit is read-only. So, it is impossible to
>> preset the pin.
>>
>> How should controllers of the second type handle GPIOBUS_PIN_SET when the
>> pin is
>> in input mode?
>> I see three options:
>> 1) just silently ignore GPIOBUS_PIN_SET or do it in a more glorious way: go
>> all
>> the way to the hardware without caring if that does anything;
>> 2) return an error that would hint that the operation is not possible at
>> this time;
>> 3) try to emulate the first class of controllers; that is, stash the value to
>> apply it when the pin is switched to output mode.
Another possibility:
4) always reject the operation under that condition
>>
>> I personally prefer 2, it's not hard to do (unlike 3) and there would be at
>> least some visibility into the problem.
>>
>
> A couple years ago I added new flags GPIO_PIN_PRESET_{LOW,HIGH}. Maybe
> we should document (where?) that the proper way to achieve the effect
> the code wants in the val==0 case is to use the preset flag along with
> the OUTPUT flag. The driver will preset the pin before changing it to
> output if it is able to do so, otherwise it will do the best it can,
> which is to set the pin to output, then set its value, perhaps
> generating a brief bogus state followed by a transition to the right
> state. Then of course we'd have to fix all existing drivers to behave
> that way, but I don't think that will be hard.
>
> My problem with #2 is that whenever you push the problem out to the
> child drivers, one of two things happens: drivers ignore the error, or
> all drivers have to have essentially identical code to handle the
> error. In this case, what could a child driver do except react to the
> error by doing the operations in the reverse order?
Well, I think that it is a choice between affected consumer drivers needing to
have some boiler plate code to handle the error and all GPIO controller drivers
needing to have some code to handle GPIO_PIN_PRESET. I am not sure which one is
less work. That is, I am not sure how many consumer drivers really *require*
the preset behavior.
Another issue is that we probably need to make *all* controller drivers support
GPIO_PIN_PRESET before any consumer drivers can start using it without extra
checks, fallback code, etc.
Also, if we universally implement GPIO_PIN_PRESET we still need to answer the
question. Because some consumer might still try to change an input, either by
mistake or for some reason, and we need a rule on how to handle that.
> The current gpio(4) documentation really only covers gpiobus(4) and a
> mentions a few of its older children and how to configure them via
> hints. We need manpages for some of the newer drivers, and we
> especially need a manpage that documents sys/gpio.h (which is used both
> from userland and internally with gpio_if.m). I can probably find some
> manpage-writing time over the new few weeks.
That would be great!
gpio would certainly benefit from more documentation.
--
Andriy Gapon
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: gpiobus: setting output value while in input mode
On Thu, 2019-10-24 at 17:04 +0300, Andriy Gapon wrote:
> For a lack of a more specific mailing list (or my not being aware of it),
> asking
> here.
>
> gpioiic, a very simple driver, has this code:
> ===
> static void
> gpioiic_setsda(device_t dev, int val)
> {
> struct gpioiic_softc*sc = device_get_softc(dev);
>
> if (val == 0) {
> GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, sc->sda_pin, 0);
> GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
> GPIO_PIN_OUTPUT);
> } else {
> GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
> GPIO_PIN_INPUT);
> }
> }
> ===
>
> The interesting case is val == 0.
>
> I know that there are GPIO controllers where input and output values are
> represented by different bits, so it is possible to set a future output value
> while the pin is in input mode.
> At the same time, there are controllers where there is a single bit per pin
> and
> while the pin is input mode the bit is read-only. So, it is impossible to
> preset the pin.
>
> How should controllers of the second type handle GPIOBUS_PIN_SET when the pin
> is
> in input mode?
> I see three options:
> 1) just silently ignore GPIOBUS_PIN_SET or do it in a more glorious way: go
> all
> the way to the hardware without caring if that does anything;
> 2) return an error that would hint that the operation is not possible at this
> time;
> 3) try to emulate the first class of controllers; that is, stash the value to
> apply it when the pin is switched to output mode.
>
> I personally prefer 2, it's not hard to do (unlike 3) and there would be at
> least some visibility into the problem.
>
A couple years ago I added new flags GPIO_PIN_PRESET_{LOW,HIGH}. Maybe
we should document (where?) that the proper way to achieve the effect
the code wants in the val==0 case is to use the preset flag along with
the OUTPUT flag. The driver will preset the pin before changing it to
output if it is able to do so, otherwise it will do the best it can,
which is to set the pin to output, then set its value, perhaps
generating a brief bogus state followed by a transition to the right
state. Then of course we'd have to fix all existing drivers to behave
that way, but I don't think that will be hard.
My problem with #2 is that whenever you push the problem out to the
child drivers, one of two things happens: drivers ignore the error, or
all drivers have to have essentially identical code to handle the
error. In this case, what could a child driver do except react to the
error by doing the operations in the reverse order?
The current gpio(4) documentation really only covers gpiobus(4) and a
mentions a few of its older children and how to configure them via
hints. We need manpages for some of the newer drivers, and we
especially need a manpage that documents sys/gpio.h (which is used both
from userland and internally with gpio_if.m). I can probably find some
manpage-writing time over the new few weeks.
-- Ian
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
gpiobus: setting output value while in input mode
For a lack of a more specific mailing list (or my not being aware of it), asking
here.
gpioiic, a very simple driver, has this code:
===
static void
gpioiic_setsda(device_t dev, int val)
{
struct gpioiic_softc*sc = device_get_softc(dev);
if (val == 0) {
GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, sc->sda_pin, 0);
GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
GPIO_PIN_OUTPUT);
} else {
GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
GPIO_PIN_INPUT);
}
}
===
The interesting case is val == 0.
I know that there are GPIO controllers where input and output values are
represented by different bits, so it is possible to set a future output value
while the pin is in input mode.
At the same time, there are controllers where there is a single bit per pin and
while the pin is input mode the bit is read-only. So, it is impossible to
preset the pin.
How should controllers of the second type handle GPIOBUS_PIN_SET when the pin is
in input mode?
I see three options:
1) just silently ignore GPIOBUS_PIN_SET or do it in a more glorious way: go all
the way to the hardware without caring if that does anything;
2) return an error that would hint that the operation is not possible at this
time;
3) try to emulate the first class of controllers; that is, stash the value to
apply it when the pin is switched to output mode.
I personally prefer 2, it's not hard to do (unlike 3) and there would be at
least some visibility into the problem.
--
Andriy Gapon
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
