Skip to content

Conversation

@mbolivar-nordic
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic commented Aug 12, 2021

The commit which introduces new pin control helper macros to devicetree.h is:

devicetree: add devicetree/pinctrl.h

This contains accessor macros for getting phandles out of pinctrl
properties by name and index. As usual, the representation in C for a
phandle is a node identifier.

Add these new macros:

- DT_PINCTRL_BY_IDX(node_id, pc_idx, idx): phandle at index idx
  in the pinctrl-<pc_idx> property

- DT_PINCTRL_0(node_id, idx): pinctrl-0 convenience for the same

- DT_PINCTRL_BY_NAME(node_id, name, idx): phandle at index idx
  in the pinctrl property named 'name'

- DT_PINCTRL_NAME_TO_IDX(node_id, name): convert a pinctrl property
  name to its index number

- DT_NUM_PINCTRLS_BY_IDX(node_id, pc_idx): number of phandles in
  pinctrl-<pc_idx>

- DT_NUM_PINCTRLS_BY_NAME(node_id, name): number of phandles in a
  named pinctrl property

- DT_NUM_PINCTRL_STATES(node_id): total number of pinctrl-<pc_idx>
  properties

- DT_PINCTRL_HAS_IDX(node_id, pc_idx): does pinctrl-<pc_idx> exist?

- DT_PINCTRL_HAS_NAME(node_id, name): does a named pinctrl property
  exist?

- DT_PINCTRL_IDX_TO_NAME_TOKEN(node_id, pc_idx): convert a pinctrl
  index to its name as a token, similar to DT_STRING_TOKEN()

- DT_PINCTRL_IDX_TO_NAME_UPPER_TOKEN(node_id, pc_idx): like
  DT_PINCTRL_IDX_TO_NAME_TOKEN, but with an uppercase result

As well as DT_DRV_INST equivalents, which take inst wherever node_id
appears above:

- DT_INST_PINCTRL_BY_IDX()
- DT_INST_PINCTRL_0()
- DT_INST_PINCTRL_BY_NAME()
- DT_INST_PINCTRL_NAME_TO_IDX()
- DT_INST_NUM_PINCTRLS_BY_IDX()
- DT_INST_NUM_PINCTRLS_BY_NAME()
- DT_INST_NUM_PINCTRL_STATES()
- DT_INST_PINCTRL_HAS_IDX()
- DT_INST_PINCTRL_HAS_NAME()
- DT_INST_PINCTRL_IDX_TO_NAME_TOKEN()
- DT_INST_PINCTRL_IDX_TO_NAME_UPPER_TOKEN()


Signed-off-by: Martí Bolívar <[email protected]>

The rest of the PR is prep work, bookkeeping, and documentation.


This change is Reviewable

@github-actions github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree Tooling PR modifies or adds a Device Tree tooling area: Documentation area: I2C area: PWM Pulse Width Modulation area: Tests Issues related to a particular existing or missing test labels Aug 12, 2021
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Some initial thoughts, thanks for this DT update

Copy link
Contributor

@galak galak left a comment

Choose a reason for hiding this comment

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

Looks good, one comment on what _BY_IDX means and impact on PINCTRL_0 api variants.

Copy link
Member

@brockus-zephyr brockus-zephyr left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 3 files at r1, 18 of 18 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ABOSTM, @FRASTM, @henrikbrixandersen, @ioannisg, @jackrosenthal, @katsuster, @keith-zephyr, @kgugala, @MaureenHelm, @mgielda, @nandojve, @nashif, @pgielda, and @sjg20)

This will be necessary to going back and forth between indexes and
names in C.

Signed-off-by: Martí Bolívar <[email protected]>
Missing 's'.

Signed-off-by: Martí Bolívar <[email protected]>
Move the partition handling code into its own function and rework the
comment. This is prep work for adding additional generated macros to
this function.

Signed-off-by: Martí Bolívar <[email protected]>
We need to be able to access pinctrl-<index> property contents by
name, convert names to indexes, convert indexes to names, and perform
existence checks by name and by index.

This is currently not possible because we don't track these properties
the same way we do other named properties. That in turn is because
they are different then the usual named properties.

The usual case looks like this, picking DMAs just for the sake of
example:

  dmas = <&dma0 ...>, <&dma1 ...>;
  dma-names = "tx", "rx";

So "tx" is the name for the <&dma0 ...> element, and "rx" is the name
for the <&dma1 ...> element, all in a single "dmas" property.

By contrast, pinctrl properties look like this:

  pinctrl-0 = <&foo &bar ...>;
  pinctrl-1 = <&baz &blub ...>;
  pinctrl-names = "default", "sleep";

Here, "default" is the name for the entire pinctrl-0 property.
Similarly, "sleep" is the name of the pinctrl-1 property. It's a
strange situation where the node itself is kind of a container for an
array of pin control properties, which Zephyr's bindings language
can't really capture and has some special case handling in edtlib.

This is easiest to handle with ad-hoc code. Add special case macros
for pinctrls.

Signed-off-by: Martí Bolívar <[email protected]>
This isn't following the <compatible>.yaml naming convention; fix it.

Signed-off-by: Martí Bolívar <[email protected]>
This contains accessor macros for getting phandles out of pinctrl
properties by name and index. As usual, the representation in C for a
phandle is a node identifier.

Add these new macros:

- DT_PINCTRL_BY_IDX(node_id, pc_idx, idx): phandle at index idx
  in the pinctrl-<pc_idx> property

- DT_PINCTRL_0(node_id, idx): pinctrl-0 convenience for the same

- DT_PINCTRL_BY_NAME(node_id, name, idx): phandle at index idx
  in the pinctrl property named 'name'

- DT_PINCTRL_NAME_TO_IDX(node_id, name): convert a pinctrl property
  name to its index number

- DT_NUM_PINCTRLS_BY_IDX(node_id, pc_idx): number of phandles in
  pinctrl-<pc_idx>

- DT_NUM_PINCTRLS_BY_NAME(node_id, name): number of phandles in a
  named pinctrl property

- DT_NUM_PINCTRL_STATES(node_id): total number of pinctrl-<pc_idx>
  properties

- DT_PINCTRL_HAS_IDX(node_id, pc_idx): does pinctrl-<pc_idx> exist?

- DT_PINCTRL_HAS_NAME(node_id, name): does a named pinctrl property
  exist?

- DT_PINCTRL_IDX_TO_NAME_TOKEN(node_id, pc_idx): convert a pinctrl
  index to its name as a token, similar to DT_STRING_TOKEN()

- DT_PINCTRL_IDX_TO_NAME_UPPER_TOKEN(node_id, pc_idx): like
  DT_PINCTRL_IDX_TO_NAME_TOKEN, but with an uppercase result

As well as DT_DRV_INST equivalents, which take inst wherever node_id
appears above:

- DT_INST_PINCTRL_BY_IDX()
- DT_INST_PINCTRL_0()
- DT_INST_PINCTRL_BY_NAME()
- DT_INST_PINCTRL_NAME_TO_IDX()
- DT_INST_NUM_PINCTRLS_BY_IDX()
- DT_INST_NUM_PINCTRLS_BY_NAME()
- DT_INST_NUM_PINCTRL_STATES()
- DT_INST_PINCTRL_HAS_IDX()
- DT_INST_PINCTRL_HAS_NAME()
- DT_INST_PINCTRL_IDX_TO_NAME_TOKEN()
- DT_INST_PINCTRL_IDX_TO_NAME_UPPER_TOKEN()

Signed-off-by: Martí Bolívar <[email protected]>
Test cases for new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Add a new section in the API reference for the newly added
devicetree/pinctrl.h macros.

Amend macros.bnf in the guide to reflect the new generated macros for
getting at pinctrl information by name.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs. Fix a typo while I'm here.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
Update to use the new APIs.

Signed-off-by: Martí Bolívar <[email protected]>
@mbolivar-nordic mbolivar-nordic removed the DNM This PR should not be merged (Do Not Merge) label Aug 20, 2021
@mbolivar-nordic
Copy link
Contributor Author

mbolivar-nordic commented Aug 20, 2021

Rebased and removed DNM. I'm going to leave it up to @galak to decide if we want this in for LTS.

@cfriedt
Copy link
Member

cfriedt commented Aug 20, 2021

@galak - feel free to merge or if you would like to hold off apply the DNM label again

Copy link
Member

@brockus-zephyr brockus-zephyr left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ABOSTM, @FRASTM, @henrikbrixandersen, @ioannisg, @jackrosenthal, @katsuster, @keith-zephyr, @kgugala, @MaureenHelm, @mgielda, @nashif, @pgielda, and @sjg20)

@nashif nashif merged commit 36c73da into zephyrproject-rtos:main Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ADC Analog-to-Digital Converter (ADC) area: API Changes to public APIs area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree Tooling PR modifies or adds a Device Tree tooling area: Devicetree area: Documentation area: I2C area: PWM Pulse Width Modulation area: RISCV RISCV Architecture (32-bit & 64-bit) area: Tests Issues related to a particular existing or missing test platform: ITE ITE platform: Nuvoton NPCX Nuvoton NPCX platform: STM32 ST Micro STM32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants