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? > >> >>>> >>>>> + $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. > >>>>> + 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