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
