RE: [PATCH] arm64: dts: renesas: r9a07g0{4,5}4: Add support for enabling MTU3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH] arm64: dts: renesas: r9a07g0{4,5}4: Add support for
> enabling MTU3
> 
> Hi Biju,
> 
> On Mon, Jul 3, 2023 at 3:29 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > Add support for MTU3 macro to enable MTU3 node on RZ/{G2,V2}L SMARC
> EVK.
> >
> > The MTU3a PWM pins are muxed with spi1 pins and counter external input
> > phase clock pins are muxed with scif2 pins. Disable these IPs when
> > MTU3 macro is enabled.
> >
> > Apart from this, the counter Z phase clock signal is muxed with the
> > SDHI1 cd signal. So disable SDHI1 IP, when the counter Z phase signal
> > is enabled.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts
> > @@ -12,7 +12,43 @@
> >  #include "rz-smarc-common.dtsi"
> >  #include "rzg2l-smarc.dtsi"
> >
> > +#define MTU3   0
> 
> Should this be called PMOD_MTU3 instead? The signals are provided on the
> PMOD0 and PMOD1 connectors.
> Perhaps add some checking to make sure PMOD1_SER0 and PMOD_MTU3 are
> mutually exclusive?

OK will do.

> 
> > +#define MTU3_COUNTER_Z_PHASE_SIGNAL    0
> > +#if (!MTU3 && MTU3_COUNTER_Z_PHASE_SIGNAL) #error "Cannot set 1 to
> > +MTU3_COUNTER_Z_PHASE_SIGNAL as MTU3=0"
> > +#endif
> > +
> 
> Please move these up, before the various includes, like is done in
> arch/arm64/boot/dts/renesas/r9a07g044c2-smarc.dts.

Ok.

> 
> >  / {
> >         model = "Renesas SMARC EVK based on r9a07g044l2";
> >         compatible = "renesas,smarc-evk", "renesas,r9a07g044l2",
> > "renesas,r9a07g044";  };
> > +
> > +#if MTU3
> > +&mtu3 {
> > +       pinctrl-0 = <&mtu3_pins>;
> > +       pinctrl-names = "default";
> > +
> > +       status = "okay";
> > +};
> > +
> > +&scif2 {
> > +       status = "disabled";
> > +};
> > +
> > +#if MTU3_COUNTER_Z_PHASE_SIGNAL
> > +/* SDHI cd pin is used for counter Z phase signal */
> 
> And this pin is not available on any other extension connector but the
> microSD connector.

Yes, that is correct.

> 
> > +&mtu3_pins {
> > +       mtu3-zphase-clk {
> > +               pinmux = <RZG2L_PORT_PINMUX(19, 0, 3)>; /* MTIOC1A */
> > +       };
> > +};
> 
> With the #defines moved up, mtu3-zphase-clk can be moved to mtu3_pins in
> rzg2l-smarc-pinfunction.dtsi.

Z-phase support is added only for cascade counter(MTU1 + MTU2)

I thought by making this as optional, SDHI + standalone MTU1 or MTU2
can still work. That is the reason it is moved here.

If we move "mtu3-zphase-clk" to  mtu3_pins in rzg2l-smarc-pinfunction.dtsi
Either 

we need to make MTU3 mutually exclusive with SDHI

Or

Guard "mtu3-zphase-clk" with "MTU3_COUNTER_Z_PHASE_SIGNAL" macro in
rzg2l-smarc-pinfunction.dtsi.

Which one I need to select??

> 
> > +
> > +&sdhi1 {
> > +       status = "disabled";
> > +};
> > +#endif /* MTU3_COUNTER_Z_PHASE_SIGNAL */
> 
> BTW, how does the driver know it can use the counter Z phase clock
> signal?  I understand this can be either an input or output signal?

It is an input signal and is supported only for the cascade(MTU1 + MTU2) operation. When we supply z-phase signal(By inserting SD card or applying a voltage to cd pin on the sd connector), counter value gets cleared.

> 
> > +
> > +&spi1 {
> > +       status = "disabled";
> > +};
> > +#endif /* MTU3 */
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g054l2-smarc.dts
> > b/arch/arm64/boot/dts/renesas/r9a07g054l2-smarc.dts
> > index 3d01a4cf0fbe..4186bfe739fa 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g054l2-smarc.dts
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g054l2-smarc.dts
> 
> The changes to r9a07g054l2-smarc.dts are identical to those to
> r9a07g044l2-smarc.dts.  So I think they should be unified and moved to
> rzg2l-smarc-som.dtsi and rzg2l-smarc.dtsi.
> The various #include "rzg2l-smarc-pinfunction.dtsi" probably need to be
> moved down for that to work, though.

OK, will do this change.

Cheers,
Biju

> 
> rzg2l-smarc-som.dtsi and rzg2l-smarc.dtsi already have similar handling
> for EMMC, SDHI, and PMOD1_SER0.
> 
> > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-pinfunction.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-pinfunction.dtsi
> > @@ -53,6 +53,20 @@ i2c3_pins: i2c3 {
> >                          <RZG2L_PORT_PINMUX(18, 1, 3)>; /* SCL */
> >         };
> >
> > +       mtu3_pins: mtu3 {
> > +               mtu3-ext-clk-input-pin {
> > +                       pinmux = <RZG2L_PORT_PINMUX(48, 0, 4)>, /*
> MTCLKA */
> > +                                <RZG2L_PORT_PINMUX(48, 1, 4)>; /*
> MTCLKB */
> > +               };
> > +
> > +               mtu3-pwm {
> > +                       pinmux = <RZG2L_PORT_PINMUX(44, 0, 4)>, /*
> MTIOC3A */
> > +                                <RZG2L_PORT_PINMUX(44, 1, 4)>, /*
> MTIOC3B */
> > +                                <RZG2L_PORT_PINMUX(44, 2, 4)>, /*
> MTIOC3C */
> > +                                <RZG2L_PORT_PINMUX(44, 3, 4)>; /*
> MTIOC3D */
> > +               };
> > +       };
> > +
> >         scif0_pins: scif0 {
> >                 pinmux = <RZG2L_PORT_PINMUX(38, 0, 1)>, /* TxD */
> >                          <RZG2L_PORT_PINMUX(38, 1, 1)>; /* RxD */
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux