[PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Geert Uytterhoeven]

"make dt_binding_check":

    Documentation/devicetree/bindings/firmware/arm,scpi.example.dt.yaml: scpi: shmem: [[2], [1]] is too long

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Exposed by commit 39bd2b6a3783b899 ("dt-bindings: Improve phandle-array
schemas").
---
 Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
index 800417a76bda0bd9..3735f221fdf24e0a 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
@@ -51,6 +51,7 @@ properties:
       be any memory reserved for the purpose of this communication between the
       processors.
     minItems: 1
+    maxItems: 2
 
   power-controller:
     type: object
-- 
2.25.1

Re: [PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Krzysztof Kozlowski]

On 16/02/2022 14:21, Geert Uytterhoeven wrote:
> "make dt_binding_check":
> 
>     Documentation/devicetree/bindings/firmware/arm,scpi.example.dt.yaml: scpi: shmem: [[2], [1]] is too long
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Exposed by commit 39bd2b6a3783b899 ("dt-bindings: Improve phandle-array
> schemas").
> ---
>  Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> index 800417a76bda0bd9..3735f221fdf24e0a 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> @@ -51,6 +51,7 @@ properties:
>        be any memory reserved for the purpose of this communication between the
>        processors.
>      minItems: 1
> +    maxItems: 2

The same should be done to "mboxes" (I mean 39bd2b6a3783b899 and what
you are doing here) and the maxItems should match both, shouldn't it?


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Geert Uytterhoeven]

Hi Krzysztof,

On Wed, Feb 16, 2022 at 2:39 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
> On 16/02/2022 14:21, Geert Uytterhoeven wrote:
> > "make dt_binding_check":
> >
> >     Documentation/devicetree/bindings/firmware/arm,scpi.example.dt.yaml: scpi: shmem: [[2], [1]] is too long
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Exposed by commit 39bd2b6a3783b899 ("dt-bindings: Improve phandle-array
> > schemas").
> > ---
> >  Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> > index 800417a76bda0bd9..3735f221fdf24e0a 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> > @@ -51,6 +51,7 @@ properties:
> >        be any memory reserved for the purpose of this communication between the
> >        processors.
> >      minItems: 1
> > +    maxItems: 2
>
> The same should be done to "mboxes" (I mean 39bd2b6a3783b899 and what
> you are doing here)

Indeed, thanks!

> and the maxItems should match both, shouldn't it?

No idea, I'm not familiar with SCPI ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Krzysztof Kozlowski]

On 16/02/2022 14:42, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Wed, Feb 16, 2022 at 2:39 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
>> On 16/02/2022 14:21, Geert Uytterhoeven wrote:
>>> "make dt_binding_check":
>>>
>>>     Documentation/devicetree/bindings/firmware/arm,scpi.example.dt.yaml: scpi: shmem: [[2], [1]] is too long
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> Exposed by commit 39bd2b6a3783b899 ("dt-bindings: Improve phandle-array
>>> schemas").
>>> ---
>>>  Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
>>> index 800417a76bda0bd9..3735f221fdf24e0a 100644
>>> --- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
>>> +++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
>>> @@ -51,6 +51,7 @@ properties:
>>>        be any memory reserved for the purpose of this communication between the
>>>        processors.
>>>      minItems: 1
>>> +    maxItems: 2
>>
>> The same should be done to "mboxes" (I mean 39bd2b6a3783b899 and what
>> you are doing here)
> 
> Indeed, thanks!
> 
>> and the maxItems should match both, shouldn't it?
> 
> No idea, I'm not familiar with SCPI ;-)

Neither I am. The driver though counts number of mboxes and then loops
till that number to get each shmem device node. The driver does not
limit number of mboxes or shmem to 2, but will fail if they are not equal.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Sudeep Holla]

On Wed, Feb 16, 2022 at 02:21:43PM +0100, Geert Uytterhoeven wrote:
> "make dt_binding_check":
> 
>     Documentation/devicetree/bindings/firmware/arm,scpi.example.dt.yaml: scpi: shmem: [[2], [1]] is too long
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Exposed by commit 39bd2b6a3783b899 ("dt-bindings: Improve phandle-array
> schemas").

Interesting !

> ---
>  Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> index 800417a76bda0bd9..3735f221fdf24e0a 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> @@ -51,6 +51,7 @@ properties:
>        be any memory reserved for the purpose of this communication between the
>        processors.
>      minItems: 1
> +    maxItems: 2
>

There is no max limit strictly speaking. The driver can use all the specified
mboxes and associated shmem in round robin fashion. That is the reason I
didn't add maxItems unlike the newer SCMI protocol which clearly restricts
to one Tx and one Rx(much saner I must admit).

-- 
Regards,
Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Geert Uytterhoeven]

Hi Sudeep,

On Wed, Feb 16, 2022 at 3:40 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Wed, Feb 16, 2022 at 02:21:43PM +0100, Geert Uytterhoeven wrote:
> > "make dt_binding_check":
> >
> >     Documentation/devicetree/bindings/firmware/arm,scpi.example.dt.yaml: scpi: shmem: [[2], [1]] is too long
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Exposed by commit 39bd2b6a3783b899 ("dt-bindings: Improve phandle-array
> > schemas").
>
> Interesting !
>
> > ---
> >  Documentation/devicetree/bindings/firmware/arm,scpi.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> > index 800417a76bda0bd9..3735f221fdf24e0a 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scpi.yaml
> > @@ -51,6 +51,7 @@ properties:
> >        be any memory reserved for the purpose of this communication between the
> >        processors.
> >      minItems: 1
> > +    maxItems: 2
> >
>
> There is no max limit strictly speaking. The driver can use all the specified
> mboxes and associated shmem in round robin fashion. That is the reason I
> didn't add maxItems unlike the newer SCMI protocol which clearly restricts
> to one Tx and one Rx(much saner I must admit).

In the absence of maxItems, the validator assumes it is equal to minItems,
so we do need a sensible maxItems value here.
Any suggestions? 16? 64?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dt-bindings: firmware: arm,scpi: Add missing maxItems to shmem property

[Sudeep Holla]

Hi Geert,

On Wed, Feb 16, 2022 at 03:55:35PM +0100, Geert Uytterhoeven wrote:
> 
> In the absence of maxItems, the validator assumes it is equal to minItems,
> so we do need a sensible maxItems value here.

Ah OK, makes sense.

> Any suggestions? 16? 64?
>

I did a quick look and only 2 platforms use it(juno/amlogic meson).
Juno can go upto max of 62 theoretically but I don't think we we go beyond 4
due to associated shmem limitation. So maxitem of 4 should work just fine
for now. Since it is obsolete spec, I don't see any new extensions or users.

So you can add my ACK for the value of 4. 2 must work too but since I have
tested juno with 4, I would like to keep that possibility.

--
Regards,
Sudeep