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

[Sai Krishna Potthuri]

Hi Rob,

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Tuesday, February 9, 2021 7:27 PM
> 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, 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.ya
> > > > ml
> > > > b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.ya
> > > > ml
> > > > new file mode 100644
> > > > index 000000000000..9f2efbafcaa4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctr
> > > > +++ l.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.
Thanks.
Sure, I will update accordingly in v3 for groups and pins.

Regards
Sai Krishna
> 
> Rob