-
Notifications
You must be signed in to change notification settings - Fork 183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for GPIO Control using libgpiod (as sysfsgpio is deprecated) #1569
base: master
Are you sure you want to change the base?
Conversation
db29152
to
d114043
Compare
Rebased so that the commits are no longer behind the main branch :) |
e6486a4
to
4003d51
Compare
A button can be used to simulate pressing a physical button on a device. For example a DigitalOutputButtonDriver can be used when a reset button is connected via a relay to a DigitalOutput. In addition there are the ManualButtonDriver and ExternalButtonDriver drivers analogous to the ManualPowerDriver and ExternalPowerDriver drivers. Operations on a button include "press", "release", "press_for", and "get". Signed-off-by: Perry Melange <[email protected]>
…tput The "invert" method should invert the value of a digital output. Signed-off-by: Perry Melange <[email protected]>
Add the LibGPIO, NetworkLibGPIO and MatchedLibGPIO resources. These resouces bind to the /dev/gpiochipN and it's associated gpio line. In addition there is an optional active_low attribute which can be used to invert the logical value used on the gpio line. The resources SysfsGPIO, NetworkSysfsGPIO and MatchedSysfsGPIO resources have been modified to have an additional optional active_low attribute which can be used to invert the logical value used on the gpio line. The SysfsGPIO, NetworkSysfsGPIO and MatchedSysfsGPIO resouces are now marked as deprecated. See https://www.kernel.org/doc/Documentation/gpio/sysfs.txt Signed-off-by: Perry Melange <[email protected]>
Add the LibGPIODigitalOutputDriver which implements the DigitalOutputProtocol, ResetProtocol, PowerProtocol, and ButtonProtocol. Update the GPIODigitalOutputDriver to also implement the ResetProtocol, PowerProtocol, and ButtonProtocol. Update the GpioDigitalOutput agent to use the new active_low attribute Mark GPIODigitalOutputDriver and GpioDigitalOutput as deprecated. See https://www.kernel.org/doc/Documentation/gpio/sysfs.txt Signed-off-by: Perry Melange <[email protected]>
Add the LibGPIOExport class to the exporter. Update the GPIOSysFSExport class to use the new active_low attribute Signed-off-by: Perry Melange <[email protected]>
4003d51
to
987021c
Compare
…otocol Add NetworkLibGPIO to the client, including commands for io, power, and button protocols. Update NetworkLibGPIO to support the commands for power and button protocols. Signed-off-by: Perry Melange <[email protected]>
…Drivers Add documentation for LibGPIO, MatchedLibGPIO, ManualButtonDriver, ExternalButtonDriver, DigitalOutputButtonDriver, and LibGPIODigitalOutputDriver. Update the SysfsGPIO, MatchedSysfsGPIO, and GpioDigitalOutputDriver to add the new active_low attribute and the new protocols supported by the GpioDigitalOutputDriver. In addition, add documentation that the SysfsGpio classes are deprecated. Signed-off-by: Perry Melange <[email protected]>
Signed-off-by: Perry Melange <[email protected]>
Signed-off-by: Perry Melange <[email protected]>
Signed-off-by: Perry Melange <[email protected]>
987021c
to
2ede768
Compare
@@ -906,6 +938,8 @@ def digital_io(self): | |||
drv = self._get_driver_or_new(target, "HttpDigitalOutputDriver", name=name) | |||
elif isinstance(resource, NetworkDeditecRelais8): | |||
drv = self._get_driver_or_new(target, "DeditecRelaisDriver", name=name) | |||
elif isinstance(resource, NetworkLibGPIO): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With NetworkSysfsGPIO
, lines are requested by the kernel and GPIOs remain driven even after labgrid-client
terminates. With LibGPIODigitalOutputDriver
, it seems to me that this won't be the case.
To quote gpioset(1)
:
The line output state is maintained until the process exits, but after that is not guaranteed.
In practice, some kernel drivers keep driving the last output and others don't. Does your PR address this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With all the tests I have done, the line state stays the same. Also, I am not calling command gpioset
After completion of a command via labgrid-client, such as power on
, the line stays on. Afterwards, I can get with power get
and call power cycle
and the GPIO lines act as expected. Do you have a specific test scenario that I could use to test with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With all the tests I have done, the line state stays the same.
I haven't tested on a Raspberry Pi, but you can try:
- boot kernel with
pinctrl_bcm2835.persist_gpio_outputs=n
- remove optocoupler/latches and work on the raw GPIO pin directly
- attach weak pull-down
- set GPIO high
- terminate GPIO setting program
- measure GPIO line
Raspberry Pi users have a way out using above module parameter, but in general, it's up to the driver what happens when a line is released.
If this happens to work, one would need to check what is preventing the kernel from calling bcm2835_pmx_gpio_disable_free()
, which would mux the pin as input.
Also, I am not calling command gpioset
This behavior is inherent to the GPIOD API, gpioset
is only one example of a tool affected by it.
After completion of a command via labgrid-client, such as power on, the line stays on. Afterwards, I can get with power get and call power cycle and the GPIO lines act as expected. Do you have a specific test scenario that I could use to test with?
Undefined can mean that it works for some, because they have the lucky combination of hardware, kernel module parameter, other GPIOs requested on the same controller, ... etc, but it's not universal.
The GPIOD DBus API is one solution for this problem. I don't know if it's a suitable solution for Labgrid, but replacing SysFS usage with libgpiod without ensuring the lines remain requested won't cut it, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did a bit more research. https://pip.raspberrypi.com/categories/685-whitepapers-app-notes-compliance-guides/documents/RP-006553-WP/A-history-of-GPIO-usage-on-Raspberry-Pi-devices-and-current-best-practices.pdf on page 9 had some information which I didn't know before.
I have modified the config parameters on the RaspberryPi4 which I am using to include dtparam=strict_gpiod
. In this mode, I see the instability of the gpio line which you have described. But only on gpiochip0. On the HATs which I am using I do not see the instability.
Is this the reason that an agent was used for the GpioDigitalOutput class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An agent is just a way to execute some Python code on the exporter. The main issue with the gpiochip/libgpiod API in some cases state is lost as soon as labgrid-client io high
exits (which also terminates any agents it started on the exporter, closing the gpiochip FD).
I'm not sure if that has been solved with the daemon and D-Bus API. If so, using that instead of the sysfs could be an option.
There have been past discussion about this fyi, e.g. #468 This shouldn't mean that libgpiod is not wanted (I don't make maintenance decisions anyway), but just so you are aware of past arguments for and against. |
Description
As far as new features go...
ButtonProtocol is to be used in my test environment to control pressing the reset button of DUTs. For many of the DUTs, pressing the reset button when powering on provides (via uboot) a TFTP client or server or simply access to a web socket to be able to upload a new firmware image. A logical button is better for the test environment I am creating since it encapsulates the high/low logic levels of IO ports into something logical.
LibGPIO classes are added to get away from the deprecated sysfsgpio subsystem (see https://www.kernel.org/doc/Documentation/gpio/sysfs.txt).
Extending the GPIO based drivers (both libgpio and sysfs) to the Reset, Power, and Button protocols was very straight forward and I did wonder why the Reset and Power protocols had not jet been implemented. They seem like logical extensions to general purpose IO lines.
Adding the active_low attribute was necessary because the IO lines used in the testbed are connected to relays which are active_low.
All development and testing was done on a raspberry pi 4 with opto-coupler relays connected to the GPIO lines to control power and reset buttons.
Checklist