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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6c8a3ed2-879c-4423-b6e7-80f8b127a1c1@linaro.org>
Date:   Wed, 18 Oct 2023 07:58:13 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Jacky Huang <ychuang570808@...il.com>, linus.walleij@...aro.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, p.zabel@...gutronix.de, j.neuschaefer@....net
Cc:     linux-arm-kernel@...ts.infradead.org, linux-gpio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        schung@...oton.com, Jacky Huang <ychuang3@...oton.com>
Subject: Re: [PATCH 2/4] dt-bindings: pinctrl: Document nuvoton ma35d1 pin
 control

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