| Message ID | 1543319795-48325-20-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | Add more support to RZ/G1N | expand |
Hi Biju, On Tue, Nov 27, 2018 at 1:05 PM Biju Das <biju.das@bp.renesas.com> wrote: > Add support for the SPI NOR device used to boot up the system > to the iWave RZ/G1N Qseven System On Module DT. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> Thanks for your patch! > --- a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > +++ b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > @@ -53,6 +58,27 @@ > status = "okay"; > }; > > +&qspi { > + pinctrl-0 = <&qspi_pins>; > + pinctrl-names = "default"; > + > + status = "okay"; > + > + /* WARNING - This device contains the bootloader. Handle with care. */ > + flash: flash@0 { > + #address-cells = <1>; > + #size-cells = <1>; > + compatible = "sst,sst25vf016b", "jedec,spi-nor"; According to the schematics, this is an ISSI IS25LP016D? ISSI was acquired by GigaDevice, according to Wikipedia. While SST is now MicroChip. > + reg = <0>; > + spi-max-frequency = <50000000>; > + spi-tx-bus-width = <1>; > + spi-rx-bus-width = <1>; <1> is the default, but it's indeed good to make this explicit, as this is a QSPI device with 2 unwired data pins. However, as the device seems to support dual transfers, and dual mode uses the standard MOSI/MISO pins, you should use <2> for both. The RSPI driver supports this. The same applies to the RZ/G1M version. > + m25p,fast-read; > + spi-cpol; > + spi-cpha; > + }; > +}; Apart from that: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH 19/22] ARM: dts: r8a7744-iwg20m: Add SPI NOR support > > Hi Biju, > > On Tue, Nov 27, 2018 at 1:05 PM Biju Das <biju.das@bp.renesas.com> wrote: > > Add support for the SPI NOR device used to boot up the system to the > > iWave RZ/G1N Qseven System On Module DT. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > Thanks for your patch! > > > --- a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > +++ b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > @@ -53,6 +58,27 @@ > > status = "okay"; > > }; > > > > +&qspi { > > + pinctrl-0 = <&qspi_pins>; > > + pinctrl-names = "default"; > > + > > + status = "okay"; > > + > > + /* WARNING - This device contains the bootloader. Handle with care. > */ > > + flash: flash@0 { > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "sst,sst25vf016b", "jedec,spi-nor"; > > According to the schematics, this is an ISSI IS25LP016D? > ISSI was acquired by GigaDevice, according to Wikipedia. > While SST is now MicroChip. As per the schematic and BoM, it is. IC FLASH 16MBIT 50MHZ 8SOIC SST25VF016B-50-4I-S2AF Microchip Technology 1 U1 I agree for RZ/G1C, it is ISSI IS25LP016D. > > > + reg = <0>; > > + spi-max-frequency = <50000000>; > > + spi-tx-bus-width = <1>; > > + spi-rx-bus-width = <1>; > > <1> is the default, but it's indeed good to make this explicit, as this is a QSPI > device with 2 unwired data pins. > However, as the device seems to support dual transfers, and dual mode uses > the standard MOSI/MISO pins, you should use <2> for both. > The RSPI driver supports this. > > The same applies to the RZ/G1M version. SST25VF016B this doesn't support dual mode. Please let me know are you ok with this findings. > > + m25p,fast-read; > > + spi-cpol; > > + spi-cpha; > > + }; > > +}; > > Apart from that: > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Regards, Biju Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Hi Biju, On Fri, Nov 30, 2018 at 11:34 AM Biju Das <biju.das@bp.renesas.com> wrote: > > Subject: Re: [PATCH 19/22] ARM: dts: r8a7744-iwg20m: Add SPI NOR support > > On Tue, Nov 27, 2018 at 1:05 PM Biju Das <biju.das@bp.renesas.com> wrote: > > > Add support for the SPI NOR device used to boot up the system to the > > > iWave RZ/G1N Qseven System On Module DT. > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > +++ b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > > > @@ -53,6 +58,27 @@ > > > status = "okay"; > > > }; > > > > > > +&qspi { > > > + pinctrl-0 = <&qspi_pins>; > > > + pinctrl-names = "default"; > > > + > > > + status = "okay"; > > > + > > > + /* WARNING - This device contains the bootloader. Handle with care. > > */ > > > + flash: flash@0 { > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + compatible = "sst,sst25vf016b", "jedec,spi-nor"; > > > > According to the schematics, this is an ISSI IS25LP016D? > > ISSI was acquired by GigaDevice, according to Wikipedia. > > While SST is now MicroChip. > > As per the schematic and BoM, it is. > IC FLASH 16MBIT 50MHZ 8SOIC SST25VF016B-50-4I-S2AF Microchip Technology 1 U1 Oh, this seems to differ for different revisions of the schematics. R5.1 has the ISSI part, R3.4 has the SST part. Due to "jedec,spi-nor", it will auto-detect, but IIRC, the driver will warn if the compatible doesn't match the detected part, which thus may happen for some boards. > > > + reg = <0>; > > > + spi-max-frequency = <50000000>; > > > + spi-tx-bus-width = <1>; > > > + spi-rx-bus-width = <1>; > > > > <1> is the default, but it's indeed good to make this explicit, as this is a QSPI > > device with 2 unwired data pins. > > However, as the device seems to support dual transfers, and dual mode uses > > the standard MOSI/MISO pins, you should use <2> for both. > > The RSPI driver supports this. > > > > The same applies to the RZ/G1M version. > > SST25VF016B this doesn't support dual mode. OK. So some boards have a dual-capable part, others don't. I'm not 100% sure, but I think writing <2> will still work, as the driver won't use dual mode on the SST25VF016B part, due to lack of SPI_NOR_DUAL_READ in the entry in the spi-nor driver. Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH 19/22] ARM: dts: r8a7744-iwg20m: Add SPI NOR support > > Hi Biju, > > On Fri, Nov 30, 2018 at 11:34 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > Subject: Re: [PATCH 19/22] ARM: dts: r8a7744-iwg20m: Add SPI NOR > > > support On Tue, Nov 27, 2018 at 1:05 PM Biju Das > <biju.das@bp.renesas.com> wrote: > > > > Add support for the SPI NOR device used to boot up the system to > > > > the iWave RZ/G1N Qseven System On Module DT. > > > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > > +++ b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > > > > > @@ -53,6 +58,27 @@ > > > > status = "okay"; > > > > }; > > > > > > > > +&qspi { > > > > + pinctrl-0 = <&qspi_pins>; > > > > + pinctrl-names = "default"; > > > > + > > > > + status = "okay"; > > > > + > > > > + /* WARNING - This device contains the bootloader. Handle with > care. > > > */ > > > > + flash: flash@0 { > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > + compatible = "sst,sst25vf016b", "jedec,spi-nor"; > > > > > > According to the schematics, this is an ISSI IS25LP016D? > > > ISSI was acquired by GigaDevice, according to Wikipedia. > > > While SST is now MicroChip. > > > > As per the schematic and BoM, it is. > > IC FLASH 16MBIT 50MHZ 8SOIC SST25VF016B-50-4I-S2AF Microchip > > Technology 1 U1 > > Oh, this seems to differ for different revisions of the schematics. > R5.1 has the ISSI part, R3.4 has the SST part. > > Due to "jedec,spi-nor", it will auto-detect, but IIRC, the driver will warn if the > compatible doesn't match the detected part, which thus may happen for > some boards. OK. Will remove "sst" part for the compatible string. > > > > + reg = <0>; > > > > + spi-max-frequency = <50000000>; > > > > + spi-tx-bus-width = <1>; > > > > + spi-rx-bus-width = <1>; > > > > > > <1> is the default, but it's indeed good to make this explicit, as > > > this is a QSPI device with 2 unwired data pins. > > > However, as the device seems to support dual transfers, and dual > > > mode uses the standard MOSI/MISO pins, you should use <2> for both. > > > The RSPI driver supports this. > > > > > > The same applies to the RZ/G1M version. > > > > SST25VF016B this doesn't support dual mode. > > OK. > > So some boards have a dual-capable part, others don't. > > I'm not 100% sure, but I think writing <2> will still work, as the driver won't > use dual mode on the SST25VF016B part, due to lack of > SPI_NOR_DUAL_READ in the entry in the spi-nor driver. OK. Will check this. Regards, Biju Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
On Fri, Nov 30, 2018 at 11:02:08AM +0000, Biju Das wrote: > Hi Geert, > > Thanks for the feedback. > > > Subject: Re: [PATCH 19/22] ARM: dts: r8a7744-iwg20m: Add SPI NOR support > > > > Hi Biju, > > > > On Fri, Nov 30, 2018 at 11:34 AM Biju Das <biju.das@bp.renesas.com> wrote: > > > > Subject: Re: [PATCH 19/22] ARM: dts: r8a7744-iwg20m: Add SPI NOR > > > > support On Tue, Nov 27, 2018 at 1:05 PM Biju Das > > <biju.das@bp.renesas.com> wrote: > > > > > Add support for the SPI NOR device used to boot up the system to > > > > > the iWave RZ/G1N Qseven System On Module DT. > > > > > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > > > > > > Thanks for your patch! > > > > > > > > > --- a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > > > +++ b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi > > > > > > > > > @@ -53,6 +58,27 @@ > > > > > status = "okay"; > > > > > }; > > > > > > > > > > +&qspi { > > > > > + pinctrl-0 = <&qspi_pins>; > > > > > + pinctrl-names = "default"; > > > > > + > > > > > + status = "okay"; > > > > > + > > > > > + /* WARNING - This device contains the bootloader. Handle with > > care. > > > > */ > > > > > + flash: flash@0 { > > > > > + #address-cells = <1>; > > > > > + #size-cells = <1>; > > > > > + compatible = "sst,sst25vf016b", "jedec,spi-nor"; > > > > > > > > According to the schematics, this is an ISSI IS25LP016D? > > > > ISSI was acquired by GigaDevice, according to Wikipedia. > > > > While SST is now MicroChip. > > > > > > As per the schematic and BoM, it is. > > > IC FLASH 16MBIT 50MHZ 8SOIC SST25VF016B-50-4I-S2AF Microchip > > > Technology 1 U1 > > > > Oh, this seems to differ for different revisions of the schematics. > > R5.1 has the ISSI part, R3.4 has the SST part. > > > > Due to "jedec,spi-nor", it will auto-detect, but IIRC, the driver will warn if the > > compatible doesn't match the detected part, which thus may happen for > > some boards. > > OK. Will remove "sst" part for the compatible string. I have marked this patch as "Changes Requested" and am awaiting v2. > > > > > > + reg = <0>; > > > > > + spi-max-frequency = <50000000>; > > > > > + spi-tx-bus-width = <1>; > > > > > + spi-rx-bus-width = <1>; > > > > > > > > <1> is the default, but it's indeed good to make this explicit, as > > > > this is a QSPI device with 2 unwired data pins. > > > > However, as the device seems to support dual transfers, and dual > > > > mode uses the standard MOSI/MISO pins, you should use <2> for both. > > > > The RSPI driver supports this. > > > > > > > > The same applies to the RZ/G1M version. > > > > > > SST25VF016B this doesn't support dual mode. > > > > OK. > > > > So some boards have a dual-capable part, others don't. > > > > I'm not 100% sure, but I think writing <2> will still work, as the driver won't > > use dual mode on the SST25VF016B part, due to lack of > > SPI_NOR_DUAL_READ in the entry in the spi-nor driver. > > OK. Will check this. > > Regards, > Biju > > > > Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
diff --git a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi index 503583e..664a827 100644 --- a/arch/arm/boot/dts/r8a7744-iwg20m.dtsi +++ b/arch/arm/boot/dts/r8a7744-iwg20m.dtsi @@ -36,6 +36,11 @@ function = "mmc"; }; + qspi_pins: qspi { + groups = "qspi_ctrl", "qspi_data2"; + function = "qspi"; + }; + sdhi0_pins: sd0 { groups = "sdhi0_data4", "sdhi0_ctrl"; function = "sdhi0"; @@ -53,6 +58,27 @@ status = "okay"; }; +&qspi { + pinctrl-0 = <&qspi_pins>; + pinctrl-names = "default"; + + status = "okay"; + + /* WARNING - This device contains the bootloader. Handle with care. */ + flash: flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "sst,sst25vf016b", "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <50000000>; + spi-tx-bus-width = <1>; + spi-rx-bus-width = <1>; + m25p,fast-read; + spi-cpol; + spi-cpha; + }; +}; + &sdhi0 { pinctrl-0 = <&sdhi0_pins>; pinctrl-names = "default";
Add support for the SPI NOR device used to boot up the system to the iWave RZ/G1N Qseven System On Module DT. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- arch/arm/boot/dts/r8a7744-iwg20m.dtsi | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)