2022-01-21 19:25:58

by Arnaud Pouliquen

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: Improve phandle-array schemas

Hello Rob,

On 1/19/22 2:50 AM, Rob Herring wrote:
> The 'phandle-array' type is a bit ambiguous. It can be either just an
> array of phandles or an array of phandles plus args. Many schemas for
> phandle-array properties aren't clear in the schema which case applies
> though the description usually describes it.
>
> The array of phandles case boils down to needing:
>
> items:
> maxItems: 1
>
> The phandle plus args cases should typically take this form:
>
> items:
> - items:
> - description: A phandle
> - description: 1st arg cell
> - description: 2nd arg cell
>
> With this change, some examples need updating so that the bracketing of
> property values matches the schema.
>
> Cc: Damien Le Moal <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Chun-Kuang Hu <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Kieran Bingham <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: Georgi Djakov <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Lee Jones <[email protected]>
> Cc: Daniel Thompson <[email protected]>
> Cc: Jingoo Han <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Wolfgang Grandegger <[email protected]>
> Cc: Marc Kleine-Budde <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Vivien Didelot <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: Vladimir Oltean <[email protected]>
> Cc: Kalle Valo <[email protected]>
> Cc: Viresh Kumar <[email protected]>
> Cc: Stephen Boyd <[email protected]>
> Cc: Kishon Vijay Abraham I <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Kevin Hilman <[email protected]>
> Cc: Ulf Hansson <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Mathieu Poirier <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: Zhang Rui <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Jonathan Hunter <[email protected]>
> Cc: Sudeep Holla <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Rob Herring <[email protected]>
> ---

[...]

> .../bindings/remoteproc/st,stm32-rproc.yaml | 33 ++++++--

[...]

> diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
> index b587c97c282b..be3d9b0e876b 100644
> --- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
> @@ -29,17 +29,22 @@ properties:
>
> st,syscfg-holdboot:
> description: remote processor reset hold boot
> - - Phandle of syscon block.
> - - The offset of the hold boot setting register.
> - - The field mask of the hold boot.
> $ref: "/schemas/types.yaml#/definitions/phandle-array"
> - maxItems: 1
> + items:
> + - items:
> + - description: Phandle of syscon block
> + - description: The offset of the hold boot setting register
> + - description: The field mask of the hold boot
>
> st,syscfg-tz:
> description:
> Reference to the system configuration which holds the RCC trust zone mode
> $ref: "/schemas/types.yaml#/definitions/phandle-array"
> - maxItems: 1
> + items:
> + - items:
> + - description: Phandle of syscon block
> + - description: FIXME
> + - description: FIXME

- description: The offset of the trust zone setting register
- description: The field mask of the trust zone state

>
> interrupts:
> description: Should contain the WWDG1 watchdog reset interrupt
> @@ -93,20 +98,32 @@ properties:
> $ref: "/schemas/types.yaml#/definitions/phandle-array"
> description: |
> Reference to the system configuration which holds the remote
> - maxItems: 1
> + items:
> + - items:
> + - description: Phandle of syscon block
> + - description: FIXME
> + - description: FIXME

- description: The offset of the power setting register
- description: The field mask of the PDDS selection

>
> st,syscfg-m4-state:
> $ref: "/schemas/types.yaml#/definitions/phandle-array"
> description: |
> Reference to the tamp register which exposes the Cortex-M4 state.
> - maxItems: 1
> + items:
> + - items:
> + - description: Phandle of syscon block with the tamp register
> + - description: FIXME
> + - description: FIXME

- description: The offset of the tamp register
- description: The field mask of the Cortex-M4 state

>
> st,syscfg-rsc-tbl:
> $ref: "/schemas/types.yaml#/definitions/phandle-array"
> description: |
> Reference to the tamp register which references the Cortex-M4
> resource table address.
> - maxItems: 1
> + items:
> + - items:
> + - description: Phandle of syscon block with the tamp register
> + - description: FIXME
> + - description: FIXME

- description: The offset of the tamp register
- description: The field mask of the Cortex-M4 resource table address

Please tell me if you prefer that I fix this in a dedicated patch.

Thanks,
Arnaud


2022-01-21 19:29:10

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: Improve phandle-array schemas

On Wed, Jan 19, 2022 at 9:22 AM Arnaud POULIQUEN
<[email protected]> wrote:
>
> Hello Rob,
>
> On 1/19/22 2:50 AM, Rob Herring wrote:
> > The 'phandle-array' type is a bit ambiguous. It can be either just an
> > array of phandles or an array of phandles plus args. Many schemas for
> > phandle-array properties aren't clear in the schema which case applies
> > though the description usually describes it.
> >
> > The array of phandles case boils down to needing:
> >
> > items:
> > maxItems: 1
> >
> > The phandle plus args cases should typically take this form:
> >
> > items:
> > - items:
> > - description: A phandle
> > - description: 1st arg cell
> > - description: 2nd arg cell
> >
> > With this change, some examples need updating so that the bracketing of
> > property values matches the schema.
> >
> > Cc: Damien Le Moal <[email protected]>
> > Cc: Herbert Xu <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Cc: Chun-Kuang Hu <[email protected]>
> > Cc: Philipp Zabel <[email protected]>
> > Cc: Laurent Pinchart <[email protected]>
> > Cc: Kieran Bingham <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Cc: Georgi Djakov <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Marc Zyngier <[email protected]>
> > Cc: Joerg Roedel <[email protected]>
> > Cc: Lee Jones <[email protected]>
> > Cc: Daniel Thompson <[email protected]>
> > Cc: Jingoo Han <[email protected]>
> > Cc: Pavel Machek <[email protected]>
> > Cc: Mauro Carvalho Chehab <[email protected]>
> > Cc: Krzysztof Kozlowski <[email protected]>
> > Cc: Jakub Kicinski <[email protected]>
> > Cc: Wolfgang Grandegger <[email protected]>
> > Cc: Marc Kleine-Budde <[email protected]>
> > Cc: Andrew Lunn <[email protected]>
> > Cc: Vivien Didelot <[email protected]>
> > Cc: Florian Fainelli <[email protected]>
> > Cc: Vladimir Oltean <[email protected]>
> > Cc: Kalle Valo <[email protected]>
> > Cc: Viresh Kumar <[email protected]>
> > Cc: Stephen Boyd <[email protected]>
> > Cc: Kishon Vijay Abraham I <[email protected]>
> > Cc: Linus Walleij <[email protected]>
> > Cc: "Rafael J. Wysocki" <[email protected]>
> > Cc: Kevin Hilman <[email protected]>
> > Cc: Ulf Hansson <[email protected]>
> > Cc: Sebastian Reichel <[email protected]>
> > Cc: Mark Brown <[email protected]>
> > Cc: Mathieu Poirier <[email protected]>
> > Cc: Daniel Lezcano <[email protected]>
> > Cc: Zhang Rui <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Thierry Reding <[email protected]>
> > Cc: Jonathan Hunter <[email protected]>
> > Cc: Sudeep Holla <[email protected]>
> > Cc: Geert Uytterhoeven <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Rob Herring <[email protected]>
> > ---
>
> [...]
>
> > .../bindings/remoteproc/st,stm32-rproc.yaml | 33 ++++++--
>
> [...]
>
> > diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
> > index b587c97c282b..be3d9b0e876b 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
> > @@ -29,17 +29,22 @@ properties:
> >
> > st,syscfg-holdboot:
> > description: remote processor reset hold boot
> > - - Phandle of syscon block.
> > - - The offset of the hold boot setting register.
> > - - The field mask of the hold boot.
> > $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > - maxItems: 1
> > + items:
> > + - items:
> > + - description: Phandle of syscon block
> > + - description: The offset of the hold boot setting register
> > + - description: The field mask of the hold boot
> >
> > st,syscfg-tz:
> > description:
> > Reference to the system configuration which holds the RCC trust zone mode
> > $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > - maxItems: 1
> > + items:
> > + - items:
> > + - description: Phandle of syscon block
> > + - description: FIXME
> > + - description: FIXME
>
> - description: The offset of the trust zone setting register
> - description: The field mask of the trust zone state
>
> >
> > interrupts:
> > description: Should contain the WWDG1 watchdog reset interrupt
> > @@ -93,20 +98,32 @@ properties:
> > $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > description: |
> > Reference to the system configuration which holds the remote
> > - maxItems: 1
> > + items:
> > + - items:
> > + - description: Phandle of syscon block
> > + - description: FIXME
> > + - description: FIXME
>
> - description: The offset of the power setting register
> - description: The field mask of the PDDS selection
>
> >
> > st,syscfg-m4-state:
> > $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > description: |
> > Reference to the tamp register which exposes the Cortex-M4 state.
> > - maxItems: 1
> > + items:
> > + - items:
> > + - description: Phandle of syscon block with the tamp register
> > + - description: FIXME
> > + - description: FIXME
>
> - description: The offset of the tamp register
> - description: The field mask of the Cortex-M4 state
>
> >
> > st,syscfg-rsc-tbl:
> > $ref: "/schemas/types.yaml#/definitions/phandle-array"
> > description: |
> > Reference to the tamp register which references the Cortex-M4
> > resource table address.
> > - maxItems: 1
> > + items:
> > + - items:
> > + - description: Phandle of syscon block with the tamp register
> > + - description: FIXME
> > + - description: FIXME
>
> - description: The offset of the tamp register
> - description: The field mask of the Cortex-M4 resource table address
>
> Please tell me if you prefer that I fix this in a dedicated patch.

Thanks! I'll fold this into this patch.

Rob