Re: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver

[Rob Herring]

On Tue, Feb 9, 2021 at 2:17 AM Sai Krishna Potthuri <lakshmis@xxxxxxxxxx> wrote:
>
> Hi Rob,
>
> Thanks for the review.
>
> > -----Original Message-----
> > From: Rob Herring <robh@xxxxxxxxxx>
> > Sent: Tuesday, February 9, 2021 7:51 AM
> > To: Sai Krishna Potthuri <lakshmis@xxxxxxxxxx>
> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>; Michal Simek
> > <michals@xxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>;
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx; git
> > <git@xxxxxxxxxx>; saikrishna12468@xxxxxxxxx
> > Subject: Re: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP
> > pinctrl driver
> >
> > On Tue, Jan 19, 2021 at 10:57:33AM +0530, Sai Krishna Potthuri wrote:
> > > Added documentation and dt-bindings file which contains MIO pin
> > > configuration defines for Xilinx ZynqMP pinctrl driver.
> > >
> > > Signed-off-by: Sai Krishna Potthuri
> > > <lakshmi.sai.krishna.potthuri@xxxxxxxxxx>
> > > ---
> > >  .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 337
> > > ++++++++++++++++++  include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |
> > > 23 ++
> > >  2 files changed, 360 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > >  create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > > b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > > new file mode 100644
> > > index 000000000000..9f2efbafcaa4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.ya
> > > +++ ml
> > > @@ -0,0 +1,337 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Xilinx ZynqMP Pinctrl
> > > +
> > > +maintainers:
> > > +  - Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xxxxxxxxxx>
> > > +  - Rajan Vaja <rajan.vaja@xxxxxxxxxx>
> > > +
> > > +description: |
> > > +  Please refer to pinctrl-bindings.txt in this directory for details
> > > +of the
> > > +  common pinctrl bindings used by client devices, including the
> > > +meaning of the
> > > +  phrase "pin configuration node".
> > > +
> > > +  ZynqMP's pin configuration nodes act as a container for an
> > > + arbitrary number of  subnodes. Each of these subnodes represents
> > > + some desired configuration for a  pin, a group, or a list of pins or
> > > + groups. This configuration can include the  mux function to select
> > > + on those pin(s)/group(s), and various pin configuration  parameters, such
> > as pull-up, slew rate, etc.
> > > +
> > > +  Each configuration node can consist of multiple nodes describing
> > > + the pinmux and  pinconf options. Those nodes can be pinmux nodes or
> > pinconf nodes.
> > > +
> > > +  The name of each subnode is not important; all subnodes should be
> > > + enumerated  and processed purely based on their content.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: xlnx,zynqmp-pinctrl
> > > +
> > > +patternProperties:
> > > +  '^(.*-)?(default|gpio)$':
> > > +    type: object
> > > +    patternProperties:
> > > +      '^mux(.*)$':
> >
> > '^mux' is equivalent.
> I will fix in v3.
>
> >
> > > +        type: object
> > > +        description:
> > > +          Pinctrl node's client devices use subnodes for pin muxes,
> > > +          which in turn use below standard properties.
> > > +        $ref: pinmux-node.yaml#
> > > +
> > > +        properties:
> > > +          groups:
> > > +            description:
> > > +              List of groups to select (either this or "pins" must be
> > > +              specified), available groups for this subnode.
> > > +            items:
> > > +              oneOf:
> > > +                - enum: [ethernet0_0_grp, ethernet1_0_grp,
> > > + ethernet2_0_grp,
> >
> > Don't need 'oneOf' for a single item.
> Here we have a possibility to have more than one group item as below,
> hence used 'oneOf'.
> groups = "uart0_4_grp", "uart0_5_grp";
> Please suggest me if there is a better/another way to represent this.

'items' has 2 forms: a list with a schema per entry or a schema that
applies to all entries.

1 item:
items:
  - enum: [...]

all items:
items:
  enum: [...]

You should use the latter form. You may need 'maxItems' here. Pick a
'should be enough' value if you don't have an actual max.

Rob