Re: [PATCH 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin control

[Jacky Huang]

Dear Krzysztof,

Thank you for the review.


On 2023/10/18 下午 01:58, Krzysztof Kozlowski wrote:
> On 18/10/2023 05:26, Jacky Huang wrote:
> > Dear Krzysztof,
> >
> > Thank you for the review.
> >
> >
> > On 2023/10/17 上午 03:52, Krzysztof Kozlowski wrote:
> > > On 16/10/2023 06:32, Jacky Huang wrote:
> > > > > > +  '#size-cells':
> > > > > > +    const: 1
> > > > > > +
> > > > > > +  nuvoton,sys:
> > > > > > +    description:
> > > > > > +      phandle to the syscon node
> > > > > sys is quite generic. Description explains nothing except duplicating
> > > > > known information. Drop duplicated info and instead explain to what this
> > > > > phandle points and how it is going to be used.
> > > Read comments carefully.
> >
> > I will update the description of 'nuvoton,sys'.
> What is the full name of destination block?


The full name is 'system-management'. From:

        sys: system-management@40460000 {
            compatible = "nuvoton,ma35d1-reset", "syscon";
            reg = <0x0 0x40460000 0x0 0x200>;
            #reset-cells = <1>;
        };


> > > > > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > > > +    items:
> > > > > > +      maxItems: 1
> > > > > So just phandle, not phandle-array, unless it is defined like this in
> > > > > some other binding.
> > > > I would like to update this as:
> > > >
> > > >      nuvoton,sys:
> > > Nothing improved.
> > Here just fix  the 'phandle-array' to 'phandle' and remove 'maxItems'.
> >
> > > >        $ref: /schemas/types.yaml#/definitions/phandle
> > > >        description:
> > > >          Help pinctrl driver to access system registers by means of regmap.
> > > Driver is not relevant here. Say which part of syscon are necessary for
> > > pinctrl operation.
> > I will update description as:
> >
> >     nuvoton,sys:
> >       $ref: /schemas/types.yaml#/definitions/phandle
> >       description:
> >         The pin function control registers are located in the system
> >         control register space. This phandle provides pinctrl the
> >         ability to access the pin function control registers through
> >         the use of regmap.
> regmap is unrelated to the bindings.

So, I will just update the description as:

phandle of the system-management node.


> > > > > > +        maximum: 7
> > > > > > +
> > > > > > +      input-enable: true
> > > > > > +
> > > > > > +      input-schmitt-enable: true
> > > > > > +
> > > > > > +      power-source:
> > > > > > +        description:
> > > > > > +          I/O voltage in millivolt.
> > > > > > +        enum: [ 1800, 3300 ]
> > > > > Missing units in property name. power-source also does not really
> > > > > describe the property.
> > > > The output voltage level of GPIO can be configured as 1.8V or 3.3V,
> > > > but I cannot find any suitable output properties in 'pincfg-node.yaml.'
> > > There is actually power-source, but treated as actual choice of power
> > > supplies.
> > >
> > > > I noticed that 'xlnx,zynq-pinctrl.yaml' and 'xlnx,zynq-pinctrl.yaml' use
> > > > 'power source' to specify the output voltage.  Should I follow their
> > > > approach or define a vendor-specific one?
> > > Maybe Rob or Linus have here some recommendation, but I would suggest to
> > > go either with rtd1319d-pinctrl.yaml approach or add a generic property
> > > to pincfg-node expressed in real units like "io-microvolt".
> > OK, I will update it as:
> >
> >         power-source:
> >           description: |
> >             Valid arguments are described as below:
> >             0: power supply of 1.8V
> >             1: power supply of 3.3V
> >           enum: [0, 1]
> >
> >
> > > Rob, Linus, any ideas for generic property replacing register-specific
> > > power-source?
> I proposed io-microvolt
>
> Best regards,
> Krzysztof

I will use 'io-microvolt' once it is available.



Best Regards,
Jacky Huang