diff mbox series

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

Message ID 20181210171511.21002-1-tudor.ambarus@microchip.com
State Not Applicable
Headers show
Series ARM: dts: at91: sama5d2 Xplained: add QSPI0 + SPI NOR memory nodes | expand

Commit Message

Tudor Ambarus Dec. 10, 2018, 5:15 p.m. UTC 
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(+)

Comments

Alexandre Belloni Dec. 10, 2018, 9:35 p.m. UTC | #1 
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
>
Tudor Ambarus Dec. 11, 2018, 12:32 p.m. UTC | #2 
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
Alexandre Belloni Dec. 11, 2018, 2:35 p.m. UTC | #3 
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.
Boris Brezillon Dec. 11, 2018, 2:40 p.m. UTC | #4 
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?
Ludovic Desroches Dec. 11, 2018, 2:48 p.m. UTC | #5 
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
Tudor Ambarus Dec. 11, 2018, 2:49 p.m. UTC | #6 
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
Ludovic Desroches Dec. 11, 2018, 2:50 p.m. UTC | #7 
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
diff mbox series

Patch

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>,