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 <[email protected]>
---
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
opp-microvolt:
description: |
--
2.43.0
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 <[email protected]>
> ---
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
>>>> ---
>>>> 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
--
David Heidelberg
On Mon, Mar 4, 2024 at 2:34 PM David Heidelberg <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> 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