Re: [PATCH 3/3] arm64: dts: renesas: rzt2h-n2h-evk: Enable xSPI nodes

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Wed, May 27, 2026 at 11:43 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Tue, 5 May 2026 at 14:02, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Enable the xSPI0 and xSPI1 controllers on the RZ/T2H N2H EVK board.
> >
> > The xSPI0 controller is connected to an MX25LW51245 octal flash device.
> > Although the hardware supports octal mode, configure the bus width to
> > 1-bit (x1) mode. Enabling octal mode causes the BootROM to fail loading
> > the first-stage bootloader following a Watchdog Timer (WDT) reset.
> >
> > The xSPI1 controller is connected to an AT25SF128A flash device.
> > Configure this interface for 4-bit (x4) mode to utilize the available
> > data lines.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/rzt2h-n2h-evk-common.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzt2h-n2h-evk-common.dtsi
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2025 Renesas Electronics Corp.
> >   */
> >
> > +#include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h>
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/leds/common.h>
> >  #include <dt-bindings/net/mscc-phy-vsc8531.h>
> > @@ -20,6 +21,8 @@ aliases {
> >                 mmc0 = &sdhi0;
> >                 mmc1 = &sdhi1;
> >                 serial0 = &sci0;
> > +               spi0 = &xspi0;
> > +               spi1 = &xspi1;
> >         };
> >
> >         chosen {
> > @@ -331,6 +334,59 @@ ctrl-pins {
> >                                  <RZT2H_PORT_PINMUX(17, 4, 0x29)>; /* SD1_CD */
> >                 };
> >         };
> > +
> > +       xspi0_pins: xspi0-group {
> > +               clk-pins {
> > +                       pinmux = <RZT2H_PORT_PINMUX(5, 1, 0x1c)>, /* XSPI0_CKP */
> > +                                <RZT2H_PORT_PINMUX(5, 2, 0x1c)>; /* XSPI0_CKN */
>
> XSPI0_CKN is not connected to the OSPI FLASH (zero-ohm "DNF" resistor!),
> only to the HyperRAM.
>
Agreed, I will drop it.

> > +                       input-schmitt-disable;
> > +                       slew-rate = <1>;
> > +                       drive-strength-microamp = <9000>;
> > +               };
> > +
> > +               data-pins {
> > +                       pinmux = <RZT2H_PORT_PINMUX(5, 6, 0x1c)>, /* XSPI0_IO0 */
> > +                                <RZT2H_PORT_PINMUX(5, 7, 0x1c)>, /* XSPI0_IO1 */
> > +                                <RZT2H_PORT_PINMUX(6, 0, 0x1c)>, /* XSPI0_IO2 */
> > +                                <RZT2H_PORT_PINMUX(6, 1, 0x1c)>, /* XSPI0_IO3 */
> > +                                <RZT2H_PORT_PINMUX(6, 2, 0x1c)>, /* XSPI0_IO4 */
> > +                                <RZT2H_PORT_PINMUX(6, 3, 0x1c)>, /* XSPI0_IO5 */
> > +                                <RZT2H_PORT_PINMUX(6, 4, 0x1c)>, /* XSPI0_IO6 */
> > +                                <RZT2H_PORT_PINMUX(6, 5, 0x1c)>; /* XSPI0_IO7 */
> > +                       input-schmitt-disable;
> > +                       slew-rate = <1>;
> > +                       drive-strength-microamp = <9000>;
> > +               };
> > +
> > +               ctrl-pins {
> > +                       pinmux = <RZT2H_PORT_PINMUX(5, 3, 0x1c)>, /* XSPI0_CS0 */
> > +                                <RZT2H_PORT_PINMUX(6, 6, 0x1c)>, /* XSPI0_RESET0 */
> > +                                <RZT2H_PORT_PINMUX(5, 5, 0x1c)>; /* XSPI0_DS */
> > +                       input-schmitt-disable;
> > +                       slew-rate = <1>;
> > +                       drive-strength-microamp = <9000>;
> > +               };
>
> As the three subnodes define the same parameters, you can do without
> the subnodes, and keep all pins together?
>
> Don't you also need P7_5 for XSPI0_ES, and SW5-6 closed?
>
Agreed, XSPI0_ES is needed (5: OFF, 6: ON). Since XSPI0_ES has
different DRCTL settings I'll keep it as a group but with only two sub
nodes.

> > +       };
> > +
> > +       xspi1_pins: xspi1-group {
> > +               ctrl-pins {
> > +                       pinmux = <RZT2H_PORT_PINMUX(1, 0, 0x1c)>, /* XSPI1_CKP */
> > +                                <RZT2H_PORT_PINMUX(1, 1, 0x1c)>; /* XSPI1_CS0 */
> > +                       input-schmitt-enable;
> > +                       slew-rate = <1>;
> > +                       drive-strength-microamp = <9000>;
> > +               };
> > +
> > +               data-pins {
> > +                       pinmux = <RZT2H_PORT_PINMUX(1, 4, 0x1c)>, /* XSPI1_IO0 */
> > +                                <RZT2H_PORT_PINMUX(1, 5, 0x1c)>, /* XSPI1_IO1 */
> > +                                <RZT2H_PORT_PINMUX(1, 6, 0x1c)>, /* XSPI1_IO2 */
> > +                                <RZT2H_PORT_PINMUX(1, 7, 0x1c)>; /* XSPI1_IO3 */
> > +                       input-schmitt-enable;
> > +                       slew-rate = <1>;
> > +                       drive-strength-microamp = <9000>;
> > +               };
>
> As the two subnodes define the same parameters, you can do without
> the subnodes, and keep all pins together?
>
Ok, I will drop the subnodes.

> And SW1-6 must be closed?
>
SW1[6] needs to be ON, I will add comments for it.

> > +       };
> >  };
> >
> >  &sci0 {
> > @@ -395,3 +451,82 @@ &wdt2 {
> >         timeout-sec = <60>;
> >  };
> >
> > +&xspi0 {
> > +       pinctrl-0 = <&xspi0_pins>;
> > +       pinctrl-names = "default";
> > +       status = "okay";
> > +
> > +       assigned-clocks = <&cpg CPG_CORE R9A09G077_XSPI_CLK0>;
> > +       assigned-clock-rates = <50000000>;
> > +
> > +       flash@0 {
> > +               compatible = "jedec,spi-nor";
> > +               reg = <0>;
> > +               m25p,fast-read;
> > +               spi-tx-bus-width = <1>;
> > +               spi-rx-bus-width = <1>;
> > +               vcc-supply = <&reg_3p3v>;
> > +
> > +               partitions {
> > +                       compatible = "fixed-partitions";
> > +                       #address-cells = <1>;
> > +                       #size-cells = <1>;
> > +
> > +                       partition@0 {
> > +                               label = "bl2-0";
> > +                               reg = <0x00000000 0x00060000>;
> > +                               read-only;
> > +                       };
> > +
> > +                       partition@1fb000 {
>
> 60000?
>
Agreed.

> > +                               label = "fip-0";
> > +                               reg = <0x00060000 0x007a0000>;
> > +                               read-only;
> > +                       };
> > +
> > +                       partition@300000 {
>
> 800000?
>
Agreed.

> > +                               label = "user-0";
> > +                               reg = <0x800000 0x003800000>;
> > +                       };
>
> Given this is a 64 MiB part, I assume the reg entries are correct,
> and the unit-addresses are not?
>
Yes, thats right.

I will address the comments and send a v2 for just this single patch.

Cheers,
Prabhakar