[Linux-stm32] [PATCH] ARM: dts: stm32: Consistently enable internal pull-ups for SD bus

[Alexandre Torgue]

Hi

On 10/15/20 3:47 PM, Ahmad Fatoum wrote:
> Hello,
> 
> On 10/15/20 3:40 PM, Marek Vasut wrote:
>>>> Currently the
>>>> stm32mp15-pinmux.dtsi is not consistent in that aspect. I am not arguing
>>>> about any one single board, but about the dtsi, I suspect by default the
>>>> pull ups should be enabled, and possibly on board-DT-level they should
>>>> be disabled if not needed instead ?
>>>
>>> I think this is a good idea. But existing boards should be fixed up so that
>>> their old behavior is maintained.
>>
>> So, which boards do we fix. The automation-1 board and the ST ones ?
> 
> fixed up, not fixed. I don't know if ST is fine with the changes, for
> the MC-1, please do:
> 
> -------------- 8< -------------
> diff --git a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
> index 5700e6b700d3..7134466256b3 100644
> --- a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
> @@ -208,6 +208,14 @@ pins2 {
>          };
>   };
> 
> +&sdmmc2_d47_pins_b {
> +       /* board already has external 47K pull-ups */
> +
> +       pins {
> +               /delete-property/ bias-pull-up;
> +               bias-disable;
> +       };
> +};
> +

This proposition remind me an old discussion we got with Marek about 
"where pin definitions has to be done ?". My approach is to not define 
pins groups inside board dts file mainly because pinmux is a SoC 
configuration and a board only use one of those configurations blablabla 
:). But counter-argument is that pull-up/pull-down, Open-drain push-pull 
settings are "driven" by the board configuration and I agree.

We are exactly in this case here but before to rework all STM32 DT to 
split pin config I propose to keep what we have today, and to enable 
bias-pull-up, (or other settings) in boards that need it.

Marek, what's your feeling ?

cheers
alex



>   &sdmmc2 {
>          pinctrl-names = "default", "opendrain", "sleep";
>          pinctrl-0 = <&sdmmc2_b4_pins_a &sdmmc2_d47_pins_b>;
> 
>