[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7d1d72e-54d7-63ae-0eae-685a207d36ef@linux.intel.com>
Date: Wed, 6 Nov 2019 18:24:25 +0800
From: "Tanwar, Rahul" <rahul.tanwar@...ux.intel.com>
To: Rob Herring <robh@...nel.org>
Cc: linus.walleij@...aro.org, mark.rutland@....com,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, andriy.shevchenko@...el.com,
qi-ming.wu@...el.com, yixin.zhu@...ux.intel.com,
cheol.yong.kim@...el.com
Subject: Re: [PATCH v3 2/2] dt-bindings: pinctrl: intel: Add for new SoC
Hi Rob,
Thanks for the feedback.
On 6/11/2019 5:29 AM, Rob Herring wrote:
>> + bias-pull-up:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Specifies pull-up configuration.
> Isn't this boolean?
>
>> +
>> + bias-pull-down:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Specifies pull-down configuration.
> And this?
>
> Though looks like sometimes it has a value? Pull strength I guess.
>
>> +
>> + drive-strength:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Enables driver-current.
>> +
>> + slew-rate:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Enables slew-rate.
>> +
>> + drive-open-drain:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Specifies open-drain configuration.
> boolean?
>
>> +
>> + output-enable:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Specifies if the pin is to be configured as output.
> boolean?
>
> But really, all of these should have a common schema defining the types
> and only put any additional constraints here.
Yes, you are right. These are all boolean types.
All these are standard properties & we are using them with no
additional constraintsi.e conforming to how they are already
documented in pinctrl-bindings.txt. Shall ijust omit documenting
these properties here in driver bindings ?
>> +
>> +examples:
>> + # Pinmux controller node
>> + - |
>> + pinctrl: pinctrl@...80000 {
>> + compatible = "intel,lgm-pinctrl";
>> + reg = <0xe2880000 0x100000>;
>> +
>> + # Client device subnode
>> + uart0:uart0 {
> space ^
Just to be sure, you mean space misalignment at below
line <65>; /* UART_TX0 */ ?Or is it something else ?
>> + pins = <64>, /* UART_RX0 */
>> + <65>; /* UART_TX0 */
>> + function = "CONSOLE_UART0";
>> + pinmux = <1>,
>> + <1>;
>> + groups = "CONSOLE_UART0";
>> + };
>> + };
>> +
>> +...
>> --
>> 2.11.0
>>
Regards,
Rahul
Powered by blists - more mailing lists