| Message ID | f6d1ea27e8b8dc47fbe849661cc5a843bc2f1ba5.1645017656.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: firmware: arm, scpi: Add missing maxItems to shmem property | expand |
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
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
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
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).
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
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
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
"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(+)