[PATCH v7 1/3] dt-bindings: pinctrl: Add aspeed,ast2700-soc0-pinctrl

[Billy Tsai]

> > > > +    properties:
> > > > +      function:
> > > > +        enum:
> > > > +          - EMMC
> > > > +          - JTAGDDR
> > > > +          - JTAGM0
> > > > +          - JTAGPCIEA
> > > > +          - JTAGPCIEB
> > > > +          - JTAGPSP
> > > > +          - JTAGSSP
> > > > +          - JTAGTSP
> > > > +          - JTAGUSB3A
> > > > +          - JTAGUSB3B
> > > > +          - PCIERC0PERST
> > > > +          - PCIERC1PERST
> > > > +          - TSPRSTN
> > > > +          - UFSCLKI
> > > > +          - USB2AD0
> > > > +          - USB2AD1
> > > > +          - USB2AH
> > > > +          - USB2AHP
> > > > +          - USB2AHPD0
> > > > +          - USB2AXH
> > > > +          - USB2AXH2B
> > > > +          - USB2AXHD1
> > > > +          - USB2AXHP
> > > > +          - USB2AXHP2B
> > > > +          - USB2AXHPD1
> > > > +          - USB2BD0
> > > > +          - USB2BD1
> > > > +          - USB2BH
> > > > +          - USB2BHP
> > > > +          - USB2BHPD0
> > > > +          - USB2BXH
> > > > +          - USB2BXH2A
> > > > +          - USB2BXHD1
> > > > +          - USB2BXHP
> > > > +          - USB2BXHP2A
> > > > +          - USB2BXHPD1
> > > > +          - USB3AXH
> > > > +          - USB3AXH2B
> > > > +          - USB3AXHD
> > > > +          - USB3AXHP
> > > > +          - USB3AXHP2B
> > > > +          - USB3AXHPD
> > > > +          - USB3BXH
> > > > +          - USB3BXH2A
> > > > +          - USB3BXHD
> > > > +          - USB3BXHP
> > > > +          - USB3BXHP2A
> > > > +          - USB3BXHPD
> > > > +          - VB
> > > > +          - VGADDC
> > > > +
> > > > +      groups:
> > > > +        enum:
> > > > +          - EMMCCDN
> > > > +          - EMMCG1
> > > > +          - EMMCG4
> > > > +          - EMMCG8
> > > > +          - EMMCWPN
> > > > +          - JTAG0
> > > > +          - PCIERC0PERST
> > > > +          - PCIERC1PERST
> > > > +          - TSPRSTN
> > > > +          - UFSCLKI
> > > > +          - USB2A
> > > > +          - USB2AAP
> > > > +          - USB2ABP
> > > > +          - USB2ADAP
> > > > +          - USB2AH
> > > > +          - USB2AHAP
> > > > +          - USB2B
> > > > +          - USB2BAP
> > > > +          - USB2BBP
> > > > +          - USB2BDBP
> > > > +          - USB2BH
> > > > +          - USB2BHBP
> > > > +          - USB3A
> > > > +          - USB3AAP
> > > > +          - USB3ABP
> > > > +          - USB3B
> > > > +          - USB3BAP
> > > > +          - USB3BBP
> > > > +          - VB0
> > > > +          - VB1
> > > > +          - VGADDC
> > > > +      pins:
> > > > +        enum:
> > > > +          - AB13
> > > > +          - AB14
> > > > +          - AC13
> > > > +          - AC14
> > > > +          - AD13
> > > > +          - AD14
> > > > +          - AE13
> > > > +          - AE14
> > > > +          - AE15
> > > > +          - AF13
> > > > +          - AF14
> > > > +          - AF15

> > > Why do you have groups and pins?
> > > Is it valid in your device to have groups and pins in the same node?

> > The intent is to support both group-based mux selection and
> > configuration, as well as per-pin configuration.

> > In our hardware:
> > - `function` + `groups` are used for pinmux selection.
> > - `pins` is used for per-pin configuration (e.g. drive strength,
> >   bias settings).
> > - `groups` may also be used for group-level configuration.

> > As a result, both `groups` and `pins` may appear in the same node,
> > but they serve different purposes and do not conflict:
> > - `groups` selects the mux function and may apply configuration to
> >   the entire group.
> > - `pins` allows overriding or specifying configuration for individual
> >   pins.

> > In most cases, only one of them is needed, but both are allowed when
> > both group-level and per-pin configuration are required.

> To be honest, that sounds like your groups are not sufficiently
> granular and should be reduced such that you can use them for pin
> settings.

The intent was to keep the binding flexible, but in practice the mixed
use of `groups` and `pins` in the same node is not expected to be used.

Given that, I agree this flexibility is unnecessary and makes the
binding semantics less clear. I'll rework the binding to make the
expected usage explicit rather than allowing combinations that do not
correspond to a real use case.

In particular, I'll split the constraints as follows:

- For pinmux, the presence of `function` will require `groups`, and
  `pins` will not be allowed. This reflects the hardware design, where
  the groups are defined by the pins affected by a given mux expression

- For pin configuration, exactly one of `groups` or `pins` will be
  required (using oneOf), so that configuration is applied either at
  group level or per-pin, but not both.


- if:
    required:
      - function
  then:
    required:
      - groups
    not:
      required:
        - pins
  else:
    oneOf:
      - required:
          - groups
        not:
          required:
            - pins
      - required:
          - pins
        not:
          required:
            - groups
Does this match what you had in mind?

Thanks
Billy Tsai