Skip to content

Conversation

@erwango
Copy link
Member

@erwango erwango commented Nov 6, 2019

Introduce Linux like dts bindings for stm32 devices.
Reason for this change is described with more detail in dedicated commit.

Ultimate goal is to generate -pinctrl.dtsi files from STM23CubeMx GPIO IP database (which is available for download to all users). Scripts could then be used for generation and correctness verification.

While the number of GPIO IPs to describe is limited (54 GPIO IPs for 988 MCUs according to latest database in CubeMx 5.4), I'd like we could re-use the current inclusion/inheritance model currently in use in dts/arm/st/*. This means that each stm32f/l/g(/...)xxx.dtsi would include a matching stm32f/l/gxxx-pinctrl.dtsi describing the pin configuration of the peripheral described in stm32f/l/gxxx.dtsi. Aim is to minimize pinctrl file sizes (and information redundancy, potential errors, ..)
Alternative option would be to provide stm32f/l/gxxx-pinctrl.dtsi de-correlated from stm32f/l/gxxx.dtsi but describing various GPIO IPs and using its own inclusion/inheritance schema. This should further reduce the number of duplicates, but would require additional verification effort (which might be feasible with appropriate tooling)

EDIT: Aim of this draft PR is to get early feedbacks on the direction

@galak galak force-pushed the dev_pinmux branch 2 times, most recently from c8708db to 0182252 Compare November 6, 2019 17:29
@b0661
Copy link
Contributor

b0661 commented Nov 6, 2019

It is unclear to me what the binding file shall describe:

  1. a pinmux driver
  2. a pinctrl driver
  3. a pin control configuration to be applied to the hardware
  4. ???

In case of 2&3 using st,stm32-pinmux.yaml as the name for the binding file is misleading.

Looking at the content of the binding file I would say it describes a pin control configuration node (aka 3). These nodes are sub-nodes of a pin-controller node. In STM32 Linux https://elixir.bootlin.com/linux/v5.2.21/source/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt these nodes used to be called pin function nodes. Linux generically calls them pin configuration nodes (see https://www.kernel.org/doc/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt). EDTS calls them pinconf nodes.

As a common denominator I would use st,stm32-pinconf.yaml.

I´m not aware of that currently a binding can include another binding as a child-binding. So st,stm32-pinmux.yaml may have to become part of the pin-controller binding file as a child-binding. @ulfalizer may know better.

@galak galak added the dev-review To be discussed in dev-review meeting label Nov 6, 2019
@ulfalizer
Copy link
Contributor

ulfalizer commented Nov 6, 2019

It is unclear to me what the binding file shall describe:

  1. a pinmux driver
  2. a pinctrl driver
  3. a pin control configuration to be applied to the hardware
  4. ???

Yeah, this is confusing to me too. To use the binding as it's currently written, you'd have to do this, repeating the compatible string for each configuration node:

pinctrl: pin-controller@48000000 {
	/* USART3_TX */
	usart3_tx_pb10: usart3_tx_1 {
		compatible = "st,stm32-pinmux";
		pinmux = <STM32_PINMUX('B', 10, AF7)>;
		drive-push-pull;
		bias-pull-up;
	};
	usart3_tx_pc4: usart3_tx_2 {
		compatible = "st,stm32-pinmux";
		pinmux = <STM32_PINMUX('C', 5, AF7)>;
		drive-push-pull;
		bias-pull-up;
	};
	...
};

Was it meant to be used like this?

pinctrl: pin-controller@48000000 {
	compatible = "st,stm32-pinmux"
	...
};

In that case, you need to use a child-binding: key instead (see the documentation in dts/binding-template.yaml).

As a common denominator I would use st,stm32-pinconf.yaml.

What about st,stm32-pin-controller.yaml, since that's the node it would appear on?

@galak
Copy link
Contributor

galak commented Nov 6, 2019

We should assume the .dts files are roughly correct and that the binding is what should be updated.

@galak
Copy link
Contributor

galak commented Nov 6, 2019

So I think it should be something like:

title: STM32 PINMUX

description: |
    This binding gives a base representation of the STM32 PINMUX

compatible: "st,stm32-pinmux"

include: base.yaml

properties:
    reg:
      required: true
        
child-binding:
    properties:
        pinmux:
          required: false
          type: int 
          
        bias-disable:
          required: false
          type: boolean
...

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to have a title: and description: too. The contents of child-binding: is parsed like a regular binding.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ulfalizer
The usage of child-binding did not work out for me with the current edtlib.py.

In #22255 I made a RFC for a different solution using a generic binding for the pinctrl-state nodes (and some other similar node types - dts: edtlib: enable bindings for special nodes without compatibles).

@erwango
Copy link
Member Author

erwango commented Nov 7, 2019

@ulfalizer, question around dts parsing:

Do you think the following pinctrl nodes descriptions would be equivalent from parsing point of view?

			usart1_pins_a: usart1@0 {
				pins1 {
					pinmux = <STM32_PINMUX('A', 9, AF7)>; /* USART1_TX */
					bias-disable;
					drive-push-pull;
					slew-rate = <0>;
				};
				pins2 {
					pinmux = <STM32_PINMUX('A', 10, AF7)>; /* USART1_RX */
					bias-disable;
				};
			};

used as follows:

&usart1 {
	pinctrl-0 = <&usart1_pins_a>;
	pinctrl-names = "default";
...

I don't intent to use this in zephyr, but this is the way it is done in Linux right now and it would be ideal if this would work out of the box in zephyr as well (seems equivalent to me).

Introduce Linux like dts bindings for stm32 devices.
Difference is that pinctrl subnodes describe a single pin
configuration for a dedicated function (eg: usart1_tx_pa9) while
linux proposes directly pins combinations aiming at configuring an IP.
Reason for this choice is to be able to provide a complete description
of possible configurations in a finite number of subnodes.
This will prevent need to modify these files each time a user wants
to use a new pins combination for his own board.
Other advantage is that pin configuration will appear more clearly
in board dts files.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango erwango added the DNM This PR should not be merged (Do Not Merge) label Dec 20, 2019
@erwango
Copy link
Member Author

erwango commented Mar 18, 2020

@mnkp, can you provide here a sample of alternative pinmux bindings (as discussed in previous dev-meeting) ?

@erwango erwango closed this Jun 5, 2020
@erwango erwango deleted the dev_pinmux branch June 5, 2020 10:13
@erwango erwango restored the dev_pinmux branch June 5, 2020 10:13
@erwango erwango deleted the dev_pinmux branch January 27, 2022 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: Boards area: Devicetree area: Pinmux dev-review To be discussed in dev-review meeting DNM This PR should not be merged (Do Not Merge) platform: STM32 ST Micro STM32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants