[PATCH 1/2] ARM: dts: socfpga: change qspi to "intel,socfpga-qspi"

[PATCH 1/2] ARM: dts: socfpga: change qspi to "intel,socfpga-qspi"

[Dinh Nguyen]

Because of commit 9cb2ff111712 ("spi: cadence-quadspi: Disable Auto-HW polling"),
which does a write to the CQSPI_REG_WR_COMPLETION_CTRL register
regardless of any condition. Well, the Cadence QuadSPI controller on
Intel's SoCFPGA platforms does not implement the
CQSPI_REG_WR_COMPLETION_CTRL register, thus a write to this register
results in a crash!

So starting with v5.16, I introduced the patch
98d948eb833 ("spi: cadence-quadspi: fix write completion support"),
which adds the dts property "intel,socfpga-qspi" that is specific for
the QSPI on SoCFPGA that doesn't have the CQSPI_REG_WR_COMPLETION_CTRL
register implemented.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 arch/arm/boot/dts/socfpga.dtsi                    | 2 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi            | 2 +-
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 2 +-
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 0b021eef0b53..108c3610bf52 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -782,7 +782,7 @@ ocram: sram@ffff0000 {
 		};
 
 		qspi: spi@ff705000 {
-			compatible = "cdns,qspi-nor";
+			compatible = "intel,socfpga-qpsi";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xff705000 0x1000>,
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
index a574ea91d9d3..e1a70f3a641d 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -756,7 +756,7 @@ usb0-ecc@ff8c8800 {
 		};
 
 		qspi: spi@ff809000 {
-			compatible = "cdns,qspi-nor";
+			compatible = "intel,socfpga-qspi";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xff809000 0x100>,
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index d301ac0d406b..d391153c9e6d 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -594,7 +594,7 @@ emac0-tx-ecc@ff8c0400 {
 		};
 
 		qspi: spi@ff8d2000 {
-			compatible = "cdns,qspi-nor";
+			compatible = "intel,socfpga-qspi";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xff8d2000 0x100>,
diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
index 163f33b46e4f..de6dd2189e74 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
@@ -628,7 +628,7 @@ sdmmca-ecc@ff8c8c00 {
 		};
 
 		qspi: spi@ff8d2000 {
-			compatible = "cdns,qspi-nor";
+			compatible = "intel,socfpga-qspi";
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xff8d2000 0x100>,
-- 
2.25.1

[PATCH 2/2] dt-bindings: spi: cadence-quadspi: document "intel,socfpga-qspi"

[Dinh Nguyen]

The QSPI controller on Intel's SoCFPGA platform does not implement the
CQSPI_REG_WR_COMPLETION_CTRL register, thus a write to this register
results in a crash.

Introduce the dts binding "intel,socfpga-qspi" to differentiate the
hardware.

Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index ca155abbda7a..037f41f58503 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -29,6 +29,7 @@ properties:
               - ti,am654-ospi
               - intel,lgm-qspi
               - xlnx,versal-ospi-1.0
+              - intel,socfpga-qspi
           - const: cdns,qspi-nor
       - const: cdns,qspi-nor
 
-- 
2.25.1

Re: [PATCH 1/2] ARM: dts: socfpga: change qspi to "intel,socfpga-qspi"

[Rob Herring]

On Mon, Nov 22, 2021 at 10:04:26AM -0600, Dinh Nguyen wrote:
> Because of commit 9cb2ff111712 ("spi: cadence-quadspi: Disable Auto-HW polling"),
> which does a write to the CQSPI_REG_WR_COMPLETION_CTRL register
> regardless of any condition. Well, the Cadence QuadSPI controller on
> Intel's SoCFPGA platforms does not implement the
> CQSPI_REG_WR_COMPLETION_CTRL register, thus a write to this register
> results in a crash!
> 
> So starting with v5.16, I introduced the patch
> 98d948eb833 ("spi: cadence-quadspi: fix write completion support"),
> which adds the dts property "intel,socfpga-qspi" that is specific for

'intel,socfpga-qspi' is not a DT property. It's a value for 
'compatible'.

Is this change going to stable? That would at least fix old kernels 
with new DT (assuming they get stable updates). But new kernels with old 
DT are still broken. Not a great story here.

> the QSPI on SoCFPGA that doesn't have the CQSPI_REG_WR_COMPLETION_CTRL
> register implemented.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  arch/arm/boot/dts/socfpga.dtsi                    | 2 +-
>  arch/arm/boot/dts/socfpga_arria10.dtsi            | 2 +-
>  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 2 +-
>  arch/arm64/boot/dts/intel/socfpga_agilex.dtsi     | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index 0b021eef0b53..108c3610bf52 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -782,7 +782,7 @@ ocram: sram@ffff0000 {
>  		};
>  
>  		qspi: spi@ff705000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qpsi";

Obviously not tested.


>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff705000 0x1000>,
> diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
> index a574ea91d9d3..e1a70f3a641d 100644
> --- a/arch/arm/boot/dts/socfpga_arria10.dtsi
> +++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
> @@ -756,7 +756,7 @@ usb0-ecc@ff8c8800 {
>  		};
>  
>  		qspi: spi@ff809000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qspi";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff809000 0x100>,
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index d301ac0d406b..d391153c9e6d 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -594,7 +594,7 @@ emac0-tx-ecc@ff8c0400 {
>  		};
>  
>  		qspi: spi@ff8d2000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qspi";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff8d2000 0x100>,
> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> index 163f33b46e4f..de6dd2189e74 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> @@ -628,7 +628,7 @@ sdmmca-ecc@ff8c8c00 {
>  		};
>  
>  		qspi: spi@ff8d2000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qspi";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff8d2000 0x100>,
> -- 
> 2.25.1
> 
> 

Re: [PATCH 2/2] dt-bindings: spi: cadence-quadspi: document "intel,socfpga-qspi"

[Rob Herring]

On Mon, Nov 22, 2021 at 10:04:27AM -0600, Dinh Nguyen wrote:
> The QSPI controller on Intel's SoCFPGA platform does not implement the
> CQSPI_REG_WR_COMPLETION_CTRL register, thus a write to this register
> results in a crash.
> 
> Introduce the dts binding "intel,socfpga-qspi" to differentiate the
> hardware.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index ca155abbda7a..037f41f58503 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -29,6 +29,7 @@ properties:
>                - ti,am654-ospi
>                - intel,lgm-qspi
>                - xlnx,versal-ospi-1.0
> +              - intel,socfpga-qspi
>            - const: cdns,qspi-nor

Doesn't match what you changed in the dts files.

Please run schema validation so YOU find this issue and typos instead of 
me.

Rob

Re: [PATCH 1/2] ARM: dts: socfpga: change qspi to "intel,socfpga-qspi"

[Rob Herring]

On Mon, Nov 22, 2021 at 10:04:26AM -0600, Dinh Nguyen wrote:
> Because of commit 9cb2ff111712 ("spi: cadence-quadspi: Disable Auto-HW polling"),
> which does a write to the CQSPI_REG_WR_COMPLETION_CTRL register
> regardless of any condition. Well, the Cadence QuadSPI controller on
> Intel's SoCFPGA platforms does not implement the
> CQSPI_REG_WR_COMPLETION_CTRL register, thus a write to this register
> results in a crash!
> 
> So starting with v5.16, I introduced the patch
> 98d948eb833 ("spi: cadence-quadspi: fix write completion support"),
> which adds the dts property "intel,socfpga-qspi" that is specific for
> the QSPI on SoCFPGA that doesn't have the CQSPI_REG_WR_COMPLETION_CTRL
> register implemented.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
>  arch/arm/boot/dts/socfpga.dtsi                    | 2 +-
>  arch/arm/boot/dts/socfpga_arria10.dtsi            | 2 +-
>  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 2 +-
>  arch/arm64/boot/dts/intel/socfpga_agilex.dtsi     | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index 0b021eef0b53..108c3610bf52 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -782,7 +782,7 @@ ocram: sram@ffff0000 {
>  		};
>  
>  		qspi: spi@ff705000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qpsi";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff705000 0x1000>,
> diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi b/arch/arm/boot/dts/socfpga_arria10.dtsi
> index a574ea91d9d3..e1a70f3a641d 100644
> --- a/arch/arm/boot/dts/socfpga_arria10.dtsi
> +++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
> @@ -756,7 +756,7 @@ usb0-ecc@ff8c8800 {
>  		};
>  
>  		qspi: spi@ff809000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qspi";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff809000 0x100>,
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index d301ac0d406b..d391153c9e6d 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -594,7 +594,7 @@ emac0-tx-ecc@ff8c0400 {
>  		};
>  
>  		qspi: spi@ff8d2000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qspi";
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff8d2000 0x100>,
> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> index 163f33b46e4f..de6dd2189e74 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> @@ -628,7 +628,7 @@ sdmmca-ecc@ff8c8c00 {
>  		};
>  
>  		qspi: spi@ff8d2000 {
> -			compatible = "cdns,qspi-nor";
> +			compatible = "intel,socfpga-qspi";

A few more comments...

Now instead of reusing the same generic compatible, you are reusing the 
same compatible across 4 different SoCs and setting yourself up for the 
same compatibility breakage again if there is some difference among 
these SoCs.

Also, keep "cdns,qspi-nor" as a fall back and you will solve the new DT 
+ old kernel combination and the schema errors.

Rob