Skip to content

Conversation

@gmarull
Copy link
Member

@gmarull gmarull commented Oct 17, 2021

This PR adds support for autogenerating valid pin configurations for GD32 devices based on a set of configuration files build from datasheets/reference manuals. The script supports both, devices using the remap model, e.g. GD32F103 and devices using the alternate function model, e.g. GD32F450.

The autogenerated definitions contain the following information:

Remap/AFIO model:

  • Port (e.g. A, B, C...)
  • Pin number (0...15)
  • Mode (input, alternate, analog)
  • AFIO configuration (PCF0/1, PCFx position, PCFx width, PCFx selection)

AF model:

  • Port (e.g. A, B, C...)
  • Pin number (0...15)
  • Mode (ANALOG, AFx, x=0..15)

The definitions can be used later in a pinctrl driver like this:

#include <dt-bindings/pinctrl/gd32vf103c(b-8)xx-pinctrl.h>

&pinctrl {
    usart1_default: usart1_default {
        pins1 {
            pinmux = <USART1_TX_PD5_RMP>, <USART1_CTS_PD3_RMP>;
        };
        pins2 {
            pinmux = <USART1_RX_PD6_RMP>, <USART1_RTS_PD4_RMP>;
            bias-pull-up;
        };
    };
};

Considerations:

  • Remap suffix is added for signals that can be remapped (AFIO model). This makes it easier to spot misconfigurations (all signals need to have the same suffix on a peripheral, since remap happens to all peripheral signals)
  • Linux model for STM32 is considered for representation. This model is quite flexible and allows solving problems like this which can't be solved in a node-based approach. Node-based approach also requires to generate a node per supported state, making it harder to extend to future states.
  • No "defaults" are introduced as in STM32 (not possible with defines anyway). While it's true that in some cases some settings will always be applied, e.g., open-drain for I2C, many others depend on PCB layout or peripheral operating conditions. Therefore, it doesn't make sense to add "defaults".

Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

Need clarify better documentation. It is not clear how user will pick model AFIO vs AF.
Need review package, see comment at docs.

@nandojve
Copy link
Member

Can you rebase and send an update?

@gmarull gmarull force-pushed the pinctrl-autogen branch 2 times, most recently from db111cc to 73b34d3 Compare October 22, 2021 20:52
@gmarull gmarull added the DNM Do Not Merge label Oct 22, 2021
@gmarull
Copy link
Member Author

gmarull commented Oct 22, 2021

Changed "package" to "pincount" to make naming more clear

Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

LGTM

Very nice @gmarull !

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add common AFIO utilities, e.g. config bit field and helper macros.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add details on the format of the pin configuration files.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@gmarull
Copy link
Member Author

gmarull commented Nov 14, 2021

@cameled reviews welcome


- `model` (required): Choose between `afio` or `af`
- `series` (required): Series name, e.g. gd32vf103
- `variants` (required): Each variant has a different set of valid pin
Copy link
Contributor

@cameled cameled Nov 16, 2021

Choose a reason for hiding this comment

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

From GD32 MCU Selection Guide, GD32F405/407 BGA100 and LQPF100 package both have 82 IO pins. But actually, the BGA100 doesn't have the PD8 pin. It can be check at datasheet 2.3, 2.6 section. GD32F403 doesn't have this issue.

I think it's a GigaDevice mistake, they told GD32F405/407 BGA100 have 82 IO too, but actually is 81.

Copy link
Member Author

@gmarull gmarull Nov 17, 2021

Choose a reason for hiding this comment

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

We can probably document that in the config file with a comment (or allow inserting comments from the config file to the generated headers). Let's do that when we introduce F405/407

Copy link
Contributor

@cameled cameled left a comment

Choose a reason for hiding this comment

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

LGTM, great work!

Copy link
Contributor

@cameled cameled left a comment

Choose a reason for hiding this comment

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

Since I'm not familiar with pinctrl related area, just add some comment.

The gd32pinctrl script autogenerates valid pinctrl definitions from a
configuration file.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Test that generated content matches reference files.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add development requirements, including flake8 config file.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add readme with some usage details (dependencies, tests, etc.)

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Signed-off-by: Gerard Marull-Paretas <[email protected]>
Allow DT to access `include/dt-bindings/...`.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add pin configurations for the GD32VF103XX series. Information has been
taken from datasheet/reference manual.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add definitions for remap options.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Files autogenerated using gd32pinctrl script.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add pin configurations for the GD32F450XX series. Information has been
taken from datasheet.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Files autogenerated using gd32pinctrl script.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add pin configurations for the GD32F403XX SoCs.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Add remap definitions for GD32F403XX SoCs.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
Files autogenerated using the gd32pinctrl.py script.

Signed-off-by: Gerard Marull-Paretas <[email protected]>
@nandojve
Copy link
Member

Hi @gmarull ,

I was wondering what is pending since we still have a DNM Label.
I think we already have a substantial work but not sure if still require more tests or you will work in docs.

@gmarull
Copy link
Member Author

gmarull commented Nov 17, 2021

Hi @gmarull ,

I was wondering what is pending since we still have a DNM Label. I think we already have a substantial work but not sure if still require more tests or you will work in docs.

I think it's ready, just resolved a small issue spotted by @cameled

@gmarull gmarull removed the DNM Do Not Merge label Nov 17, 2021
@nandojve nandojve merged commit cb4cc10 into zephyrproject-rtos:main Nov 17, 2021
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.

3 participants