Skip to content
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

drivers: add pinctrl device interface #6760

Closed
wants to merge 4 commits into from

Conversation

b0661
Copy link
Collaborator

@b0661 b0661 commented Mar 23, 2018

Overview

This PR provides a new device interface for pin controllers.

The PR is a cut-out from PR #5799 for review but can be independently applied.

The PINCTRL device driver provides applications and other drivers access:

  • to the drivers control properties such as pins, groups, pinctrl states, and functions,
  • to pin configuration,
  • to pin multiplexing, and
  • to pin state setting.

The Zephyr pin controller follows the concepts of the Linux PINCTRL (PIN CONTROL) subsystem.

The Zephyr pin controller does not provide the full scope of the Linux PINCTRL subsystem. Also the PINCTRL interface is specific to Zephyr and not compatible (but similar) to the Linux PINCTRL interface.

Motivation for or Use Case

The prime motivation is to be able to create a pinctrl driver that can configure the pins on startup by the information from the device tree in a standardized way.

Another motivation is that some pin properties (e.g. speed) are not configurable in a standardized way and have to be individually set up by the application directly accessing the HW registers.

The available pinmux interface is very restricted in configuration and muxing capabilities. A major drawback is that the current pinmux interface does not fit to the pinctrl device tree directive - see pinctrl bindings.

The new pin controller interface gives much more configuration and muxing capabilities in a standardized way. It allows to use the pinctrl bindings.

Design Details

The pin controller interface provides the pin control functions similar to Linux and UBoot. It mostly targets compile time configuration (vs. boot time configuration as Linux and UBoot).

The pin controller interface provides the current pinmux interface as a generic shim.

Pin control and GPIO is highly coupled (the same pins). The PINCTRL device interface provides a superset to the GPIO configuraton properties. If the PINCTRL device is selected GPIO devices have to use the new pin controller device for pin configuration. By this change the GPIO interface is also extended by generic pin definitions that allow to define a set of pins for GPIO operations. The generic GPIO pin definitions are automatically selected if the PINCTRL device is selected.

PINCTRL driver interface

The PINCTRL driver interface API is added in include/pinctrl.h. Defines that may be used in the device tree and in the interface are in include/pinctrl_common.h. That way they can be included from the interface and from the device tree bindings (e.g include/dt-bindings/pinctrl_stm32.h). Within include/pinctrl.h a generic shim that maps the PINMUX interface to the PINCTRL interface is added.

The application programming interface is available in two extension stages:

  • Basic interface
    Pin control, and initial pin setup at startup using device tree information.
  • Extended interface with run time access to device tree information
    Extended pin control and pin setup at run time using device tree information. The extended functionality is enabled by CONFIG_PINCTRL_RUNTIME_DTS.

Implementations of the extended interface usually need more FLASH and SRAM memory than needed by the basic interface.

GPIO driver interface extensions

For the GPIO-PINCTRL drivers (GPIO drivers that use the PINCTRL driver for pin configuration) generic PIN defines are introduced. These PIN defines allow to describe a set of pins by just or-ing the defines. Such operations on a set of pins of a port can easily be described and implemented.

By this one also get rid of the BIT() macro for masks:

gpio_init_callback(&gpio_cb, button_callback, BIT(SW0_GPIO_PIN));
gpio_pin_enable_callback(button_gpio_dev, SW0_GPIO_PIN);

becomes

gpio_init_callback(&gpio_cb, button_callback, SW0_GPIO_PIN);
gpio_pin_enable_callback(button_gpio_dev, SW0_GPIO_PIN);

Alternatives

Use current GPIO, pinmux with reduced capabilities and do the rest by the application.

Test Strategy

A working proof of concept for STM32F0x is available in PR #5799.

Signed-off-by: Bobby Noelte [email protected]

@codecov-io
Copy link

codecov-io commented Mar 23, 2018

Codecov Report

Merging #6760 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6760   +/-   ##
=======================================
  Coverage   52.19%   52.19%           
=======================================
  Files         215      215           
  Lines       26010    26010           
  Branches     5594     5594           
=======================================
  Hits        13576    13576           
  Misses      10175    10175           
  Partials     2259     2259

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce88792...a922f8c. Read the comment docs.

@b0661 b0661 force-pushed the pr_pinctrl_interface branch 2 times, most recently from 5a861ef to b162ede Compare March 23, 2018 15:39

Required properties are:

- gpio-controller: Indicates this device is a GPIO controller
Copy link
Contributor

Choose a reason for hiding this comment

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

outdent these three list items so the - lines up with the R in Required.

*********************************

Driver configuration information comes from three sources:
- autoconf.h (Kconfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a blank line before the first list item and outdent them all so the - lines up with the D in Driver.

Design Rationales
*****************

.. [#pin_number_space] GPIO uses a **local pin number space** instead of
Copy link
Contributor

Choose a reason for hiding this comment

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

These two lines won't appear in the generated documentation (is that what you intended)? .. followed by an unknown directive is treated as a comment.

a global space to allow for object encapsulation.


References
Copy link
Contributor

Choose a reason for hiding this comment

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

This section heading has no body, so you can delete it.

Driver configuration by soc.h
=============================

:file:`soc.h` may contain driver relevant information like e.g. HAL header files.
Copy link
Contributor

Choose a reason for hiding this comment

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

change like to such as (like is for comparison, while such as is for examples) and delete the e.g.

information such as HAL header files.

For a GPIO controller node the following information is extracted from the device tree
and provided in :file:`generated_dts_board.h`.

**TBD**
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be provided before merging?

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Thanks for having split the work, this really helps to focus the review.

Really looks interesting and enabling use of device tree
bindings from Linux is good as well.
One question is if such level of abstraction (such as provided in "Control properties") is really required for a RTOS like Zephyr.
If I don't know much how this is used in Linux, I understand it could be valuable for a complete framework, but in case of Zephyr, will we have the same needs?

:file:`include/gpio.h`.
The GPIO pin configuration properties can be mapped to the generic pin
configuration properties of the PINCTRL device that controls the GPIO
port pins. A GPIO driver implementation using the PINCTRL device as a backend
Copy link
Member

Choose a reason for hiding this comment

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

This last sentence is not quite clear.

Copy link
Collaborator Author

@b0661 b0661 Mar 27, 2018

Choose a reason for hiding this comment

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

GPIO drivers may ether directly access the pin configuration registers or use the pin controller device (as a backend) to do pin configuration.

The final goal should be all GPIO drivers use the PINCTRL device. But at the moment only the STM32F0x implementation in PR #5799 does that.

- :c:macro:`PINCTRL_CONFIG_BIAS_DISABLE`

**GPIO_PUD_PULL_UP**
Enable GPIO pin pull-up.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we remove the "Enable", here and below.
Previous definitions were more descriptive than functional

* @def PINCTRL_CONFIG_DRIVE_STRENGTH_2
* @brief Drive with minimum to medium drive strength.
*/
#define PINCTRL_CONFIG_DRIVE_STRENGTH_2 (2<<9)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder where these various levels of driver are coming from.

Copy link
Collaborator Author

@b0661 b0661 Apr 5, 2018

Choose a reason for hiding this comment

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

E.g. on the Raspberry Pi (BCM2835) you can select three ouput amplifiers that are in parallel (https://matt.ucc.asn.au/mirror/electron/GPIO-Pads-Control2.pdf).

*****************

.. [#pin_number_space] GPIO uses a **local pin number space** instead of
a global space to allow for object encapsulation.
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate?

Copy link
Collaborator Author

@b0661 b0661 Mar 26, 2018

Choose a reason for hiding this comment

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

Pin number space can be global to the SoC and can be local to the GPIO port. Local GPIO pin 0 to 15 may be SoC pin 32 to 47. Having a local pin number space allows for object encapsulation. The least significant port pin of GPIOA is acessed by pin number 0 and the least significant port pin of GPIOB is also accessed by pin number 0. If you would have a global pin numer space you would have to use pin number 32 to select the least significant port pin of GPIOA and pin number 48 to select the least significant port pin of GPIOB. This is how Linux does it, AFAIK.

the available *pins*.

A **pin**
is the physical input/ output line of a chip. Pins are denoted by u16_t
Copy link
Member

Choose a reason for hiding this comment

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

input/output

in the space with numbers where no pin exists.

A pins **group**
is a set of *pins* that are can be multiplexed to a
Copy link
Member

Choose a reason for hiding this comment

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

"are can"

.. code-block:: text

gpio-ranges = <&[pin controller] [pin number in GPIO pin number space]
[pin number in pin controller number space]
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate on the of opportunity of introducing pin controller space concept in Zephyr?
How will it work and be used ?

Specially, I hardly see how this could be used when some GPIO ports/nodes could be deactivated from one build to another (say if GPIO port C is not used), in that case 16 pins will be removed from the total pin controller number space. Then how to work with that kind of moving references.
On the other hand, if total pin controller number space stays constant even when a GPIO port is disabled from the build, then we'll reserve pin that don't "exist" in HW and then we're wasting footprint.

Copy link
Collaborator Author

@b0661 b0661 Mar 27, 2018

Choose a reason for hiding this comment

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

Can you elaborate on the of opportunity of introducing pin controller space concept in Zephyr?
How will it work and be used ?

The opportunity is more on the GPIO controller side. Having a GPIO port controller with its own pin number space allows to have a generic implementation for all ports. It also allows to code the pin numbers in a way to be able to define sets of GPIO pins for operations on a set of GPIO pins (PIN0 = 0x0001, PIN1=0x0002, ... PIN15 = 0x8000). The mapping between these pin numbers and the ones used by the pin controller is done by the gpio-ranges directive in the device tree. The information can also be retrieved at run time by pinctrl_get_gpio_range().

Using the same coding as for GPIOs for the pin controller pin numbers would not work. A driver implementer would therefor most probably choose to use consecutive pin numbers. You may see that as another opportunity. Driver writers are free to choose the pin numbering scheme that fits best to their driver implementation. Usually a driver provides a header file with defines for the pin numbers.

Another advantage of the pin controller pin number space is that it supports multiple identical pin controllers - e.g pin controllers controlling I/O chips that are external to the SoC.

Specially, I hardly see how this could be used when some GPIO ports/nodes could be deactivated from one build to another (say if GPIO port C is not used), in that case 16 pins will be removed from the total pin controller number space. Then how to work with that kind of moving references

The pin controller pin number space is independent from GPIOs activated or not activated. A pin controller controls a set of physical pins - independent whether these pins are used by a GPIO driver or not. If you de-activate the GPIO port the GPIO driver for the port will not be instantiated but the physical pins are still there and can be controlled by the pin controller.

Another case is if GPIO C is not available in a specific SoC variant. A pin controller driver that covers all SoC variants and must be generic would just treat that as a hole in the pin number space. There is no guarantee that pin numbers are consecutive. I mentioned that in the documentation.

On the other hand, if total pin controller number space stays constant even when a GPIO port is disabled from the build, then we'll reserve pin that don't "exist" in HW and then we're wasting footprint.

The pin controller driver holds resources for the information provided in the device tree. If there is no information regarding the non-existing GPIO port pins then there is also no wasted footprint. You have to assure that the non-existing GPIO port is deactivated in the device tree.

The pinctrl state number space is local to a *pin controller*.

A **function**
is a logical or physical function block of a chip. Functions are
Copy link
Member

Choose a reason for hiding this comment

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

To ease reading, can you provide an example of physical functional block at this point?

Copy link
Contributor

@andrewboie andrewboie left a comment

Choose a reason for hiding this comment

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

Unless I'm missing something, I do not see any handler functions defined for this new subsystem interface.

http://docs.zephyrproject.org/kernel/usermode/syscalls.html#handler-function

@b0661 b0661 force-pushed the pr_pinctrl_interface branch 2 times, most recently from d5c5273 to 72391ca Compare March 27, 2018 19:47
@b0661 b0661 requested a review from andyross as a code owner April 4, 2018 09:09
@b0661
Copy link
Collaborator Author

b0661 commented Apr 4, 2018

@andrewboie

I do not see any handler functions defined for this new subsystem interface.

Added the userspace commit including the handler functions to this PR for review.

@erwango

One question is if such level of abstraction (such as provided in "Control properties") is really required for a RTOS like Zephyr. ... Linux ... will we have the same needs?

Split the pinctrl interface in a basic interface and an extended interface to access pinctrl DTS info at run time. The run time interface can now be selected by CONFIG_PINCTRL_RUNTIME_DTS.

@b0661 b0661 force-pushed the pr_pinctrl_interface branch 3 times, most recently from 83b6b7b to 959176b Compare May 12, 2018 06:02
@b0661
Copy link
Collaborator Author

b0661 commented May 12, 2018

recheck

@SebastianBoe
Copy link
Collaborator

SebastianBoe commented May 14, 2018

I believe that the CI failure can be resolved by waiting for #7514 to be merged and then rebasing.

@b0661 b0661 force-pushed the pr_pinctrl_interface branch 3 times, most recently from e2a7367 to 0aea357 Compare May 21, 2018 08:11
@b0661
Copy link
Collaborator Author

b0661 commented May 21, 2018

This PR was kept on RFC because the measures to effectively implement a pinctrl device were unclear.

As there seems now consensus that inline code generation (PR #6762 ) is acceptable to Zephyr I remove the [RFC] tag also here.

Please start the PR review

@b0661 b0661 changed the title [RFC] drivers: add pinctrl device interface drivers: add pinctrl device interface May 21, 2018
@carlescufi
Copy link
Member

What's the status on this PR? Are we waiting for #6762 to be merged?

@b0661 b0661 force-pushed the pr_pinctrl_interface branch 2 times, most recently from f8a5664 to 97d9401 Compare June 17, 2018 19:00
@b0661
Copy link
Collaborator Author

b0661 commented Jul 10, 2018

#8815 is a typical use case for the pinctrl interface to be used to prevent/ manage concurrent access to a GPIO device.

Add the interface for pinctrl drivers.

Adapt the pinmux driver interface to use the pinctrl pinmux interface
if CONFIG_PINCTRL is selected. The pinctrl pinmux interface is a
generic shim that directly calls the pinctrl driver.

Add generic pin number defines for GPIO ports to be used by GPIO
drivers that use the pinctrl driver as backend driver.

The pin controller device driver:
    - enumerates controllable pins,
    - multiplexes pins, and
    - configures pins.

The Zephyr pin controller follows the concepts of the Linux PINCTRL
(PIN CONTROL) subsystem. However the Zephyr pin controller does not
provide the full scope of the Linux PINCTRL subsystem. Also the PINCTRL
interface is specific to Zephyr and not compatible (but similar)to the
Linux PINCTRL interface.

Signed-off-by: Bobby Noelte <[email protected]>
Add usermode interface generation for pinctrl.

Signed-off-by: Bobby Noelte <[email protected]>
Include the pinctrl driver interface and the interface for
gpio devices with pinctrl backend in the API documentation.

Signed-off-by: Bobby Noelte <[email protected]>
Document the design principles and usage of the Zephyr pinctrl driver
and the related GPIO driver with pinctrl backend.

Signed-off-by: Bobby Noelte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants