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

[Chester Lin]

Hi Radu,

On Thu, Oct 21, 2021 at 04:32:52PM +0300, Radu Nicolae Pirea (NXP OSS) wrote:
> 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?
> 

Yes, I'm planning to add real clock bindings but not just fixed rates I observed
from EVB and RDB2 boards. Since the upstream support is based on the psci
method and TF-A, I noticed that we can use SCMI clocks [+smc transport driver,
e.g. "arm,scmi-smc"] as clock inputs for most IPs on S32G, and AFAICS the
downstream TF-A[1] has infrastructure to support SCMI clock protocl
[protocol-id: 0x14, arm,smc-id: 0xc20000fe].

However it will take much time to verify and upstream all SCMI-related stuff
such as clock dt-bindings, so I am thinking of using fixed-clock as the first
step to have initial support before we can have all SCMI bindings get accepted
by upstream.

Please feel free to let me know if any suggestions.

Regards,
Chester

[1] https://source.codeaurora.org/external/autobsps32/arm-trusted-firmware/tree/plat/nxp/s32g/s32g_svc.c?h=release/bsp30.0-2.3

> 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";
> > +};
> 
>