2022-04-30 06:49:34

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
required, the other two are optional. Fix the dt-binding so that the
optional regions can be omitted and passed in any order.

Also add the missing minItems to the reg property and update the
description.

Signed-off-by: Nícolas F. R. A. Prado <[email protected]>

---

.../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index 823a236242de..ec9ddeb6ca2c 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -24,14 +24,20 @@ properties:
reg:
description:
Should contain the address ranges for memory regions SRAM, CFG, and
- L1TCM.
+ L1TCM. Only SRAM is required, while CFG and L1TCM are optional.
+ minItems: 1
maxItems: 3

reg-names:
+ minItems: 1
items:
- const: sram
- - const: cfg
- - const: l1tcm
+ - enum:
+ - l1tcm
+ - cfg
+ - enum:
+ - l1tcm
+ - cfg

clocks:
description:
--
2.36.0


2022-05-03 00:41:36

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

On Mon, May 02, 2022 at 10:33:29AM -0500, Rob Herring wrote:
> On Fri, 29 Apr 2022 17:11:09 -0400, N?colas F. R. A. Prado wrote:
> > The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
> > required, the other two are optional. Fix the dt-binding so that the
> > optional regions can be omitted and passed in any order.
> >
> > Also add the missing minItems to the reg property and update the
> > description.
> >
> > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> >
> > ---
> >
> > .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
>
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.

Hi Rob,

These indeed aren't new warnings. But in any case, the fix for the interrupts
one is already on its way to mainline [1]. And the memory-region one is what is
fixed by patch 2 in this series.

Thanks,
N?colas

[1] https://lore.kernel.org/all/[email protected]/

>
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>
> Full log is available here: https://patchwork.ozlabs.org/patch/
>
>
> scp@10500000: interrupts: [[0, 174, 4]] is not of type 'object'
> arch/arm64/boot/dts/mediatek/mt8183-evb.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-cozmo.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-damu.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14-sku2.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku1.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku6.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku7.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-juniper-sku16.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kappa.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kenzo.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku0.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku1.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu-sku22.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku16.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku272.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku288.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku32.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku0.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dtb
> arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dtb
>
> scp@10500000: memory-region: [[25]] is not of type 'object'
> arch/arm64/boot/dts/mediatek/mt8183-evb.dtb
>
> scp@10500000: memory-region: [[27]] is not of type 'object'
> arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dtb
>
> scp@10500000: memory-region: [[28]] is not of type 'object'
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-cozmo.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-damu.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14-sku2.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku1.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku6.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku7.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-juniper-sku16.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kappa.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kenzo.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku0.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku1.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu-sku22.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku16.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku272.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku288.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku32.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku0.dtb
> arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dtb
>
>
> --
> To unsubscribe, send mail to [email protected].

2022-05-03 00:57:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

On Fri, 29 Apr 2022 17:11:09 -0400, Nícolas F. R. A. Prado wrote:
> The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
> required, the other two are optional. Fix the dt-binding so that the
> optional regions can be omitted and passed in any order.
>
> Also add the missing minItems to the reg property and update the
> description.
>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>
> ---
>
> .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


scp@10500000: interrupts: [[0, 174, 4]] is not of type 'object'
arch/arm64/boot/dts/mediatek/mt8183-evb.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-cozmo.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-damu.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14-sku2.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku1.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku6.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku7.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-juniper-sku16.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kappa.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kenzo.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku0.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku1.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu-sku22.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku16.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku272.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku288.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku32.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku0.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dtb
arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dtb

scp@10500000: memory-region: [[25]] is not of type 'object'
arch/arm64/boot/dts/mediatek/mt8183-evb.dtb

scp@10500000: memory-region: [[27]] is not of type 'object'
arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dtb

scp@10500000: memory-region: [[28]] is not of type 'object'
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-burnet.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-cozmo.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-damu.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel14-sku2.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku1.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku6.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-fennel-sku7.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-juniper-sku16.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kappa.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-kenzo.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku0.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi-willow-sku1.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kakadu-sku22.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku16.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku272.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku288.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-kodama-sku32.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku0.dtb
arch/arm64/boot/dts/mediatek/mt8183-kukui-krane-sku176.dtb

Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

Il 03/05/22 14:19, Krzysztof Kozlowski ha scritto:
> On 29/04/2022 23:11, Nícolas F. R. A. Prado wrote:
>> The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
>> required, the other two are optional. Fix the dt-binding so that the
>> optional regions can be omitted and passed in any order.
>
> No, cannot be passed in any order.
>
>>
>> Also add the missing minItems to the reg property and update the
>> description.
>>
>> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>>
>> ---
>>
>> .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>> index 823a236242de..ec9ddeb6ca2c 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
>> @@ -24,14 +24,20 @@ properties:
>> reg:
>> description:
>> Should contain the address ranges for memory regions SRAM, CFG, and
>> - L1TCM.
>> + L1TCM. Only SRAM is required, while CFG and L1TCM are optional.
>> + minItems: 1
>> maxItems: 3
>>
>> reg-names:
>> + minItems: 1
>> items:
>> - const: sram
>> - - const: cfg
>> - - const: l1tcm
>> + - enum:
>> + - l1tcm
>> + - cfg
>> + - enum:
>> + - l1tcm
>> + - cfg
>
> This allows them in any combination which is not what we want. If both
> are optional and both can appear, then last should be a const:l1tcm.
>
> Best regards,
> Krzysztof

Nicolas, I think that you weren't clear about what you're trying to solve with this
commit in the description.

I remember you had this kind of instance, but I don't really remember if it was
about mtk,scp or (and?) something else.... so.... are you trying to fix issues
with devicetrees declaring

reg-names = "sram", "l1tcm"; ?

Was this giving dtbs_check errors?

Cheers,
Angelo

2022-05-04 01:18:37

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

On Tue, May 03, 2022 at 02:19:39PM +0200, Krzysztof Kozlowski wrote:
> On 29/04/2022 23:11, N?colas F. R. A. Prado wrote:
> > The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
> > required, the other two are optional. Fix the dt-binding so that the
> > optional regions can be omitted and passed in any order.
>
> No, cannot be passed in any order.
>
> >
> > Also add the missing minItems to the reg property and update the
> > description.
> >
> > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> >
> > ---
> >
> > .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > index 823a236242de..ec9ddeb6ca2c 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > @@ -24,14 +24,20 @@ properties:
> > reg:
> > description:
> > Should contain the address ranges for memory regions SRAM, CFG, and
> > - L1TCM.
> > + L1TCM. Only SRAM is required, while CFG and L1TCM are optional.
> > + minItems: 1
> > maxItems: 3
> >
> > reg-names:
> > + minItems: 1
> > items:
> > - const: sram
> > - - const: cfg
> > - - const: l1tcm
> > + - enum:
> > + - l1tcm
> > + - cfg
> > + - enum:
> > + - l1tcm
> > + - cfg
>
> This allows them in any combination which is not what we want. If both
> are optional and both can appear, then last should be a const:l1tcm.

Hi Krzysztof,

yes, that's the case. I'll update it in v3 then.

Thanks,
N?colas

>
> Best regards,
> Krzysztof

2022-05-04 01:56:49

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

On Tue, May 03, 2022 at 02:26:15PM +0200, AngeloGioacchino Del Regno wrote:
> Il 03/05/22 14:19, Krzysztof Kozlowski ha scritto:
> > On 29/04/2022 23:11, N?colas F. R. A. Prado wrote:
> > > The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
> > > required, the other two are optional. Fix the dt-binding so that the
> > > optional regions can be omitted and passed in any order.
> >
> > No, cannot be passed in any order.
> >
> > >
> > > Also add the missing minItems to the reg property and update the
> > > description.
> > >
> > > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> > >
> > > ---
> > >
> > > .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
> > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > > index 823a236242de..ec9ddeb6ca2c 100644
> > > --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > > +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > > @@ -24,14 +24,20 @@ properties:
> > > reg:
> > > description:
> > > Should contain the address ranges for memory regions SRAM, CFG, and
> > > - L1TCM.
> > > + L1TCM. Only SRAM is required, while CFG and L1TCM are optional.
> > > + minItems: 1
> > > maxItems: 3
> > > reg-names:
> > > + minItems: 1
> > > items:
> > > - const: sram
> > > - - const: cfg
> > > - - const: l1tcm
> > > + - enum:
> > > + - l1tcm
> > > + - cfg
> > > + - enum:
> > > + - l1tcm
> > > + - cfg
> >
> > This allows them in any combination which is not what we want. If both
> > are optional and both can appear, then last should be a const:l1tcm.
> >
> > Best regards,
> > Krzysztof
>
> Nicolas, I think that you weren't clear about what you're trying to solve with this
> commit in the description.
>
> I remember you had this kind of instance, but I don't really remember if it was
> about mtk,scp or (and?) something else.... so.... are you trying to fix issues
> with devicetrees declaring
>
> reg-names = "sram", "l1tcm"; ?
>
> Was this giving dtbs_check errors?

Hi Angelo,

yes, some devicetrees (like mt8183) have

reg-names = "sram", "cfg";

I'll include mention of this in the commit description for v3.

This was also supposed to fix the warning for mt8192, where the order is
different: "sram", "l1tcm", "cfg". But since Krzysztof said that we want a fixed
order, then the mt8192 DT will need to be updated. (I also just noticed that
it's just l1tcm that is optional, I was just being blind, so I'll also fix that
in v3)

Thanks,
N?colas

>
> Cheers,
> Angelo

2022-05-04 06:25:02

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

On 29/04/2022 23:11, Nícolas F. R. A. Prado wrote:
> The SCP has three memory regions: sram, l1tcm and cfg. Only sram is
> required, the other two are optional. Fix the dt-binding so that the
> optional regions can be omitted and passed in any order.

No, cannot be passed in any order.

>
> Also add the missing minItems to the reg property and update the
> description.
>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
>
> ---
>
> .../devicetree/bindings/remoteproc/mtk,scp.yaml | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 823a236242de..ec9ddeb6ca2c 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -24,14 +24,20 @@ properties:
> reg:
> description:
> Should contain the address ranges for memory regions SRAM, CFG, and
> - L1TCM.
> + L1TCM. Only SRAM is required, while CFG and L1TCM are optional.
> + minItems: 1
> maxItems: 3
>
> reg-names:
> + minItems: 1
> items:
> - const: sram
> - - const: cfg
> - - const: l1tcm
> + - enum:
> + - l1tcm
> + - cfg
> + - enum:
> + - l1tcm
> + - cfg

This allows them in any combination which is not what we want. If both
are optional and both can appear, then last should be a const:l1tcm.

Best regards,
Krzysztof

2022-05-07 06:51:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: Fix optional reg-names for mtk,scp

On 03/05/2022 22:41, Nícolas F. R. A. Prado wrote:
>
> This was also supposed to fix the warning for mt8192, where the order is
> different: "sram", "l1tcm", "cfg". But since Krzysztof said that we want a fixed
> order, then the mt8192 DT will need to be updated.

Yes, please update DTS.


Best regards,
Krzysztof