[PATCH v4 0/3] Add initialization of clock for StarFive JH7110 SoC

[PATCH v4 0/3] Add initialization of clock for StarFive JH7110 SoC

[William Qiu]

Hi,

This patchset adds initial rudimentary support for the StarFive
Quad SPI controller driver. And this driver will be used in
StarFive's VisionFive 2 board. In 6.4, the QSPI_AHB and QSPI_APB
clocks changed from the default ON state to the default OFF state,
so these clocks need to be enabled in the driver.At the same time,
dts patch is added to this series.

Changes v3->v4:
- Added minItems for clocks.
- Added clock names property.
- Fixed formatting issues.

Changes v2->v3:
- Rebaed to v6.4rc6.
- Renamed the clock names.
- Changed the variable definition type.

Changes v1->v2:
- Renamed the clock names.
- Specified a different array of clocks.
- Used clk_bulk_ APIs.

The patch series is based on v6.4rc6.

William Qiu (3):
  dt-bindings: qspi: cdns,qspi-nor: Add clocks for StarFive JH7110 SoC
  spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI
  riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

 .../bindings/spi/cdns,qspi-nor.yaml           | 12 ++++++-
 .../jh7110-starfive-visionfive-2.dtsi         | 32 +++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++++++++
 drivers/spi/spi-cadence-quadspi.c             | 20 ++++++++++++
 4 files changed, 81 insertions(+), 1 deletion(-)

--
2.34.1

[PATCH v4 1/3] dt-bindings: qspi: cdns,qspi-nor: Add clocks for StarFive JH7110 SoC

[William Qiu]

The QSPI controller needs three clock items to work properly on StarFive
JH7110 SoC, so there is need to change the maxItems's value to 3.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/spi/cdns,qspi-nor.yaml       | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index b310069762dd..e048cf63215b 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -70,7 +70,17 @@ properties:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
+
+  clock-names:
+    oneOf:
+      - items:
+          - const: ref
+      - items:
+          - const: ref
+          - const: ahb
+          - const: apb
 
   cdns,fifo-depth:
     description:
-- 
2.34.1

[PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[William Qiu]

Add QSPI clock operation in device probe.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/
Reported-by: Julia Lawall <julia.lawall@inria.fr>
Closes: https://lore.kernel.org/r/202306040644.6ZHs55x4-lkp@intel.com/
---
 drivers/spi/spi-cadence-quadspi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 6ddb2dfc0f00..8774f9aaff61 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -63,6 +63,8 @@ struct cqspi_st {
 	struct platform_device	*pdev;
 	struct spi_master	*master;
 	struct clk		*clk;
+	struct clk_bulk_data	*clks;
+	int			num_clks;
 	unsigned int		sclk;
 
 	void __iomem		*iobase;
@@ -1715,6 +1717,16 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	if (of_device_is_compatible(pdev->dev.of_node, "starfive,jh7110-qspi")) {
+		cqspi->num_clks = devm_clk_bulk_get_all(dev, &cqspi->clks);
+		if (cqspi->num_clks < 0) {
+			dev_err(dev, "Cannot claim clock: %u\n", cqspi->num_clks);
+			return -EINVAL;
+		}
+
+		ret = clk_bulk_prepare_enable(cqspi->num_clks, cqspi->clks);
+		if (ret)
+			dev_err(dev, "Cannot enable clock clks\n");
+
 		rstc_ref = devm_reset_control_get_optional_exclusive(dev, "rstc_ref");
 		if (IS_ERR(rstc_ref)) {
 			ret = PTR_ERR(rstc_ref);
@@ -1816,6 +1828,9 @@ static void cqspi_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(cqspi->clk);
 
+	if (of_device_is_compatible(pdev->dev.of_node, "starfive,jh7110-qspi"))
+		clk_bulk_disable_unprepare(cqspi->num_clks, cqspi->clks);
+
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 }
@@ -1831,6 +1846,9 @@ static int cqspi_suspend(struct device *dev)
 
 	clk_disable_unprepare(cqspi->clk);
 
+	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
+		clk_bulk_disable_unprepare(cqspi->num_clks, cqspi->clks);
+
 	return ret;
 }
 
@@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
 	struct spi_master *master = dev_get_drvdata(dev);
 
 	clk_prepare_enable(cqspi->clk);
+	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
+		clk_bulk_prepare_enable(cqspi->num_clks, cqspi->clks);
 	cqspi_wait_idle(cqspi);
 	cqspi_controller_init(cqspi);
 
-- 
2.34.1

[PATCH v4 3/3] riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

[William Qiu]

Add the quad spi controller node for the StarFive JH7110 SoC.

Co-developed-by: Ziv Xu <ziv.xu@starfivetech.com>
Signed-off-by: Ziv Xu <ziv.xu@starfivetech.com>
Signed-off-by: William Qiu <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../jh7110-starfive-visionfive-2.dtsi         | 32 +++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++++++++
 2 files changed, 50 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index 2a6d81609284..983b683e2f27 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -126,6 +126,38 @@ &i2c6 {
 	status = "okay";
 };
 
+&qspi {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	nor_flash: flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		cdns,read-delay = <5>;
+		spi-max-frequency = <12000000>;
+		cdns,tshsl-ns = <1>;
+		cdns,tsd2d-ns = <1>;
+		cdns,tchsh-ns = <1>;
+		cdns,tslch-ns = <1>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			spl@0 {
+				reg = <0x0 0x20000>;
+			};
+			uboot@100000 {
+				reg = <0x100000 0x300000>;
+			};
+			data@f00000 {
+				reg = <0xf00000 0x100000>;
+			};
+		};
+	};
+};
+
 &sysgpio {
 	i2c0_pins: i2c0-0 {
 		i2c-pins {
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 4c5fdb905da8..fe33c5616565 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -440,6 +440,24 @@ i2c6: i2c@12060000 {
 			status = "disabled";
 		};
 
+		qspi: spi@13010000 {
+			compatible = "starfive,jh7110-qspi", "cdns,qspi-nor";
+			reg = <0x0 0x13010000 0x0 0x10000>,
+			      <0x0 0x21000000 0x0 0x400000>;
+			interrupts = <25>;
+			clocks = <&syscrg JH7110_SYSCLK_QSPI_REF>,
+				 <&syscrg JH7110_SYSCLK_QSPI_AHB>,
+				 <&syscrg JH7110_SYSCLK_QSPI_APB>;
+			clock-names = "ref", "ahb", "apb";
+			resets = <&syscrg JH7110_SYSRST_QSPI_APB>,
+				 <&syscrg JH7110_SYSRST_QSPI_AHB>,
+				 <&syscrg JH7110_SYSRST_QSPI_REF>;
+			reset-names = "qspi", "qspi-ocp", "rstc_ref";
+			cdns,fifo-depth = <256>;
+			cdns,fifo-width = <4>;
+			cdns,trigger-address = <0x0>;
+		};
+
 		syscrg: clock-controller@13020000 {
 			compatible = "starfive,jh7110-syscrg";
 			reg = <0x0 0x13020000 0x0 0x10000>;
-- 
2.34.1

Re: [PATCH v4 3/3] riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

[Krzysztof Kozlowski]

On 04/07/2023 11:04, William Qiu wrote:
> Add the quad spi controller node for the StarFive JH7110 SoC.
> 
> Co-developed-by: Ziv Xu <ziv.xu@starfivetech.com>
> Signed-off-by: Ziv Xu <ziv.xu@starfivetech.com>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>

...

> +		qspi: spi@13010000 {
> +			compatible = "starfive,jh7110-qspi", "cdns,qspi-nor";
> +			reg = <0x0 0x13010000 0x0 0x10000>,
> +			      <0x0 0x21000000 0x0 0x400000>;
> +			interrupts = <25>;
> +			clocks = <&syscrg JH7110_SYSCLK_QSPI_REF>,
> +				 <&syscrg JH7110_SYSCLK_QSPI_AHB>,
> +				 <&syscrg JH7110_SYSCLK_QSPI_APB>;
> +			clock-names = "ref", "ahb", "apb";
> +			resets = <&syscrg JH7110_SYSRST_QSPI_APB>,
> +				 <&syscrg JH7110_SYSRST_QSPI_AHB>,
> +				 <&syscrg JH7110_SYSRST_QSPI_REF>;
> +			reset-names = "qspi", "qspi-ocp", "rstc_ref";
> +			cdns,fifo-depth = <256>;
> +			cdns,fifo-width = <4>;
> +			cdns,trigger-address = <0x0>;

Bus nodes are usually disabled by default and enabled when needed for
specific boards.

Best regards,
Krzysztof

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

Hey William,

On Tue, Jul 04, 2023 at 05:04:52PM +0800, William Qiu wrote:
> Add QSPI clock operation in device probe.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/

These Reported-by tags don't seem correct, given they were reports about
this patch, not the reason for it - but did you actually check that you
fixed the errors that the patch produces?

This particular one seems to complain about a hunk that is still in the
patch & the CI running on the RISC-V patchwork is complaining about it.

Cheers,
Conor.

> @@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
>  	struct spi_master *master = dev_get_drvdata(dev);
>  
>  	clk_prepare_enable(cqspi->clk);
> +	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
> +		clk_bulk_prepare_enable(cqspi->num_clks, cqspi->clks);
>  	cqspi_wait_idle(cqspi);
>  	cqspi_controller_init(cqspi);
>  
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Tue, Jul 04, 2023 at 05:36:03PM +0100, Conor Dooley wrote:
> On Tue, Jul 04, 2023 at 05:04:52PM +0800, William Qiu wrote:
> > Add QSPI clock operation in device probe.
> > 
> > Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> > Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/

> These Reported-by tags don't seem correct, given they were reports about
> this patch, not the reason for it - but did you actually check that you
> fixed the errors that the patch produces?

Yeah, the Reported-bys that LKP sends in response to on list patches are
a menace, they just generate noise.

> This particular one seems to complain about a hunk that is still in the
> patch & the CI running on the RISC-V patchwork is complaining about it.

I'm surprised that builds cleanly anywhere...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[Krzysztof Kozlowski]

On 04/07/2023 11:04, William Qiu wrote:
> Add QSPI clock operation in device probe.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/
> Reported-by: Julia Lawall <julia.lawall@inria.fr>
> Closes: https://lore.kernel.org/r/202306040644.6ZHs55x4-lkp@intel.com/


>  
> @@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
>  	struct spi_master *master = dev_get_drvdata(dev);
>  
>  	clk_prepare_enable(cqspi->clk);
> +	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))

Don't add compatible checks inside the code. It does not scale. We
expect compatibles to be listed only in one place - of_device_id - and
customize driver with match data / quirks / flags.

Comment applies to all your diff hunks.

> +		clk_bulk_prepare_enable(cqspi->num_clks, cqspi->clks);
>  	cqspi_wait_idle(cqspi);
>  	cqspi_controller_init(cqspi);
>  

Best regards,
Krzysztof

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[William Qiu]

On 2023/7/5 0:36, Conor Dooley wrote:
> Hey William,
> 
> On Tue, Jul 04, 2023 at 05:04:52PM +0800, William Qiu wrote:
>> Add QSPI clock operation in device probe.
>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/
> 
> These Reported-by tags don't seem correct, given they were reports about
> this patch, not the reason for it - but did you actually check that you
> fixed the errors that the patch produces?
> 
> This particular one seems to complain about a hunk that is still in the
> patch & the CI running on the RISC-V patchwork is complaining about it.
> 
> Cheers,
> Conor.
> 
I checked and found that I had only partially fixed it. I'll fix it in next
version.
Thanks for your comments.

Best regards,
William
>> @@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
>>  	struct spi_master *master = dev_get_drvdata(dev);
>>  
>>  	clk_prepare_enable(cqspi->clk);
>> +	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
>> +		clk_bulk_prepare_enable(cqspi->num_clks, cqspi->clks);
>>  	cqspi_wait_idle(cqspi);
>>  	cqspi_controller_init(cqspi);
>>  
>> -- 
>> 2.34.1
>> 

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[William Qiu]

On 2023/7/5 14:21, Krzysztof Kozlowski wrote:
> On 04/07/2023 11:04, William Qiu wrote:
>> Add QSPI clock operation in device probe.
>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/
>> Reported-by: Julia Lawall <julia.lawall@inria.fr>
>> Closes: https://lore.kernel.org/r/202306040644.6ZHs55x4-lkp@intel.com/
> 
> 
>>  
>> @@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
>>  	struct spi_master *master = dev_get_drvdata(dev);
>>  
>>  	clk_prepare_enable(cqspi->clk);
>> +	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
> 
> Don't add compatible checks inside the code. It does not scale. We
> expect compatibles to be listed only in one place - of_device_id - and
> customize driver with match data / quirks / flags.
> 
> Comment applies to all your diff hunks.
> 
I'll use "of_device_get_match_data" to replace it. But the way I added
reset before is also by compatible checks. Should I change this place to 
"of_device_get_match_data" as well?
>> +		clk_bulk_prepare_enable(cqspi->num_clks, cqspi->clks);
>>  	cqspi_wait_idle(cqspi);
>>  	cqspi_controller_init(cqspi);
>>  
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 3/3] riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

[William Qiu]

On 2023/7/4 17:43, Krzysztof Kozlowski wrote:
> On 04/07/2023 11:04, William Qiu wrote:
>> Add the quad spi controller node for the StarFive JH7110 SoC.
>> 
>> Co-developed-by: Ziv Xu <ziv.xu@starfivetech.com>
>> Signed-off-by: Ziv Xu <ziv.xu@starfivetech.com>
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> 
> ...
> 
>> +		qspi: spi@13010000 {
>> +			compatible = "starfive,jh7110-qspi", "cdns,qspi-nor";
>> +			reg = <0x0 0x13010000 0x0 0x10000>,
>> +			      <0x0 0x21000000 0x0 0x400000>;
>> +			interrupts = <25>;
>> +			clocks = <&syscrg JH7110_SYSCLK_QSPI_REF>,
>> +				 <&syscrg JH7110_SYSCLK_QSPI_AHB>,
>> +				 <&syscrg JH7110_SYSCLK_QSPI_APB>;
>> +			clock-names = "ref", "ahb", "apb";
>> +			resets = <&syscrg JH7110_SYSRST_QSPI_APB>,
>> +				 <&syscrg JH7110_SYSRST_QSPI_AHB>,
>> +				 <&syscrg JH7110_SYSRST_QSPI_REF>;
>> +			reset-names = "qspi", "qspi-ocp", "rstc_ref";
>> +			cdns,fifo-depth = <256>;
>> +			cdns,fifo-width = <4>;
>> +			cdns,trigger-address = <0x0>;
> 
> Bus nodes are usually disabled by default and enabled when needed for
> specific boards.
> 
Will add status.
Thanks for your comments.

Best regards,
William
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[Krzysztof Kozlowski]

On 05/07/2023 09:04, William Qiu wrote:
> 
> 
> On 2023/7/5 14:21, Krzysztof Kozlowski wrote:
>> On 04/07/2023 11:04, William Qiu wrote:
>>> Add QSPI clock operation in device probe.
>>>
>>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/
>>> Reported-by: Julia Lawall <julia.lawall@inria.fr>
>>> Closes: https://lore.kernel.org/r/202306040644.6ZHs55x4-lkp@intel.com/
>>
>>
>>>  
>>> @@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
>>>  	struct spi_master *master = dev_get_drvdata(dev);
>>>  
>>>  	clk_prepare_enable(cqspi->clk);
>>> +	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
>>
>> Don't add compatible checks inside the code. It does not scale. We
>> expect compatibles to be listed only in one place - of_device_id - and
>> customize driver with match data / quirks / flags.
>>
>> Comment applies to all your diff hunks.
>>
> I'll use "of_device_get_match_data" to replace it. But the way I added
> reset before is also by compatible checks. Should I change this place to 
> "of_device_get_match_data" as well?

I don't know what's there, but in general driver should be written in a
consistent style.

Best regards,
Krzysztof

Re: [PATCH v4 2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI

[William Qiu]

On 2023/7/5 15:23, Krzysztof Kozlowski wrote:
> On 05/07/2023 09:04, William Qiu wrote:
>> 
>> 
>> On 2023/7/5 14:21, Krzysztof Kozlowski wrote:
>>> On 04/07/2023 11:04, William Qiu wrote:
>>>> Add QSPI clock operation in device probe.
>>>>
>>>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>>>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202306022017.UbwjjWRN-lkp@intel.com/
>>>> Reported-by: Julia Lawall <julia.lawall@inria.fr>
>>>> Closes: https://lore.kernel.org/r/202306040644.6ZHs55x4-lkp@intel.com/
>>>
>>>
>>>>  
>>>> @@ -1840,6 +1858,8 @@ static int cqspi_resume(struct device *dev)
>>>>  	struct spi_master *master = dev_get_drvdata(dev);
>>>>  
>>>>  	clk_prepare_enable(cqspi->clk);
>>>> +	if (of_device_is_compatible(dev->of_node, "starfive,jh7110-qspi"))
>>>
>>> Don't add compatible checks inside the code. It does not scale. We
>>> expect compatibles to be listed only in one place - of_device_id - and
>>> customize driver with match data / quirks / flags.
>>>
>>> Comment applies to all your diff hunks.
>>>
>> I'll use "of_device_get_match_data" to replace it. But the way I added
>> reset before is also by compatible checks. Should I change this place to 
>> "of_device_get_match_data" as well?
> 
> I don't know what's there, but in general driver should be written in a
> consistent style.
>It's in line 1719, inside the "cqspi_probe", but this part of the code is
already merged in the main line. Should I keep it in a consistent style?

Best regards,
William
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 3/3] riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

[Aurelien Jarno]

Hi,

On 2023-07-04 17:04, William Qiu wrote:
> Add the quad spi controller node for the StarFive JH7110 SoC.
> 
> Co-developed-by: Ziv Xu <ziv.xu@starfivetech.com>
> Signed-off-by: Ziv Xu <ziv.xu@starfivetech.com>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-2.dtsi         | 32 +++++++++++++++++++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index 2a6d81609284..983b683e2f27 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -126,6 +126,38 @@ &i2c6 {
>  	status = "okay";
>  };
>  
> +&qspi {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	nor_flash: flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		cdns,read-delay = <5>;
> +		spi-max-frequency = <12000000>;
> +		cdns,tshsl-ns = <1>;
> +		cdns,tsd2d-ns = <1>;
> +		cdns,tchsh-ns = <1>;
> +		cdns,tslch-ns = <1>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			spl@0 {
> +				reg = <0x0 0x20000>;
> +			};
> +			uboot@100000 {
> +				reg = <0x100000 0x300000>;
> +			};
> +			data@f00000 {
> +				reg = <0xf00000 0x100000>;
> +			};

It appears that this uses the old layout for the SPI flash. The new
layout is described there:

https://doc-en.rvspace.org/VisionFive2/Boot_UG/JH7110_SDK/boot_address_allocation.html

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net

Re: [PATCH v4 3/3] riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

[William Qiu]

On 2023/7/18 1:13, Aurelien Jarno wrote:
> Hi,
> 
> On 2023-07-04 17:04, William Qiu wrote:
>> Add the quad spi controller node for the StarFive JH7110 SoC.
>> 
>> Co-developed-by: Ziv Xu <ziv.xu@starfivetech.com>
>> Signed-off-by: Ziv Xu <ziv.xu@starfivetech.com>
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
>> ---
>>  .../jh7110-starfive-visionfive-2.dtsi         | 32 +++++++++++++++++++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++++++++
>>  2 files changed, 50 insertions(+)
>> 
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index 2a6d81609284..983b683e2f27 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -126,6 +126,38 @@ &i2c6 {
>>  	status = "okay";
>>  };
>>  
>> +&qspi {
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	nor_flash: flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		cdns,read-delay = <5>;
>> +		spi-max-frequency = <12000000>;
>> +		cdns,tshsl-ns = <1>;
>> +		cdns,tsd2d-ns = <1>;
>> +		cdns,tchsh-ns = <1>;
>> +		cdns,tslch-ns = <1>;
>> +
>> +		partitions {
>> +			compatible = "fixed-partitions";
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			spl@0 {
>> +				reg = <0x0 0x20000>;
>> +			};
>> +			uboot@100000 {
>> +				reg = <0x100000 0x300000>;
>> +			};
>> +			data@f00000 {
>> +				reg = <0xf00000 0x100000>;
>> +			};
> 
> It appears that this uses the old layout for the SPI flash. The new
> layout is described there:
> 
> https://doc-en.rvspace.org/VisionFive2/Boot_UG/JH7110_SDK/boot_address_allocation.html
> 
> Regards
> Aurelien
> 
I'll take a look, and use it then.
Thanks for your comments.

Best regards,
William

Re: (subset) [PATCH v4 0/3] Add initialization of clock for StarFive JH7110 SoC

[Mark Brown]

On Tue, 04 Jul 2023 17:04:50 +0800, William Qiu wrote:
> This patchset adds initial rudimentary support for the StarFive
> Quad SPI controller driver. And this driver will be used in
> StarFive's VisionFive 2 board. In 6.4, the QSPI_AHB and QSPI_APB
> clocks changed from the default ON state to the default OFF state,
> so these clocks need to be enabled in the driver.At the same time,
> dts patch is added to this series.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/3] dt-bindings: qspi: cdns,qspi-nor: Add clocks for StarFive JH7110 SoC
      commit: 0d2b6a1b8515204924b9174ae0135e1f4ff29b21
[2/3] spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI
      commit: 33f1ef6d4eb6bca726608ed939c9fd94d96ceefd

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

[PATCH v4 0/3] Add initialization of clock for StarFive JH7110 SoC

[William Qiu]

Hi,

This patchset adds initial rudimentary support for the StarFive
Quad SPI controller driver. And this driver will be used in
StarFive's VisionFive 2 board. In 6.4, the QSPI_AHB and QSPI_APB
clocks changed from the default ON state to the default OFF state,
so these clocks need to be enabled in the driver.At the same time,
dts patch is added to this series.

Changes v3->v4:
- Added minItems for clocks.
- Added clock names property.
- Fixed formatting issues.

Changes v2->v3:
- Rebaed to v6.4rc6.
- Renamed the clock names.
- Changed the variable definition type.

Changes v1->v2:
- Renamed the clock names.
- Specified a different array of clocks.
- Used clk_bulk_ APIs.

The patch series is based on v6.4rc6.

William Qiu (3):
  dt-bindings: qspi: cdns,qspi-nor: Add clocks for StarFive JH7110 SoC
  spi: cadence-quadspi: Add clock configuration for StarFive JH7110 QSPI
  riscv: dts: starfive: Add QSPI controller node for StarFive JH7110 SoC

 .../bindings/spi/cdns,qspi-nor.yaml           | 12 ++++++-
 .../jh7110-starfive-visionfive-2.dtsi         | 32 +++++++++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 18 +++++++++++
 drivers/spi/spi-cadence-quadspi.c             | 20 ++++++++++++
 4 files changed, 81 insertions(+), 1 deletion(-)

--
2.34.1