| Message ID | 20230619083517.415597-2-william.qiu@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add initialization of clock for StarFive JH7110 SoC | expand |
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 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 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
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
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: