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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux