Re: [PATCH v3 3/8] ARM: dts: microchip: sama5d27_som1_ek: Add power-supply properties for sdmmc nodes

[claudiu beznea]

Hi, Mihai,

I'm replying to this series as the description is still better to me than
the one from v4, but, few notes:

On 06.02.2024 14:03, Mihai Sain wrote:
> The sdmmc0 and sdmmc1 controllers are powered from 3.3V regulator.

In schematic at [1] (for SDMMC0 at least) I see VDDSDHC could be either 1v8
or 3v3 and is selected by SDMMC_VDDSEL signal. Maybe update the description
to reflect the this.

[1]
https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/UserGuides/SAMA5D27-SOM1-Kit1-Users-Guide-DS50002667.pdf

> Add vmmc-supply and vqmmc-supply properties to sdmmc nodes.
> The sdmmc controller from SAMA5D2 MPU has support for
> IO voltage signaling/switching required by the UHS sd-card.

Ok, it has support as you mentioned in one of the replies to v1.

> In order to avoid the issues from the tuning procedure required by
> the UHS cards, keep the vqmmc at 3V3 to use the sd high-speed mode.

But it is still not clear enough for me why you want to keep it at 3v3.
Maybe it would have been clearer if you kept the statement from your reply
to v1:

"On the SAMA5 MPUs there is support for IO voltage switching but since we
have issues with the tuning procedure required but the UHS cards,
we want to keep vqmmc at 3V3 in order to use high-speed mode"

Thank you,
Claudiu Beznea


> 
> Signed-off-by: Mihai Sain <mihai.sain@xxxxxxxxxxxxx>
> ---
>  arch/arm/boot/dts/microchip/at91-sama5d27_som1_ek.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_som1_ek.dts b/arch/arm/boot/dts/microchip/at91-sama5d27_som1_ek.dts
> index f3ffb8f01d8a..255ee0640133 100644
> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_som1_ek.dts
> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_som1_ek.dts
> @@ -56,6 +56,8 @@ sdmmc0: sdio-host@a0000000 {
>  			bus-width = <8>;
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&pinctrl_sdmmc0_default>;
> +			vmmc-supply = <&vddin_3v3>;
> +			vqmmc-supply = <&vddin_3v3>;
>  			status = "okay";
>  		};
>  
> @@ -63,6 +65,8 @@ sdmmc1: sdio-host@b0000000 {
>  			bus-width = <4>;
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&pinctrl_sdmmc1_default>;
> +			vmmc-supply = <&vddin_3v3>;
> +			vqmmc-supply = <&vddin_3v3>;
>  			status = "okay";
>  		};
>