Skip to content

Conversation

@BoKragelund
Copy link

@BoKragelund BoKragelund commented Mar 29, 2019

This change enables configuration of generic gpio for inputs and outputs similar to gpio-leds and gpio-keys.

Signed-off-by: Bo Kragelund [email protected]

@zephyrbot
Copy link

zephyrbot commented Mar 29, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

This change enables configuration of generic gpio for inputs
and outputs similar to gpio-leds and gpio-keys.

Signed-off-by: Bo Kragelund <[email protected]>
@henrikbrixandersen
Copy link
Member

Wouldn't something along the lines of Linux' named GPIOs be a more generic solution? So, for any given GPIO controller node in DTS, you could specify a list of GPIO names. These would then in turn be included as defines in the generated header file.

@BoKragelund
Copy link
Author

Hi Henrik
And thank you for your comment.
Any recommendation is much appreciated.
Is it possible for you to provide an example of the solution you describe?
Not coming from the Linux world myself, makes my skills rather limited with dts.

Working with the frdm_k64f board, I noticed the very nice way to specify the pins for the leds in the frdm_k64f.dts file compatible with the gpio-leds.yaml file.
And I must admit I got stuck regarding the syntax on how to add a GPIO in the frdm_k64f.dts file compatible with, which I assume is the intension in zephyr, the nxp,kinetis-gpio.yaml file.
I was only able to make it work making these new generic gpio yaml files and use a syntax similar to the leds.

@henrikbrixandersen
Copy link
Member

Is it possible for you to provide an example of the solution you describe?
Not coming from the Linux world myself, makes my skills rather limited with dts.

I was thinking something along the lines of the gpio-line-names DTS property on gpio-controllers used by the Linux kernel: https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/gpio/gpio.txt

In Zephyr, that property could easily be added to generate compile-time definitions for the named GPIOs.

@henrikbrixandersen
Copy link
Member

@BoKragelund Would my suggested solution fit your use-case?

@zephyrproject-rtos zephyrproject-rtos deleted a comment from codecov-io Jun 6, 2019
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BoKragelund and thank you for your contribution!

You may not be aware that there is a large and ongoing discussion on the future of the GPIO API, which recently resulted in this pull request: #16648.

I think that one conflicts with this one. Since the other PR has been widely discussed among the community and is being proposed by @mnkp, who has volunteeered to maintain the GPIO API in the future, I suggest that you join your contributions with the existing efforts.

Thanks!

@henrikbrixandersen henrikbrixandersen removed their request for review June 7, 2019 20:01
Copy link
Member

@mnkp mnkp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @BoKragelund for the PR. This looks like an easy issue to solve but after a closer look I don't think any more that it is. I'm not sure what would be the right thing to do, nevertheless I have some comments regarding your and other discussed proposals.

By introducing new .yaml files you introduced new (not yet complete) drivers. We would also need a corresponding gpio-in.h, gpio-out.h header as well as gpio-in.c, gpio-out.c implementation files. Looking at the LED GPIO node and the blinky example that may not be clear but that's because blinky is 'misusing' the DT description. It should have been implemented using include/led.h API. And yes, we are missing the API and driver for GPIO KEYS node. It's present in Linux from where the GPIO KEYS node was copied.

As far as I understand the problem you are trying to solve is to provide the application a general way to access a named GPIO pin which is not part of any existing driver.

If the pin was a part of a driver, e.g. some SPI drivers require to control CS pin 'by hand' rather than rely on the hardware module, then the solution already exists. As described in gpio bindings document one should use "[<name>-]gpios" property. E.g. Zephyr dts/bindings/spi/spi.yaml defines cs-gpios.

The other discussed proposal was to use gpio-line-names. I don't think it would work since this is more of a one way street. If we know the gpio controller and the pin number we want to use then by means of gpio-line-names property we can find out the pin name. But it doesn't really work the other way. E.g. to find out to which gpio controller the pin is attached.

The Linux gpio bindings document describes also concept of GPIO hogs. They can be used to initialize pins and are embedded in GPIO node. Here however we have a similar problem. Knowing the pin name doesn't really let us find out which gpio controller controls the pin.

From my point of view the 'cleanest' solution would be to use "[<name>-]gpios" property attached to the root, board or soc DTS node. But we currently miss this part of the infrastructure, i.e. we don't provide any .yaml files for root/board/soc DTS nodes.

Maybe someone has a better idea.

@mbolivar mbolivar dismissed their stale review June 10, 2019 13:34

Dismissing my review since mnkp is on it

@BoKragelund
Copy link
Author

Thank you @mbolivar and @mnkp for your comments.
And I'm very sorry for my late reply.

It is great to see/read, that making a GPIO API is an ongoing process :-)

@galak galak added area: Devicetree Binding PR modifies or adds a Device Tree binding area: GPIO and removed area: Devicetree labels Jan 30, 2020
@galak
Copy link
Contributor

galak commented Aug 4, 2020

See #25945 as to the provider solution path for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: GPIO

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants