Messages in this thread | | | Date | Wed, 18 Oct 2023 15:01:08 +0800 | Subject | Re: [PATCH 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin control | From | 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
| |