Re: [PATCH 3/3] arm64: dts: s32g2: add USDHC support

[Radu Nicolae Pirea (NXP OSS)]

Hi Chester,

On Thu, 2021-10-21 at 15:13 +0800, Chester Lin wrote:
> Add a mmc node to support USDHC on NXP S32G2 platforms.
> 
> Signed-off-by: Chester Lin <clin@xxxxxxxx>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi      | 32
> +++++++++++++++++++
>  .../arm64/boot/dts/freescale/s32g274a-evb.dts |  4 +++
>  .../boot/dts/freescale/s32g274a-rdb2.dts      |  4 +++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index 59ea8a25aa4c..19e2e2561374 100644
> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> @@ -79,6 +79,26 @@ psci {
>                 };
>         };
>  
> +       clocks {
> +               usdhc_clk_module: usdhc_clk_module {
> +                       compatible = "fixed-clock";
> +                       clock-frequency = <133333333>;
> +                       #clock-cells = <0>;
> +               };
> +
> +               usdhc_clk_ahb: usdhc_clk_ahb {
> +                       compatible = "fixed-clock";
> +                       clock-frequency = <400000000>;
> +                       #clock-cells = <0>;
> +               };
> +
> +               usdhc_clk_core: usdhc_clk_core {
> +                       compatible = "fixed-clock";
> +                       clock-frequency = <400000000>;
> +                       #clock-cells = <0>;
> +               };

Adding the clock bindings as fixed-clock doesn't describe the hardware.
Using fixed-clock is suitable for quartz crystals and oscillators. Here
we should have the bindings to the clock driver. Are you planning to
submit such driver soon or you will add here more fixed clocks every
time you add a peripheral in the dts?

Cheers.
Radu P.

> +       };
> +
>         soc {
>                 compatible = "simple-bus";
>                 #address-cells = <1>;
> @@ -109,6 +129,18 @@ uart2: serial@402bc000 {
>                         status = "disabled";
>                 };
>  
> +               usdhc0: mmc@402f0000 {
> +                       compatible = "nxp,s32g2-usdhc";
> +                       reg = <0x402f0000 0x1000>;
> +                       interrupts = <GIC_SPI 36
> IRQ_TYPE_LEVEL_HIGH>;
> +                       bus-width = <8>;
> +                       clocks = <&usdhc_clk_module>,
> <&usdhc_clk_ahb>,
> +                                <&usdhc_clk_core>;
> +                       clock-names = "ipg", "ahb", "per";
> +                       no-1-8-v;
> +                       status = "disabled";
> +               };
> +
>                 gic: interrupt-controller@50800000 {
>                         compatible = "arm,gic-v3";
>                         reg = <0x50800000 0x10000>,
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> index 9118d8d2ee01..89428f1883d9 100644
> --- a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> @@ -32,3 +32,7 @@ memory@80000000 {
>  &uart0 {
>         status = "okay";
>  };
> +
> +&usdhc0 {
> +       status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> index e05ee854cdf5..30eae51121de 100644
> --- a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> @@ -38,3 +38,7 @@ &uart0 {
>  &uart1 {
>         status = "okay";
>  };
> +
> +&usdhc0 {
> +       status = "okay";
> +};