Re: [PATCH V2 2/2] dt-bindings: Document the Rockchip RK1608 bindings

From: Linus Walleij
Date: Mon Feb 26 2018 - 05:17:47 EST


On Mon, Feb 26, 2018 at 9:25 AM, Wen Nuan <leo.wen@xxxxxxxxxxxxxx> wrote:

> From: Leo Wen <leo.wen@xxxxxxxxxxxxxx>
>
> Add DT bindings documentation for Rockchip RK1608.
>
> Changes V2:
> - Delete spi-min-frequency property.
> - Add the external sensor's control pin and clock properties.
> - Delete the '&pinctrl' node.
>
> Signed-off-by: Leo Wen <leo.wen@xxxxxxxxxxxxxx>

(...)
> +- reset-gpio : GPIO connected to reset pin;
> +- irq-gpio : GPIO connected to irq pin;
> +- sleepst-gpio : GPIO connected to sleepst pin;
> +- wakeup-gpio : GPIO connected to wakeup pin;
> +- powerdown-gpio : GPIO connected to powerdown pin;

All these should be named something like:

reset-gpios = <>;
irq-gpios = <>;
etc

See
Documentation/devicetree/bindings/gpio/gpio.txt

So all in pluralis even if it is just one line, that is the standard.

> +- rockchip,powerdown0 : GPIO connected to the sensor0's powerdown pin;
> +- rockchip,reset0 : GPIO connected to the sensor0's reset pin;
> +- rockchip,powerdown1 : GPIO connected to the sensor1's powerdown pin;
> +- rockchip,reset1 : GPIO connected to the sensor1's reset pin;

Also get rid of the custom names here, either no lines should
have a "rockchip", prefix or all of them. Use the name of the
pin on the component, I suspect just

powerdown0-gpios
reset0-gpios
etc

By using the standard "*-gpios" suffix the kernel consumer API
will be much happier as well when you use gpiod_get() & friends.

Yours,
Linus Walleij