Re: [RFC] STM32 pinmux - simplify pinmux logic


Christer Weinigel
 

Hi Erwan,

On 2017-01-17 13:38, Erwan Gouriou wrote:
I agree with the observation that gpio/pinmux configuration is
overcomplicated.
Your proposal is interesting and look rather easy to use.
Maybe you can also have a look to Adam's porting of STM32F3x family, in
which
he's trying to by pass pin configuration this in another way:
https://gerrit.zephyrproject.org/r/#/c/9701
It looks quite similar, if I understood this correctly, he's also added
some macros to combine the pull up/pull down/function with the alternate
function in one word:

https://gerrit.zephyrproject.org/r/#/c/9325/19/drivers/pinmux/stm32/pinmux_stm32.h

Although I think it's nicer to do what I did and put the pin information
in drivers/pinmux/stm32/pinmux_stm32f4.h since it puts everything in one
file.

Other than that it looks like a nice patchset to add support for the
STM32F3xx family.

After thinking a bit more about this, I'm actually not too fond of magic
macros like my STM32F4_PINMUX_ENCODE. Sometimes abstractions can be
nice, but in this case I actually thinks that it obscures more than it
helps. I would actually prefer to change the defines to have the a
shift built in, that is change the stm32f4x_pin_config_mode enum to defines:

#define STM32F4X_PIN_CONFIG_AF_PUSH_MASK (15 << 8)
#define STM32F4X_PIN_CONFIG_AF_PUSH_PULL (0 << 8)
#define STM32F4X_PIN_CONFIG_AF_PUSH_UP (1 << 8)
#define STM32F4X_PIN_CONFIG_AF_PUSH_DOWN (2 << 8)
...
STM32F4X_PIN_CONFIG_ANALOG (15 << 8)

so that the defines in pinmux_stm32.h can be a bit more explicit not
have to use the STM32F4_PINMUX_ENCODE macro:

#define STM32F4_PINMUX_FUNC_PA9_USART1_TX
(STM32F4X_PIN_CONFIG_AF_PUSH_UP | STM32_PINMUX_FUNC_ALT_7)

this also allows more flexibility, we're no longer limited to just two
axes of parameters. For example AF_PUSH_UP actually combines three
different settings. We can split that into MODE_AF, PULL_UP and
OTYPE_PUSH_PULL:

#define STM32F4X_PIN_MODE_MASK (3 << 8)
#define STM32F4X_PIN_MODE_INPUT (0 << 8)
#define STM32F4X_PIN_MODE_OUTPUT (1 << 8)
#define STM32F4X_PIN_MODE_AF (2 << 8)
#define STM32F4X_PIN_MODE_ANALOG (3 << 8)

#define STM32F4X_PIN_PULL_MASK (3 << 10)
#define STM32F4X_PIN_PULL_NONE (0 << 10)
#define STM32F4X_PIN_PULL_UP (1 << 10)
#define STM32F4X_PIN_PULL_DOWN (2 << 10)

#define STM32F4X_PIN_OTYPE_MASK (1 << 12)
#define STM32F4X_PIN_OTYPE_PUSH_PULL (0 << 12)
#define STM32F4X_PIN_OTYPE_OPEN_DRAIN (1 << 12)

and for completeness we could also expose the speed settings:

#define STM32F4X_PIN_SPEED_MASK (3 << 13)
#define STM32F4X_PIN_SPEED_LOW (0 << 13)
#define STM32F4X_PIN_SPEED_MEDIUM (1 << 13)
#define STM32F4X_PIN_SPEED_FAST (2 << 13)
#define STM32F4X_PIN_SPEED_HIGH (3 << 13)

The above pin definition would look like this:

#define STM32F4_PINMUX_FUNC_PA9_USART1_TX (STM32F4X_PIN_FUNC_AF |
STM32F4X_PIN_PULL_UP | STM32_PINMUX_FUNC_ALT_7)

It's a bit more verbose, but it's obvious what it does, especially if
you look at the corresponding definitions in the data sheet since this
is how the hardware registers look.

If we do this we can rip out stm32_get_pin_config completely and also
simplify the __func_to_foo functions in soc_gpio.c

It might also be a good idea to unify these defines between STM32
families. If I recall correctly many STM32F1xx, STM32F2xx and STMF4xx
packages are mostly pin compatible. The same PCB could have a STM32Fxx
or STM32F4xx MCU mounted on it (with a zero ohm resistor or capacitor to
account for any differences between them) and it would be nice if they
could use the same board file.

Some functions might not be supported on all families, for example, I
don't think the STM32F1xx can have a pull-up/pull-down at the same tie
as a pin is an output or alternate function, but that shouldn't be problem.

Btw, I think that you could submit your RFC as a patch in Gerrit, so we can
better appreciate the differences and comment easily.
I'll see if I can clean up my ideas and push a few patches for review in
gerrit.

/Christer

Join devel@lists.zephyrproject.org to automatically receive all group messages.