| Message ID | 20231229191038.247258-1-david@ixit.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dt-bindings: opp: switch inner and outer min/maxItems rules for opp-hz | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 12 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 29/12/2023 20:10, David Heidelberg wrote: > Fixes issue as: > ``` Drop, it's not RST, but commit msg. > arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb: opp-table: opp-200000000:opp-hz:0: [200000000, 0, 0, 150000000, 0, 0, 0, 0, 300000000] is too long > ``` > > Fixes: 3cb16ad69bef ("dt-bindings: opp: accept array of frequencies") > > Signed-off-by: David Heidelberg <david@ixit.cz> > --- > Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > index e2f8f7af3cf4..86d3aa0eb435 100644 > --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > @@ -55,10 +55,9 @@ patternProperties: > to relate the values to their clocks or the order in which the clocks > need to be configured and that is left for the implementation > specific binding. > - minItems: 1 > - maxItems: 32 > items: > - maxItems: 1 > + minItems: 1 > + maxItems: 32 This does not look like correct fix. The original code looked fine - only one item is allowed in each sub-element (array). Best regards, Krzysztof
On Sat, Dec 30, 2023 at 03:17:21PM +0100, Krzysztof Kozlowski wrote: > On 29/12/2023 20:10, David Heidelberg wrote: > > Fixes issue as: > > ``` > > Drop, it's not RST, but commit msg. > > > arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb: opp-table: opp-200000000:opp-hz:0: [200000000, 0, 0, 150000000, 0, 0, 0, 0, 300000000] is too long > > ``` > > > > Fixes: 3cb16ad69bef ("dt-bindings: opp: accept array of frequencies") > > > > Signed-off-by: David Heidelberg <david@ixit.cz> > > --- > > Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > > index e2f8f7af3cf4..86d3aa0eb435 100644 > > --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > > +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > > @@ -55,10 +55,9 @@ patternProperties: > > to relate the values to their clocks or the order in which the clocks > > need to be configured and that is left for the implementation > > specific binding. > > - minItems: 1 > > - maxItems: 32 > > items: > > - maxItems: 1 > > + minItems: 1 > > + maxItems: 32 > > This does not look like correct fix. The original code looked fine - > only one item is allowed in each sub-element (array). This one is special being 64-bit values so we have an exception in property-units.yaml. The constraints here don't get used in decoding the dtb and the default way of 1 outer element is used. It doesn't look like opp-hz needs to be a matrix as it is really just an array. Perhaps it should just be changed to an array type. Alternatively, adding 'items: { maxItems: 1 }' to the definition in property-units.yaml fixes the issue as well. Though we can fix this, I'm looking into if we have other cases where we need this to work as-is. There's probably some room for improvement in how matrix dimensions are handled. Rob
On Tue, Jan 02, 2024 at 04:58:15PM -0700, Rob Herring wrote: > On Sat, Dec 30, 2023 at 03:17:21PM +0100, Krzysztof Kozlowski wrote: > > On 29/12/2023 20:10, David Heidelberg wrote: > > > Fixes issue as: > > > ``` > > > > Drop, it's not RST, but commit msg. > > > > > arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb: opp-table: opp-200000000:opp-hz:0: [200000000, 0, 0, 150000000, 0, 0, 0, 0, 300000000] is too long > > > ``` > > > > > > Fixes: 3cb16ad69bef ("dt-bindings: opp: accept array of frequencies") > > > > > > Signed-off-by: David Heidelberg <david@ixit.cz> > > > --- > > > Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 5 ++--- > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > > > index e2f8f7af3cf4..86d3aa0eb435 100644 > > > --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > > > +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > > > @@ -55,10 +55,9 @@ patternProperties: > > > to relate the values to their clocks or the order in which the clocks > > > need to be configured and that is left for the implementation > > > specific binding. > > > - minItems: 1 > > > - maxItems: 32 > > > items: > > > - maxItems: 1 > > > + minItems: 1 > > > + maxItems: 32 > > > > This does not look like correct fix. The original code looked fine - > > only one item is allowed in each sub-element (array). > > This one is special being 64-bit values so we have an exception in > property-units.yaml. The constraints here don't get used in decoding the > dtb and the default way of 1 outer element is used. > > It doesn't look like opp-hz needs to be a matrix as it is really just an > array. Perhaps it should just be changed to an array type. > Alternatively, adding 'items: { maxItems: 1 }' to the definition in > property-units.yaml fixes the issue as well. > > Though we can fix this, I'm looking into if we have other cases where we > need this to work as-is. There's probably some room for improvement in > how matrix dimensions are handled. I've made some improvements on matrix dimensions, but this one is still an issue. Can you respin this dropping 'items: {maxItems: 1}'. I'm going to change the definition in property-units.yaml to uint64-array. Rob
On 30/01/2024 18:06, Rob Herring wrote: > On Tue, Jan 02, 2024 at 04:58:15PM -0700, Rob Herring wrote: >> On Sat, Dec 30, 2023 at 03:17:21PM +0100, Krzysztof Kozlowski wrote: >>> On 29/12/2023 20:10, David Heidelberg wrote: >>>> Fixes issue as: >>>> ``` >>> Drop, it's not RST, but commit msg. >>> >>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb: opp-table: opp-200000000:opp-hz:0: [200000000, 0, 0, 150000000, 0, 0, 0, 0, 300000000] is too long >>>> ``` >>>> >>>> Fixes: 3cb16ad69bef ("dt-bindings: opp: accept array of frequencies") >>>> >>>> Signed-off-by: David Heidelberg <david@ixit.cz> >>>> --- >>>> Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 5 ++--- >>>> 1 file changed, 2 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml >>>> index e2f8f7af3cf4..86d3aa0eb435 100644 >>>> --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml >>>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml >>>> @@ -55,10 +55,9 @@ patternProperties: >>>> to relate the values to their clocks or the order in which the clocks >>>> need to be configured and that is left for the implementation >>>> specific binding. >>>> - minItems: 1 >>>> - maxItems: 32 >>>> items: >>>> - maxItems: 1 >>>> + minItems: 1 >>>> + maxItems: 32 >>> This does not look like correct fix. The original code looked fine - >>> only one item is allowed in each sub-element (array). >> This one is special being 64-bit values so we have an exception in >> property-units.yaml. The constraints here don't get used in decoding the >> dtb and the default way of 1 outer element is used. >> >> It doesn't look like opp-hz needs to be a matrix as it is really just an >> array. Perhaps it should just be changed to an array type. >> Alternatively, adding 'items: { maxItems: 1 }' to the definition in >> property-units.yaml fixes the issue as well. >> >> Though we can fix this, I'm looking into if we have other cases where we >> need this to work as-is. There's probably some room for improvement in >> how matrix dimensions are handled. > I've made some improvements on matrix dimensions, but this one is still > an issue. Can you respin this dropping 'items: {maxItems: 1}'. I'm going > to change the definition in property-units.yaml to uint64-array. Keeping the rest of my changes still generates warnings (today dt-schema git) even with `maxItems` dropped. The only working scenario is when I do only the dropping of `items: {maxItems: 1}` from the original code. Is it the standalone change of just dropping this what did you desired? If yes, I have the patch prepared. David > > Rob
On Mon, Mar 4, 2024 at 2:34 PM David Heidelberg <david@ixit.cz> wrote: > > On 30/01/2024 18:06, Rob Herring wrote: > > On Tue, Jan 02, 2024 at 04:58:15PM -0700, Rob Herring wrote: > >> On Sat, Dec 30, 2023 at 03:17:21PM +0100, Krzysztof Kozlowski wrote: > >>> On 29/12/2023 20:10, David Heidelberg wrote: > >>>> Fixes issue as: > >>>> ``` > >>> Drop, it's not RST, but commit msg. > >>> > >>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb: opp-table: opp-200000000:opp-hz:0: [200000000, 0, 0, 150000000, 0, 0, 0, 0, 300000000] is too long > >>>> ``` > >>>> > >>>> Fixes: 3cb16ad69bef ("dt-bindings: opp: accept array of frequencies") > >>>> > >>>> Signed-off-by: David Heidelberg <david@ixit.cz> > >>>> --- > >>>> Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 5 ++--- > >>>> 1 file changed, 2 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > >>>> index e2f8f7af3cf4..86d3aa0eb435 100644 > >>>> --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > >>>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml > >>>> @@ -55,10 +55,9 @@ patternProperties: > >>>> to relate the values to their clocks or the order in which the clocks > >>>> need to be configured and that is left for the implementation > >>>> specific binding. > >>>> - minItems: 1 > >>>> - maxItems: 32 > >>>> items: > >>>> - maxItems: 1 > >>>> + minItems: 1 > >>>> + maxItems: 32 > >>> This does not look like correct fix. The original code looked fine - > >>> only one item is allowed in each sub-element (array). > >> This one is special being 64-bit values so we have an exception in > >> property-units.yaml. The constraints here don't get used in decoding the > >> dtb and the default way of 1 outer element is used. > >> > >> It doesn't look like opp-hz needs to be a matrix as it is really just an > >> array. Perhaps it should just be changed to an array type. > >> Alternatively, adding 'items: { maxItems: 1 }' to the definition in > >> property-units.yaml fixes the issue as well. > >> > >> Though we can fix this, I'm looking into if we have other cases where we > >> need this to work as-is. There's probably some room for improvement in > >> how matrix dimensions are handled. > > I've made some improvements on matrix dimensions, but this one is still > > an issue. Can you respin this dropping 'items: {maxItems: 1}'. I'm going > > to change the definition in property-units.yaml to uint64-array. > > Keeping the rest of my changes still generates warnings (today dt-schema > git) even with `maxItems` dropped. > > The only working scenario is when I do only the dropping of `items: > {maxItems: 1}` from the original code. > > Is it the standalone change of just dropping this what did you desired? > If yes, I have the patch prepared. Yes. I still need to look at changing the type, but that shouldn't hold up the kernel change. Rob
diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml index e2f8f7af3cf4..86d3aa0eb435 100644 --- a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml +++ b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml @@ -55,10 +55,9 @@ patternProperties: to relate the values to their clocks or the order in which the clocks need to be configured and that is left for the implementation specific binding. - minItems: 1 - maxItems: 32 items: - maxItems: 1 + minItems: 1 + maxItems: 32 opp-microvolt: description: |
Fixes issue as: ``` arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb: opp-table: opp-200000000:opp-hz:0: [200000000, 0, 0, 150000000, 0, 0, 0, 0, 300000000] is too long ``` Fixes: 3cb16ad69bef ("dt-bindings: opp: accept array of frequencies") Signed-off-by: David Heidelberg <david@ixit.cz> --- Documentation/devicetree/bindings/opp/opp-v2-base.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)