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 v1->v2:
- 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 | 20 +++++++++++-
.../jh7110-starfive-visionfive-2.dtsi | 32 +++++++++++++++++++
arch/riscv/boot/dts/starfive/jh7110.dtsi | 18 +++++++++++
drivers/spi/spi-cadence-quadspi.c | 20 ++++++++++++
4 files changed, 89 insertions(+), 1 deletion(-)
--
2.34.1
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. Other
platforms do not have this constraint.
Signed-off-by: William Qiu <[email protected]>
Reviewed-by: Hal Feng <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
.../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
1 file changed, 19 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..1b83cbb9a086 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -26,6 +26,15 @@ allOf:
const: starfive,jh7110-qspi
then:
properties:
+ clocks:
+ maxItems: 3
+
+ clock-names:
+ items:
+ - const: ref
+ - const: ahb
+ - const: apb
+
resets:
minItems: 2
maxItems: 3
@@ -38,6 +47,9 @@ allOf:
else:
properties:
+ clocks:
+ maxItems: 1
+
resets:
maxItems: 2
@@ -70,7 +82,13 @@ properties:
maxItems: 1
clocks:
- maxItems: 1
+ maxItems: 3
+
+ clock-names:
+ items:
+ - const: ref
+ - const: ahb
+ - const: apb
cdns,fifo-depth:
description:
--
2.34.1
On Mon, 19 Jun 2023 16:35:15 +0800, William Qiu wrote:
> 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. Other
> platforms do not have this constraint.
>
> Signed-off-by: William Qiu <[email protected]>
> Reviewed-by: Hal Feng <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> ---
> .../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/cdns,qspi-nor.example.dtb: spi@ff705000: clocks: [[4294967295]] is too short
from schema $id: http://devicetree.org/schemas/spi/cdns,qspi-nor.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 19/06/2023 10:35, William Qiu wrote:
> 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. Other
> platforms do not have this constraint.
>
> Signed-off-by: William Qiu <[email protected]>
> Reviewed-by: Hal Feng <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> ---
> .../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
> 1 file changed, 19 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..1b83cbb9a086 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -26,6 +26,15 @@ allOf:
> const: starfive,jh7110-qspi
> then:
> properties:
> + clocks:
> + maxItems: 3
> +
> + clock-names:
> + items:
> + - const: ref
> + - const: ahb
> + - const: apb
You are duplicating top-level property. Define the items only in one
place. If this list is applicable to everything, then in top-level property.
> +
> resets:
> minItems: 2
> maxItems: 3
> @@ -38,6 +47,9 @@ allOf:
>
> else:
> properties:
> + clocks:
> + maxItems: 1
clock-names is missing. They must be in sync with clocks. What is the
first clock?
> +
> resets:
> maxItems: 2
>
> @@ -70,7 +82,13 @@ properties:
> maxItems: 1
>
> clocks:
> - maxItems: 1
> + maxItems: 3
You did not test it before sending. minItems is missing.
> +
> + clock-names:
> + items:
> + - const: ref
> + - const: ahb
> + - const: apb
>
> cdns,fifo-depth:
> description:
Best regards,
Krzysztof
On 2023/6/19 17:16, Rob Herring wrote:
>
> On Mon, 19 Jun 2023 16:35:15 +0800, William Qiu wrote:
>> 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. Other
>> platforms do not have this constraint.
>>
>> Signed-off-by: William Qiu <[email protected]>
>> Reviewed-by: Hal Feng <[email protected]>
>> Reviewed-by: Conor Dooley <[email protected]>
>> ---
>> .../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/cdns,qspi-nor.example.dtb: spi@ff705000: clocks: [[4294967295]] is too short
> from schema $id: http://devicetree.org/schemas/spi/cdns,qspi-nor.yaml#
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
It seems that my changes are not compatible with other dts files, I'll try to
fix it.
Thanks for reminding.
Best regards
William
On 2023/6/19 20:17, Krzysztof Kozlowski wrote:
> On 19/06/2023 10:35, William Qiu wrote:
>> 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. Other
>> platforms do not have this constraint.
>>
>> Signed-off-by: William Qiu <[email protected]>
>> Reviewed-by: Hal Feng <[email protected]>
>> Reviewed-by: Conor Dooley <[email protected]>
>> ---
>> .../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
>> 1 file changed, 19 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..1b83cbb9a086 100644
>> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> @@ -26,6 +26,15 @@ allOf:
>> const: starfive,jh7110-qspi
>> then:
>> properties:
>> + clocks:
>> + maxItems: 3
>> +
>> + clock-names:
>> + items:
>> + - const: ref
>> + - const: ahb
>> + - const: apb
>
> You are duplicating top-level property. Define the items only in one
> place. If this list is applicable to everything, then in top-level property.
>
Only in JH7110 SoC need there clocks, other platforms do not have this constraint.
So I need to duplicating top-level property.
>> +
>> resets:
>> minItems: 2
>> maxItems: 3
>> @@ -38,6 +47,9 @@ allOf:
>>
>> else:
>> properties:
>> + clocks:
>> + maxItems: 1
>
> clock-names is missing. They must be in sync with clocks. What is the
> first clock?
>
But there are no clock-names before, should I add it?
>> +
>> resets:
>> maxItems: 2
>>
>> @@ -70,7 +82,13 @@ properties:
>> maxItems: 1
>>
>> clocks:
>> - maxItems: 1
>> + maxItems: 3
>
>
> You did not test it before sending. minItems is missing.
>
I will add it.
As for other platforms, should I use enum to constraint the clocks?
>> +
>> + clock-names:
>> + items:
>> + - const: ref
>> + - const: ahb
>> + - const: apb
>
>
>>
>> cdns,fifo-depth:
>> description:
>
> Best regards,
> Krzysztof
>
Thanks for taking time to review this patches series.
Best regards,
William
On 21/06/2023 08:45, William Qiu wrote:
>
>
> On 2023/6/19 20:17, Krzysztof Kozlowski wrote:
>> On 19/06/2023 10:35, William Qiu wrote:
>>> 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. Other
>>> platforms do not have this constraint.
>>>
>>> Signed-off-by: William Qiu <[email protected]>
>>> Reviewed-by: Hal Feng <[email protected]>
>>> Reviewed-by: Conor Dooley <[email protected]>
>>> ---
>>> .../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
>>> 1 file changed, 19 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..1b83cbb9a086 100644
>>> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>>> @@ -26,6 +26,15 @@ allOf:
>>> const: starfive,jh7110-qspi
>>> then:
>>> properties:
>>> + clocks:
>>> + maxItems: 3
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: ref
>>> + - const: ahb
>>> + - const: apb
>>
>> You are duplicating top-level property. Define the items only in one
>> place. If this list is applicable to everything, then in top-level property.
>>
> Only in JH7110 SoC need there clocks, other platforms do not have this constraint.
> So I need to duplicating top-level property.
You don't need, why? Why writing something twice is an answer to "JH7110
needs 3 clocks"? It's not related.
What is the clock for all other variants?
>>> +
>>> resets:
>>> minItems: 2
>>> maxItems: 3
>>> @@ -38,6 +47,9 @@ allOf:
>>>
>>> else:
>>> properties:
>>> + clocks:
>>> + maxItems: 1
>>
>> clock-names is missing. They must be in sync with clocks. What is the
>> first clock?
>>
> But there are no clock-names before, should I add it?
Then let's just disallow it. Either you define it or you not allow it.
>>> +
>>> resets:
>>> maxItems: 2
>>>
>>> @@ -70,7 +82,13 @@ properties:
>>> maxItems: 1
>>>
>>> clocks:
>>> - maxItems: 1
>>> + maxItems: 3
>>
>>
>> You did not test it before sending. minItems is missing.
>>
> I will add it.
> As for other platforms, should I use enum to constraint the clocks?
What is the clock on other platforms?
Best regards,
Krzysztof
On 2023/6/21 16:10, Krzysztof Kozlowski wrote:
> On 21/06/2023 08:45, William Qiu wrote:
>>
>>
>> On 2023/6/19 20:17, Krzysztof Kozlowski wrote:
>>> On 19/06/2023 10:35, William Qiu wrote:
>>>> 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. Other
>>>> platforms do not have this constraint.
>>>>
>>>> Signed-off-by: William Qiu <[email protected]>
>>>> Reviewed-by: Hal Feng <[email protected]>
>>>> Reviewed-by: Conor Dooley <[email protected]>
>>>> ---
>>>> .../bindings/spi/cdns,qspi-nor.yaml | 20 ++++++++++++++++++-
>>>> 1 file changed, 19 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..1b83cbb9a086 100644
>>>> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>>>> @@ -26,6 +26,15 @@ allOf:
>>>> const: starfive,jh7110-qspi
>>>> then:
>>>> properties:
>>>> + clocks:
>>>> + maxItems: 3
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: ref
>>>> + - const: ahb
>>>> + - const: apb
>>>
>>> You are duplicating top-level property. Define the items only in one
>>> place. If this list is applicable to everything, then in top-level property.
>>>
>> Only in JH7110 SoC need there clocks, other platforms do not have this constraint.
>> So I need to duplicating top-level property.
>
> You don't need, why? Why writing something twice is an answer to "JH7110
> needs 3 clocks"? It's not related.
>
> What is the clock for all other variants?
>
I'll try to not duplicating top-level property.
>>>> +
>>>> resets:
>>>> minItems: 2
>>>> maxItems: 3
>>>> @@ -38,6 +47,9 @@ allOf:
>>>>
>>>> else:
>>>> properties:
>>>> + clocks:
>>>> + maxItems: 1
>>>
>>> clock-names is missing. They must be in sync with clocks. What is the
>>> first clock?
>>>
>> But there are no clock-names before, should I add it?
>
> Then let's just disallow it. Either you define it or you not allow it.
>
Fine, I'll keep it disallow.
>>>> +
>>>> resets:
>>>> maxItems: 2
>>>>
>>>> @@ -70,7 +82,13 @@ properties:
>>>> maxItems: 1
>>>>
>>>> clocks:
>>>> - maxItems: 1
>>>> + maxItems: 3
>>>
>>>
>>> You did not test it before sending. minItems is missing.
>>>
>> I will add it.
>> As for other platforms, should I use enum to constraint the clocks?
>
> What is the clock on other platforms?
>
Other platforms have only one clock.
> Best regards,
> Krzysztof
>
Thanks for taking time to review this patch series and give usefull
suggestions.
Best Regards,
William