[PATCH v2 04/12] dt-bindings: spi: Move the bindings for the FSL QSPI driver

[PATCH v2 04/12] dt-bindings: spi: Move the bindings for the FSL QSPI driver

[Frieder Schrempf]

Move the documentation of the old SPI NOR driver to the place of the new
SPI memory interface based driver.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
Changes in v2:
==============
* Split the moving and editing of the dt-bindings in two patches

 .../devicetree/bindings/mtd/fsl-quadspi.txt     | 65 --------------------
 .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 65 ++++++++++++++++++++
 2 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt b/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
deleted file mode 100644
index 483e9cf..0000000
--- a/Documentation/devicetree/bindings/mtd/fsl-quadspi.txt
+++ /dev/null
@@ -1,65 +0,0 @@
-* Freescale Quad Serial Peripheral Interface(QuadSPI)
-
-Required properties:
-  - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
-		 "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
-		 "fsl,ls1021a-qspi"
-		 or
-		 "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
-		 "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
-  - reg : the first contains the register location and length,
-          the second contains the memory mapping address and length
-  - reg-names: Should contain the reg names "QuadSPI" and "QuadSPI-memory"
-  - interrupts : Should contain the interrupt for the device
-  - clocks : The clocks needed by the QuadSPI controller
-  - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
-
-Optional properties:
-  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
-                              Each bus can be connected with two NOR flashes.
-			      Most of the time, each bus only has one NOR flash
-			      connected, this is the default case.
-			      But if there are two NOR flashes connected to the
-			      bus, you should enable this property.
-			      (Please check the board's schematic.)
-  - big-endian : That means the IP register is big endian
-
-Example:
-
-qspi0: quadspi@40044000 {
-	compatible = "fsl,vf610-qspi";
-	reg = <0x40044000 0x1000>, <0x20000000 0x10000000>;
-	reg-names = "QuadSPI", "QuadSPI-memory";
-	interrupts = <0 24 IRQ_TYPE_LEVEL_HIGH>;
-	clocks = <&clks VF610_CLK_QSPI0_EN>,
-		<&clks VF610_CLK_QSPI0>;
-	clock-names = "qspi_en", "qspi";
-
-	flash0: s25fl128s@0 {
-		....
-	};
-};
-
-Example showing the usage of two SPI NOR devices:
-
-&qspi2 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_qspi2>;
-	status = "okay";
-
-	flash0: n25q256a@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
-		compatible = "micron,n25q256a", "jedec,spi-nor";
-		spi-max-frequency = <29000000>;
-		reg = <0>;
-	};
-
-	flash1: n25q256a@1 {
-		#address-cells = <1>;
-		#size-cells = <1>;
-		compatible = "micron,n25q256a", "jedec,spi-nor";
-		spi-max-frequency = <29000000>;
-		reg = <1>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
new file mode 100644
index 0000000..483e9cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
@@ -0,0 +1,65 @@
+* Freescale Quad Serial Peripheral Interface(QuadSPI)
+
+Required properties:
+  - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
+		 "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
+		 "fsl,ls1021a-qspi"
+		 or
+		 "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
+		 "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
+  - reg : the first contains the register location and length,
+          the second contains the memory mapping address and length
+  - reg-names: Should contain the reg names "QuadSPI" and "QuadSPI-memory"
+  - interrupts : Should contain the interrupt for the device
+  - clocks : The clocks needed by the QuadSPI controller
+  - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
+
+Optional properties:
+  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
+                              Each bus can be connected with two NOR flashes.
+			      Most of the time, each bus only has one NOR flash
+			      connected, this is the default case.
+			      But if there are two NOR flashes connected to the
+			      bus, you should enable this property.
+			      (Please check the board's schematic.)
+  - big-endian : That means the IP register is big endian
+
+Example:
+
+qspi0: quadspi@40044000 {
+	compatible = "fsl,vf610-qspi";
+	reg = <0x40044000 0x1000>, <0x20000000 0x10000000>;
+	reg-names = "QuadSPI", "QuadSPI-memory";
+	interrupts = <0 24 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&clks VF610_CLK_QSPI0_EN>,
+		<&clks VF610_CLK_QSPI0>;
+	clock-names = "qspi_en", "qspi";
+
+	flash0: s25fl128s@0 {
+		....
+	};
+};
+
+Example showing the usage of two SPI NOR devices:
+
+&qspi2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_qspi2>;
+	status = "okay";
+
+	flash0: n25q256a@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "micron,n25q256a", "jedec,spi-nor";
+		spi-max-frequency = <29000000>;
+		reg = <0>;
+	};
+
+	flash1: n25q256a@1 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "micron,n25q256a", "jedec,spi-nor";
+		spi-max-frequency = <29000000>;
+		reg = <1>;
+	};
+};
-- 
2.7.4

Re: [PATCH v2 04/12] dt-bindings: spi: Move the bindings for the FSL QSPI driver

[Rob Herring]

On Thu, Jul 05, 2018 at 01:15:00PM +0200, Frieder Schrempf wrote:
> Move the documentation of the old SPI NOR driver to the place of the new
> SPI memory interface based driver.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> ---
> Changes in v2:
> ==============
> * Split the moving and editing of the dt-bindings in two patches
> 
>  .../devicetree/bindings/mtd/fsl-quadspi.txt     | 65 --------------------
>  .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 65 ++++++++++++++++++++
>  2 files changed, 65 insertions(+), 65 deletions(-)

Next time, use the -M option so the move shows any real diffs.

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 04/12] dt-bindings: spi: Move the bindings for the FSL QSPI driver

[Frieder Schrempf]

Hi Rob,

On 11.07.2018 17:54, Rob Herring wrote:
> On Thu, Jul 05, 2018 at 01:15:00PM +0200, Frieder Schrempf wrote:
>> Move the documentation of the old SPI NOR driver to the place of the new
>> SPI memory interface based driver.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>> ---
>> Changes in v2:
>> ==============
>> * Split the moving and editing of the dt-bindings in two patches
>>
>>   .../devicetree/bindings/mtd/fsl-quadspi.txt     | 65 --------------------
>>   .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 65 ++++++++++++++++++++
>>   2 files changed, 65 insertions(+), 65 deletions(-)
> 
> Next time, use the -M option so the move shows any real diffs.

Ok, I'll keep that in mind.

Thanks,
Frieder

> 
> Acked-by: Rob Herring <robh@kernel.org>
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 05/12] dt-bindings: spi: Adjust the bindings for the FSL QSPI driver

[Frieder Schrempf]

Adjust the documentation of the new SPI memory interface based
driver to reflect the new drivers settings.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
Changes in v2:
==============
* Split the moving and editing of the dt-bindings in two patches

 .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 22 ++++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
index 483e9cf..8b4eed7 100644
--- a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
@@ -3,9 +3,8 @@
 Required properties:
   - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
 		 "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
-		 "fsl,ls1021a-qspi"
+		 "fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
 		 or
-		 "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
 		 "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
   - reg : the first contains the register location and length,
           the second contains the memory mapping address and length
@@ -15,14 +14,15 @@ Required properties:
   - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
 
 Optional properties:
-  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
-                              Each bus can be connected with two NOR flashes.
-			      Most of the time, each bus only has one NOR flash
-			      connected, this is the default case.
-			      But if there are two NOR flashes connected to the
-			      bus, you should enable this property.
-			      (Please check the board's schematic.)
-  - big-endian : That means the IP register is big endian
+  - big-endian : That means the IP registers format is big endian
+
+Required SPI slave node properties:
+  - reg: There are two buses (A and B) with two chip selects each.
+	 This encodes to which bus and CS the flash is connected:
+		<0>: Bus A, CS 0
+		<1>: Bus A, CS 1
+		<2>: Bus B, CS 0
+		<3>: Bus B, CS 1
 
 Example:
 
@@ -40,7 +40,7 @@ qspi0: quadspi@40044000 {
 	};
 };
 
-Example showing the usage of two SPI NOR devices:
+Example showing the usage of two SPI NOR devices on bus A:
 
 &qspi2 {
 	pinctrl-names = "default";
-- 
2.7.4

Re: [PATCH v2 05/12] dt-bindings: spi: Adjust the bindings for the FSL QSPI driver

[Rob Herring]

On Thu, Jul 05, 2018 at 01:15:01PM +0200, Frieder Schrempf wrote:
> Adjust the documentation of the new SPI memory interface based
> driver to reflect the new drivers settings.

Bindings shouldn't change (other than new properties) due to driver 
changes.

> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> ---
> Changes in v2:
> ==============
> * Split the moving and editing of the dt-bindings in two patches
> 
>  .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 22 ++++++++++----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> index 483e9cf..8b4eed7 100644
> --- a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> @@ -3,9 +3,8 @@
>  Required properties:
>    - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
>  		 "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
> -		 "fsl,ls1021a-qspi"
> +		 "fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
>  		 or
> -		 "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
>  		 "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"

So the 2080a h/w was compatible with the 1021a h/w, but now it is not? 
How did the h/w change?

>    - reg : the first contains the register location and length,
>            the second contains the memory mapping address and length
> @@ -15,14 +14,15 @@ Required properties:
>    - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
>  
>  Optional properties:
> -  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
> -                              Each bus can be connected with two NOR flashes.
> -			      Most of the time, each bus only has one NOR flash
> -			      connected, this is the default case.
> -			      But if there are two NOR flashes connected to the
> -			      bus, you should enable this property.
> -			      (Please check the board's schematic.)

You can't just remove properties without explanation. Why is this no 
longer needed? What about backwards compatibility with existing dtbs?

> -  - big-endian : That means the IP register is big endian
> +  - big-endian : That means the IP registers format is big endian

This is a standard property so it doesn't really need to be redefined 
here, but just reference the definition.

> +
> +Required SPI slave node properties:
> +  - reg: There are two buses (A and B) with two chip selects each.
> +	 This encodes to which bus and CS the flash is connected:
> +		<0>: Bus A, CS 0
> +		<1>: Bus A, CS 1
> +		<2>: Bus B, CS 0
> +		<3>: Bus B, CS 1
>  
>  Example:
>  
> @@ -40,7 +40,7 @@ qspi0: quadspi@40044000 {
>  	};
>  };
>  
> -Example showing the usage of two SPI NOR devices:
> +Example showing the usage of two SPI NOR devices on bus A:
>  
>  &qspi2 {
>  	pinctrl-names = "default";
> -- 
> 2.7.4
> 

Re: [PATCH v2 05/12] dt-bindings: spi: Adjust the bindings for the FSL QSPI driver

[Frieder Schrempf]

Hi Rob,

On 11.07.2018 18:05, Rob Herring wrote:
> On Thu, Jul 05, 2018 at 01:15:01PM +0200, Frieder Schrempf wrote:
>> Adjust the documentation of the new SPI memory interface based
>> driver to reflect the new drivers settings.
> 
> Bindings shouldn't change (other than new properties) due to driver
> changes.

Right, I added an explanation below, why I think the changes are necessary.

> 
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>> ---
>> Changes in v2:
>> ==============
>> * Split the moving and editing of the dt-bindings in two patches
>>
>>   .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 22 ++++++++++----------
>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
>> index 483e9cf..8b4eed7 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
>> @@ -3,9 +3,8 @@
>>   Required properties:
>>     - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
>>   		 "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
>> -		 "fsl,ls1021a-qspi"
>> +		 "fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
>>   		 or
>> -		 "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
>>   		 "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
> 
> So the 2080a h/w was compatible with the 1021a h/w, but now it is not?
> How did the h/w change?

I guess this should be posted as a separate fix. Formerly there was only 
"fsl,ls1021a-qspi" handled in the driver and the bindings here claimed 
that "fsl,ls2080a-qspi" is compatible.

Some time ago a separate entry for "fsl,ls2080a-qspi" was added to the 
driver [1] and it adds a quirk, that is not set for "fsl,ls1021a-qspi". 
That's why I concluded, that these two are actually not compatible.

> 
>>     - reg : the first contains the register location and length,
>>             the second contains the memory mapping address and length
>> @@ -15,14 +14,15 @@ Required properties:
>>     - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
>>   
>>   Optional properties:
>> -  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
>> -                              Each bus can be connected with two NOR flashes.
>> -			      Most of the time, each bus only has one NOR flash
>> -			      connected, this is the default case.
>> -			      But if there are two NOR flashes connected to the
>> -			      bus, you should enable this property.
>> -			      (Please check the board's schematic.)
> 
> You can't just remove properties without explanation. Why is this no
> longer needed? What about backwards compatibility with existing dtbs?

You're right, the explanation is missing here.

The "old" driver was using this property to select one of two dual chip 
setups (two chips on one bus or two chips on separate buses). And it 
used the order in which the subnodes are defined in the dt to select the 
CS, the chip is connected to.

Both methods are wrong and in fact the "reg" property should be used to 
determine which bus and CS a chip is connected to. This also enables us 
to use different setups than just single chip, or symmetric dual chip.

So the porting of the driver from the MTD to the SPI framework actually 
enforces the use of the "reg" properties and makes 
"fsl,qspi-has-second-chip" superfluous.

As all boards that have "fsl,qspi-has-second-chip" set, also have 
correct "reg" properties, the removal of this property shouldn't lead to 
any incompatibilities.

The only compatibility issues I can see are with imx6sx-sdb.dts and 
imx6sx-sdb-reva.dts, which have their reg properties set incorrectly 
(see explanation here: [2]), all other boards should stay compatible.

> 
>> -  - big-endian : That means the IP register is big endian
>> +  - big-endian : That means the IP registers format is big endian
> 
> This is a standard property so it doesn't really need to be redefined
> here, but just reference the definition.

So I will change that to:

	big-endian : See common-properties.txt for a definition

Thanks,
Frieder

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mtd/spi-nor/fsl-quadspi.c?h=v4.18-rc4&id=d728a7ea9037c2df085bf9494d56e90d0ff69d7d
[2] https://patchwork.ozlabs.org/patch/922817/#1925445

> 
>> +
>> +Required SPI slave node properties:
>> +  - reg: There are two buses (A and B) with two chip selects each.
>> +	 This encodes to which bus and CS the flash is connected:
>> +		<0>: Bus A, CS 0
>> +		<1>: Bus A, CS 1
>> +		<2>: Bus B, CS 0
>> +		<3>: Bus B, CS 1
>>   
>>   Example:
>>   
>> @@ -40,7 +40,7 @@ qspi0: quadspi@40044000 {
>>   	};
>>   };
>>   
>> -Example showing the usage of two SPI NOR devices:
>> +Example showing the usage of two SPI NOR devices on bus A:
>>   
>>   &qspi2 {
>>   	pinctrl-names = "default";
>> -- 
>> 2.7.4
>>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 05/12] dt-bindings: spi: Adjust the bindings for the FSL QSPI driver

[Rob Herring]

On Thu, Jul 12, 2018 at 2:14 AM Frieder Schrempf
<frieder.schrempf@exceet.de> wrote:
>
> Hi Rob,
>
> On 11.07.2018 18:05, Rob Herring wrote:
> > On Thu, Jul 05, 2018 at 01:15:01PM +0200, Frieder Schrempf wrote:
> >> Adjust the documentation of the new SPI memory interface based
> >> driver to reflect the new drivers settings.
> >
> > Bindings shouldn't change (other than new properties) due to driver
> > changes.
>
> Right, I added an explanation below, why I think the changes are necessary.
>
> >
> >>
> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> >> ---
> >> Changes in v2:
> >> ==============
> >> * Split the moving and editing of the dt-bindings in two patches
> >>
> >>   .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 22 ++++++++++----------
> >>   1 file changed, 11 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> >> index 483e9cf..8b4eed7 100644
> >> --- a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> >> +++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
> >> @@ -3,9 +3,8 @@
> >>   Required properties:
> >>     - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
> >>               "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
> >> -             "fsl,ls1021a-qspi"
> >> +             "fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
> >>               or
> >> -             "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
> >>               "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
> >
> > So the 2080a h/w was compatible with the 1021a h/w, but now it is not?
> > How did the h/w change?
>
> I guess this should be posted as a separate fix. Formerly there was only
> "fsl,ls1021a-qspi" handled in the driver and the bindings here claimed
> that "fsl,ls2080a-qspi" is compatible.
>
> Some time ago a separate entry for "fsl,ls2080a-qspi" was added to the
> driver [1] and it adds a quirk, that is not set for "fsl,ls1021a-qspi".
> That's why I concluded, that these two are actually not compatible.

So before the driver change, the driver didn't work at all on the
ls2080a? If so, then removing is appropriate. If not, then removing
breaks all kernel versions before the change if you use a newer DT.


> >>     - reg : the first contains the register location and length,
> >>             the second contains the memory mapping address and length
> >> @@ -15,14 +14,15 @@ Required properties:
> >>     - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
> >>
> >>   Optional properties:
> >> -  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
> >> -                              Each bus can be connected with two NOR flashes.
> >> -                          Most of the time, each bus only has one NOR flash
> >> -                          connected, this is the default case.
> >> -                          But if there are two NOR flashes connected to the
> >> -                          bus, you should enable this property.
> >> -                          (Please check the board's schematic.)
> >
> > You can't just remove properties without explanation. Why is this no
> > longer needed? What about backwards compatibility with existing dtbs?
>
> You're right, the explanation is missing here.
>
> The "old" driver was using this property to select one of two dual chip
> setups (two chips on one bus or two chips on separate buses). And it
> used the order in which the subnodes are defined in the dt to select the
> CS, the chip is connected to.
>
> Both methods are wrong and in fact the "reg" property should be used to
> determine which bus and CS a chip is connected to. This also enables us
> to use different setups than just single chip, or symmetric dual chip.
>
> So the porting of the driver from the MTD to the SPI framework actually
> enforces the use of the "reg" properties and makes
> "fsl,qspi-has-second-chip" superfluous.
>
> As all boards that have "fsl,qspi-has-second-chip" set, also have
> correct "reg" properties, the removal of this property shouldn't lead to
> any incompatibilities.
>
> The only compatibility issues I can see are with imx6sx-sdb.dts and
> imx6sx-sdb-reva.dts, which have their reg properties set incorrectly
> (see explanation here: [2]), all other boards should stay compatible.

Add this to the commit msg.

> >> -  - big-endian : That means the IP register is big endian
> >> +  - big-endian : That means the IP registers format is big endian
> >
> > This is a standard property so it doesn't really need to be redefined
> > here, but just reference the definition.
>
> So I will change that to:
>
>         big-endian : See common-properties.txt for a definition

You can drop "for a definition"

Rob

Re: [PATCH v2 05/12] dt-bindings: spi: Adjust the bindings for the FSL QSPI driver

[Frieder Schrempf]

Hi Rob,

On 12.07.2018 17:20, Rob Herring wrote:
> On Thu, Jul 12, 2018 at 2:14 AM Frieder Schrempf
> <frieder.schrempf@exceet.de> wrote:
>>
>> Hi Rob,
>>
>> On 11.07.2018 18:05, Rob Herring wrote:
>>> On Thu, Jul 05, 2018 at 01:15:01PM +0200, Frieder Schrempf wrote:
>>>> Adjust the documentation of the new SPI memory interface based
>>>> driver to reflect the new drivers settings.
>>>
>>> Bindings shouldn't change (other than new properties) due to driver
>>> changes.
>>
>> Right, I added an explanation below, why I think the changes are necessary.
>>
>>>
>>>>
>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>>>> ---
>>>> Changes in v2:
>>>> ==============
>>>> * Split the moving and editing of the dt-bindings in two patches
>>>>
>>>>    .../devicetree/bindings/spi/spi-fsl-qspi.txt    | 22 ++++++++++----------
>>>>    1 file changed, 11 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
>>>> index 483e9cf..8b4eed7 100644
>>>> --- a/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
>>>> +++ b/Documentation/devicetree/bindings/spi/spi-fsl-qspi.txt
>>>> @@ -3,9 +3,8 @@
>>>>    Required properties:
>>>>      - compatible : Should be "fsl,vf610-qspi", "fsl,imx6sx-qspi",
>>>>                "fsl,imx7d-qspi", "fsl,imx6ul-qspi",
>>>> -             "fsl,ls1021a-qspi"
>>>> +             "fsl,ls1021a-qspi", "fsl,ls2080a-qspi"
>>>>                or
>>>> -             "fsl,ls2080a-qspi" followed by "fsl,ls1021a-qspi",
>>>>                "fsl,ls1043a-qspi" followed by "fsl,ls1021a-qspi"
>>>
>>> So the 2080a h/w was compatible with the 1021a h/w, but now it is not?
>>> How did the h/w change?
>>
>> I guess this should be posted as a separate fix. Formerly there was only
>> "fsl,ls1021a-qspi" handled in the driver and the bindings here claimed
>> that "fsl,ls2080a-qspi" is compatible.
>>
>> Some time ago a separate entry for "fsl,ls2080a-qspi" was added to the
>> driver [1] and it adds a quirk, that is not set for "fsl,ls1021a-qspi".
>> That's why I concluded, that these two are actually not compatible.
> 
> So before the driver change, the driver didn't work at all on the
> ls2080a? If so, then removing is appropriate. If not, then removing
> breaks all kernel versions before the change if you use a newer DT.

Before the driver change it couldn't work fully on the ls2080a, as the 
quirk was missing. At least that's what I deduce from the driver.

> 
> 
>>>>      - reg : the first contains the register location and length,
>>>>              the second contains the memory mapping address and length
>>>> @@ -15,14 +14,15 @@ Required properties:
>>>>      - clock-names : Should contain the name of the clocks: "qspi_en" and "qspi".
>>>>
>>>>    Optional properties:
>>>> -  - fsl,qspi-has-second-chip: The controller has two buses, bus A and bus B.
>>>> -                              Each bus can be connected with two NOR flashes.
>>>> -                          Most of the time, each bus only has one NOR flash
>>>> -                          connected, this is the default case.
>>>> -                          But if there are two NOR flashes connected to the
>>>> -                          bus, you should enable this property.
>>>> -                          (Please check the board's schematic.)
>>>
>>> You can't just remove properties without explanation. Why is this no
>>> longer needed? What about backwards compatibility with existing dtbs?
>>
>> You're right, the explanation is missing here.
>>
>> The "old" driver was using this property to select one of two dual chip
>> setups (two chips on one bus or two chips on separate buses). And it
>> used the order in which the subnodes are defined in the dt to select the
>> CS, the chip is connected to.
>>
>> Both methods are wrong and in fact the "reg" property should be used to
>> determine which bus and CS a chip is connected to. This also enables us
>> to use different setups than just single chip, or symmetric dual chip.
>>
>> So the porting of the driver from the MTD to the SPI framework actually
>> enforces the use of the "reg" properties and makes
>> "fsl,qspi-has-second-chip" superfluous.
>>
>> As all boards that have "fsl,qspi-has-second-chip" set, also have
>> correct "reg" properties, the removal of this property shouldn't lead to
>> any incompatibilities.
>>
>> The only compatibility issues I can see are with imx6sx-sdb.dts and
>> imx6sx-sdb-reva.dts, which have their reg properties set incorrectly
>> (see explanation here: [2]), all other boards should stay compatible.
> 
> Add this to the commit msg.

Ok.

> 
>>>> -  - big-endian : That means the IP register is big endian
>>>> +  - big-endian : That means the IP registers format is big endian
>>>
>>> This is a standard property so it doesn't really need to be redefined
>>> here, but just reference the definition.
>>
>> So I will change that to:
>>
>>          big-endian : See common-properties.txt for a definition
> 
> You can drop "for a definition"

Ok.

Thanks,
Frieder

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 06/12] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties

[Frieder Schrempf]

The FSL QSPI driver was moved to the SPI framework and it now
acts as a SPI controller. Therefore the subnodes need to set
spi-[rx/tx]-bus-width = <4>, so quad mode is used just as before.

Also the properties 'bus-num', 'fsl,spi-num-chipselects' and
'fsl,spi-flash-chipselects' were never read by the driver and
can be removed.

The 'reg' properties are adjusted to reflect the what bus and
chipselect the flash is connected to, as the new driver needs
this information.

The property 'fsl,qspi-has-second-chip' is not needed anymore
and will be removed after the old driver was disabled to avoid
breaking ls1021a-moxa-uc-8410a.dts.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm/boot/dts/imx6sx-sdb-reva.dts       | 8 ++++++--
 arch/arm/boot/dts/imx6sx-sdb.dts            | 8 ++++++--
 arch/arm/boot/dts/imx6ul-14x14-evk.dtsi     | 2 ++
 arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 5 ++---
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
index e3533e7..1a6f680 100644
--- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
@@ -131,13 +131,17 @@
 		#size-cells = <1>;
 		compatible = "spansion,s25fl128s", "jedec,spi-nor";
 		spi-max-frequency = <66000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 	};
 
-	flash1: s25fl128s@1 {
-		reg = <1>;
+	flash1: s25fl128s@2 {
+		reg = <2>;
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "spansion,s25fl128s", "jedec,spi-nor";
 		spi-max-frequency = <66000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 	};
 };
diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
index 6dd9beb..9acfda8 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -117,15 +117,19 @@
 		#size-cells = <1>;
 		compatible = "micron,n25q256a", "jedec,spi-nor";
 		spi-max-frequency = <29000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 
-	flash1: n25q256a@1 {
+	flash1: n25q256a@2 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		compatible = "micron,n25q256a", "jedec,spi-nor";
 		spi-max-frequency = <29000000>;
-		reg = <1>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
+		reg = <2>;
 	};
 };
 
diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
index 32a0723..c2c9a2a 100644
--- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
+++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi
@@ -176,6 +176,8 @@
 		#size-cells = <1>;
 		compatible = "micron,n25q256a";
 		spi-max-frequency = <29000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 };
diff --git a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
index d01f64b..6a83f30 100644
--- a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
+++ b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
@@ -203,9 +203,6 @@
 };
 
 &qspi {
-	bus-num = <0>;
-	fsl,spi-num-chipselects = <2>;
-	fsl,spi-flash-chipselects = <0>;
 	fsl,qspi-has-second-chip;
 	status = "okay";
 
@@ -214,6 +211,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 
 		partitions@0 {
-- 
2.7.4

[PATCH v2 07/12] arm64: dts: Reflect change of FSL QSPI driver and remove unused properties

[Frieder Schrempf]

The FSL QSPI driver was moved to the SPI framework and it now
acts as a SPI controller. Therefore the subnodes need to set
spi-[rx/tx]-bus-width = <4>, so quad mode is used just as before.

Also the properties 'num-cs' and 'bus-num' were never read by the
driver and can be removed.

The property 'fsl,qspi-has-second-chip' is not needed anymore
and will be removed after the old driver was disabled to avoid
breaking fsl-ls1046a-rdb.dts.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts  | 3 ++-
 arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts  | 4 ++--
 arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts  | 6 ++++--
 arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi | 4 ++++
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
index 6341281..31e7b31 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
@@ -170,7 +170,6 @@
 };
 
 &qspi {
-	bus-num = <0>;
 	status = "okay";
 
 	qflash0: s25fl128s@0 {
@@ -178,6 +177,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
index 434383b..dc10105 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
@@ -198,8 +198,6 @@
 };
 
 &qspi {
-	num-cs = <2>;
-	bus-num = <0>;
 	status = "okay";
 
 	qflash0: s25fl128s@0 {
@@ -207,6 +205,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
index 5dc2782..1848c33 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
@@ -136,8 +136,6 @@
 };
 
 &qspi {
-	num-cs = <2>;
-	bus-num = <0>;
 	status = "okay";
 
 	qflash0: s25fs512s@0 {
@@ -145,6 +143,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 
@@ -153,6 +153,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <1>;
 	};
 };
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
index 1de6188..fc62ed9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
@@ -170,6 +170,8 @@
 		#size-cells = <1>;
 		compatible = "st,m25p80";
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 	flash2: s25fl256s1@2 {
@@ -177,6 +179,8 @@
 		#size-cells = <1>;
 		compatible = "st,m25p80";
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <2>;
 	};
 };
-- 
2.7.4

[PATCH v2 10/12] ARM: dts: ls1021a: Remove fsl,qspi-has-second-chip as it is not used

[Frieder Schrempf]

After switching to the new FSL QSPI driver the property
'fsl,qspi-has-second-chip' is not needed anymore.

The driver now uses the 'reg' property to determine the bus and
the chipselect.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
index 6a83f30..d3a1a73 100644
--- a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
+++ b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
@@ -203,7 +203,6 @@
 };
 
 &qspi {
-	fsl,qspi-has-second-chip;
 	status = "okay";
 
 	flash: flash@0 {
-- 
2.7.4

[PATCH v2 11/12] ARM64: dts: ls1046a: Remove fsl,qspi-has-second-chip as it is not used

[Frieder Schrempf]

After switching to the new FSL QSPI driver the property
'fsl,qspi-has-second-chip' is not needed anymore.

The driver now uses the 'reg' property to determine the bus and
the chipselect.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 136ebfa..871189e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -247,7 +247,6 @@
 			clock-names = "qspi_en", "qspi";
 			clocks = <&clockgen 4 1>, <&clockgen 4 1>;
 			big-endian;
-			fsl,qspi-has-second-chip;
 			status = "disabled";
 		};
 
-- 
2.7.4