[PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Tudor.Ambarus]

From: Cyrille Pitchen <cyrille.pitchen@microchip.com>

This patch configures the QSPI0 controller pin muxing and declares
a jedec,spi-nor memory.

sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
memory which advertises a maximum frequency of 80MHz for Quad IO
Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
[tudor.ambarus@microchip.com:
- drop partitions,
- add spi-rx/tx-bus-width
- change spi-max-frequency to match the 80MHz limit advertised by
  MX25L25673G for Quad IO Fast Read,
- reword commit message and subject.]
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index 518e2b095ccf..171bc82cfbbf 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -108,6 +108,21 @@
 		};
 
 		apb {
+			qspi0: spi@f0020000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_qspi0_default>;
+				/* status = "okay"; */ /* conflict with sdmmc1 */
+
+				flash@0 {
+					compatible = "jedec,spi-nor";
+					reg = <0>;
+					spi-max-frequency = <80000000>;
+					spi-tx-bus-width = <4>;
+					spi-rx-bus-width = <4>;
+					m25p,fast-read;
+				};
+			};
+
 			spi0: spi@f8000000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&pinctrl_spi0_default>;
@@ -485,6 +500,22 @@
 					bias-disable;
 				};
 
+				pinctrl_qspi0_default: qspi0_default {
+					sck_cs {
+						pinmux = <PIN_PA22__QSPI0_SCK>,
+							 <PIN_PA23__QSPI0_CS>;
+						bias-disable;
+					};
+
+					data {
+						pinmux = <PIN_PA24__QSPI0_IO0>,
+							 <PIN_PA25__QSPI0_IO1>,
+							 <PIN_PA26__QSPI0_IO2>,
+							 <PIN_PA27__QSPI0_IO3>;
+						bias-pull-up;
+					};
+				};
+
 				pinctrl_sdmmc0_default: sdmmc0_default {
 					cmd_data {
 						pinmux = <PIN_PA1__SDMMC0_CMD>,
-- 
2.9.4

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Alexandre Belloni]

Hi,

On 10/12/2018 17:15:29+0000, Tudor.Ambarus@microchip.com wrote:
> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> 
> This patch configures the QSPI0 controller pin muxing and declares
> a jedec,spi-nor memory.
> 
> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> memory which advertises a maximum frequency of 80MHz for Quad IO
> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> [tudor.ambarus@microchip.com:
> - drop partitions,
> - add spi-rx/tx-bus-width
> - change spi-max-frequency to match the 80MHz limit advertised by
>   MX25L25673G for Quad IO Fast Read,
> - reword commit message and subject.]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> index 518e2b095ccf..171bc82cfbbf 100644
> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> @@ -108,6 +108,21 @@
>  		};
>  
>  		apb {
> +			qspi0: spi@f0020000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&pinctrl_qspi0_default>;
> +				/* status = "okay"; */ /* conflict with sdmmc1 */

Isn't that conflicting then because I think the default is okay.

> +
> +				flash@0 {
> +					compatible = "jedec,spi-nor";
> +					reg = <0>;
> +					spi-max-frequency = <80000000>;
> +					spi-tx-bus-width = <4>;
> +					spi-rx-bus-width = <4>;
> +					m25p,fast-read;
> +				};
> +			};
> +
>  			spi0: spi@f8000000 {
>  				pinctrl-names = "default";
>  				pinctrl-0 = <&pinctrl_spi0_default>;
> @@ -485,6 +500,22 @@
>  					bias-disable;
>  				};
>  
> +				pinctrl_qspi0_default: qspi0_default {
> +					sck_cs {
> +						pinmux = <PIN_PA22__QSPI0_SCK>,
> +							 <PIN_PA23__QSPI0_CS>;
> +						bias-disable;
> +					};
> +
> +					data {
> +						pinmux = <PIN_PA24__QSPI0_IO0>,
> +							 <PIN_PA25__QSPI0_IO1>,
> +							 <PIN_PA26__QSPI0_IO2>,
> +							 <PIN_PA27__QSPI0_IO3>;
> +						bias-pull-up;
> +					};
> +				};
> +
>  				pinctrl_sdmmc0_default: sdmmc0_default {
>  					cmd_data {
>  						pinmux = <PIN_PA1__SDMMC0_CMD>,
> -- 
> 2.9.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Tudor.Ambarus]

Hi, Alexandre,

On 12/10/2018 11:35 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 10/12/2018 17:15:29+0000, Tudor.Ambarus@microchip.com wrote:
>> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
>>
>> This patch configures the QSPI0 controller pin muxing and declares
>> a jedec,spi-nor memory.
>>
>> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
>> memory which advertises a maximum frequency of 80MHz for Quad IO
>> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
>> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
>> [tudor.ambarus@microchip.com:
>> - drop partitions,
>> - add spi-rx/tx-bus-width
>> - change spi-max-frequency to match the 80MHz limit advertised by
>>   MX25L25673G for Quad IO Fast Read,
>> - reword commit message and subject.]
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
>> index 518e2b095ccf..171bc82cfbbf 100644
>> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
>> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
>> @@ -108,6 +108,21 @@
>>  		};
>>  
>>  		apb {
>> +			qspi0: spi@f0020000 {
>> +				pinctrl-names = "default";
>> +				pinctrl-0 = <&pinctrl_qspi0_default>;
>> +				/* status = "okay"; */ /* conflict with sdmmc1 */
> 
> Isn't that conflicting then because I think the default is okay.
qspi0 is disabled in sama5d2.dtsi.

Thanks,
ta

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Alexandre Belloni]

On 11/12/2018 12:32:40+0000, Tudor.Ambarus@microchip.com wrote:
> Hi, Alexandre,
> 
> On 12/10/2018 11:35 PM, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 10/12/2018 17:15:29+0000, Tudor.Ambarus@microchip.com wrote:
> >> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> >>
> >> This patch configures the QSPI0 controller pin muxing and declares
> >> a jedec,spi-nor memory.
> >>
> >> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> >> memory which advertises a maximum frequency of 80MHz for Quad IO
> >> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> >> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> >>
> >> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> >> [tudor.ambarus@microchip.com:
> >> - drop partitions,
> >> - add spi-rx/tx-bus-width
> >> - change spi-max-frequency to match the 80MHz limit advertised by
> >>   MX25L25673G for Quad IO Fast Read,
> >> - reword commit message and subject.]
> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >> ---
> >>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
> >>  1 file changed, 31 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> >> index 518e2b095ccf..171bc82cfbbf 100644
> >> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> >> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> >> @@ -108,6 +108,21 @@
> >>  		};
> >>  
> >>  		apb {
> >> +			qspi0: spi@f0020000 {
> >> +				pinctrl-names = "default";
> >> +				pinctrl-0 = <&pinctrl_qspi0_default>;
> >> +				/* status = "okay"; */ /* conflict with sdmmc1 */
> > 
> > Isn't that conflicting then because I think the default is okay.
> qspi0 is disabled in sama5d2.dtsi.
> 

Ok, then maybe that comment is not necessary at all.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Boris Brezillon]

On Mon, 10 Dec 2018 17:15:29 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> 
> This patch configures the QSPI0 controller pin muxing and declares
> a jedec,spi-nor memory.
> 
> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> memory which advertises a maximum frequency of 80MHz for Quad IO
> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> [tudor.ambarus@microchip.com:
> - drop partitions,
> - add spi-rx/tx-bus-width
> - change spi-max-frequency to match the 80MHz limit advertised by
>   MX25L25673G for Quad IO Fast Read,
> - reword commit message and subject.]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> index 518e2b095ccf..171bc82cfbbf 100644
> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> @@ -108,6 +108,21 @@
>  		};
>  
>  		apb {
> +			qspi0: spi@f0020000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&pinctrl_qspi0_default>;
> +				/* status = "okay"; */ /* conflict with sdmmc1 */
> +
> +				flash@0 {
> +					compatible = "jedec,spi-nor";
> +					reg = <0>;
> +					spi-max-frequency = <80000000>;
> +					spi-tx-bus-width = <4>;
> +					spi-rx-bus-width = <4>;
> +					m25p,fast-read;
> +				};

I'm a bit lost. What's the point of defining this if the QSPI
controller is not enabled?

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Ludovic Desroches]

On Tue, Dec 11, 2018 at 03:40:33PM +0100, Boris Brezillon wrote:
> On Mon, 10 Dec 2018 17:15:29 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
> > From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> > 
> > This patch configures the QSPI0 controller pin muxing and declares
> > a jedec,spi-nor memory.
> > 
> > sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> > memory which advertises a maximum frequency of 80MHz for Quad IO
> > Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> > the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> > 
> > Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> > [tudor.ambarus@microchip.com:
> > - drop partitions,
> > - add spi-rx/tx-bus-width
> > - change spi-max-frequency to match the 80MHz limit advertised by
> >   MX25L25673G for Quad IO Fast Read,
> > - reword commit message and subject.]
> > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> > ---
> >  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > index 518e2b095ccf..171bc82cfbbf 100644
> > --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > @@ -108,6 +108,21 @@
> >  		};
> >  
> >  		apb {
> > +			qspi0: spi@f0020000 {
> > +				pinctrl-names = "default";
> > +				pinctrl-0 = <&pinctrl_qspi0_default>;
> > +				/* status = "okay"; */ /* conflict with sdmmc1 */
> > +
> > +				flash@0 {
> > +					compatible = "jedec,spi-nor";
> > +					reg = <0>;
> > +					spi-max-frequency = <80000000>;
> > +					spi-tx-bus-width = <4>;
> > +					spi-rx-bus-width = <4>;
> > +					m25p,fast-read;
> > +				};
> 
> I'm a bit lost. What's the point of defining this if the QSPI
> controller is not enabled?

It's a way to avoid customer struggling with the device tree. If he
doesn't care about sdmmc1, he can easily enable the qpsi controller and
get access to the memory.

Regards

Ludovic

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Tudor.Ambarus]

Hi, Boris,

On 12/11/2018 04:40 PM, Boris Brezillon wrote:
> On Mon, 10 Dec 2018 17:15:29 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
>>
>> This patch configures the QSPI0 controller pin muxing and declares
>> a jedec,spi-nor memory.
>>
>> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
>> memory which advertises a maximum frequency of 80MHz for Quad IO
>> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
>> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.

s/drver/driver

>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
>> [tudor.ambarus@microchip.com:
>> - drop partitions,
>> - add spi-rx/tx-bus-width
>> - change spi-max-frequency to match the 80MHz limit advertised by
>>   MX25L25673G for Quad IO Fast Read,
>> - reword commit message and subject.]
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
>> index 518e2b095ccf..171bc82cfbbf 100644
>> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
>> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
>> @@ -108,6 +108,21 @@
>>  		};
>>  
>>  		apb {
>> +			qspi0: spi@f0020000 {
>> +				pinctrl-names = "default";
>> +				pinctrl-0 = <&pinctrl_qspi0_default>;
>> +				/* status = "okay"; */ /* conflict with sdmmc1 */
>> +
>> +				flash@0 {
>> +					compatible = "jedec,spi-nor";
>> +					reg = <0>;
>> +					spi-max-frequency = <80000000>;
>> +					spi-tx-bus-width = <4>;
>> +					spi-rx-bus-width = <4>;
>> +					m25p,fast-read;
>> +				};
> 
> I'm a bit lost. What's the point of defining this if the QSPI
> controller is not enabled?

You can enable qspi if you disable sdmmc1. I thought of having all described
together and choose one or another depending on needs.

ta

Re: [PATCH] ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes

[Ludovic Desroches]

On Tue, Dec 11, 2018 at 03:35:45PM +0100, Alexandre Belloni wrote:
> On 11/12/2018 12:32:40+0000, Tudor.Ambarus@microchip.com wrote:
> > Hi, Alexandre,
> > 
> > On 12/10/2018 11:35 PM, Alexandre Belloni wrote:
> > > Hi,
> > > 
> > > On 10/12/2018 17:15:29+0000, Tudor.Ambarus@microchip.com wrote:
> > >> From: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> > >>
> > >> This patch configures the QSPI0 controller pin muxing and declares
> > >> a jedec,spi-nor memory.
> > >>
> > >> sama5d2 Xplained RevB and RevC use the Macronix MX25L25673G flash
> > >> memory which advertises a maximum frequency of 80MHz for Quad IO
> > >> Fast Read. Set the spi-max-frequency to 80MHz knowing that actually
> > >> the QSPI drver will set the SPI bus clock to 166MHz / 3 = 55.3MHz.
> > >>
> > >> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@microchip.com>
> > >> [tudor.ambarus@microchip.com:
> > >> - drop partitions,
> > >> - add spi-rx/tx-bus-width
> > >> - change spi-max-frequency to match the 80MHz limit advertised by
> > >>   MX25L25673G for Quad IO Fast Read,
> > >> - reword commit message and subject.]
> > >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> > >> ---
> > >>  arch/arm/boot/dts/at91-sama5d2_xplained.dts | 31 +++++++++++++++++++++++++++++
> > >>  1 file changed, 31 insertions(+)
> > >>
> > >> diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> index 518e2b095ccf..171bc82cfbbf 100644
> > >> --- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> +++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
> > >> @@ -108,6 +108,21 @@
> > >>  		};
> > >>  
> > >>  		apb {
> > >> +			qspi0: spi@f0020000 {
> > >> +				pinctrl-names = "default";
> > >> +				pinctrl-0 = <&pinctrl_qspi0_default>;
> > >> +				/* status = "okay"; */ /* conflict with sdmmc1 */
> > > 
> > > Isn't that conflicting then because I think the default is okay.
> > qspi0 is disabled in sama5d2.dtsi.
> > 
> 
> Ok, then maybe that comment is not necessary at all.

Usually we do it the other way around:
status = "disabled"; /* conflict with ... */

Regards

Ludovic