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 <william.qiu@starfivetech.com>
Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
.../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.1On 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 <william.qiu@starfivetech.com> > Reviewed-by: Hal Feng <hal.feng@starfivetech.com> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > --- > .../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 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 <william.qiu@starfivetech.com> >> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> --- >> .../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 <william.qiu@starfivetech.com> >>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >>> --- >>> .../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 <william.qiu@starfivetech.com> >>>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >>>> --- >>>> .../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
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 <william.qiu@starfivetech.com> > Reviewed-by: Hal Feng <hal.feng@starfivetech.com> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > --- > .../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/20230619083517.415597-2-william.qiu@starfivetech.com 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 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 <william.qiu@starfivetech.com> >> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> --- >> .../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/20230619083517.415597-2-william.qiu@starfivetech.com > > 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
© 2016 - 2026 Red Hat, Inc.