Re: [RFC PATCH 1/6] dt-bindings: pinctrl: starfive: add JH8100 pinctrl bindings

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Alex,

thanks for your patch!

On Thu, Dec 21, 2023 at 9:36 AM Alex Soo <yuklin.soo@xxxxxxxxxxxxxxxx> wrote:

> Add dt-binding documentation and header file for JH8100 pinctrl
> driver.
>
> Signed-off-by: Alex Soo <yuklin.soo@xxxxxxxxxxxxxxxx>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@xxxxxxxxxxxxxxxx>
(...)
> +title: StarFive JH8100 AON Pin Controller

If AON means "always-on" then spell that out in the title, the world has
too many opaque TLAs.

title: StarFive JH8100 AON (always-on) Pin Controller

(...)
> +        properties:
> +          pinmux:
> +            description: |
> +              The list of GPIOs and their mux settings or function select.
> +              The GPIOMUX and PINMUX macros are used to configure the
> +              I/O multiplexing and function selection respectively.
> +
> +          bias-disable: true
> +
> +          bias-pull-up:
> +            type: boolean
> +
> +          bias-pull-down:
> +            type: boolean
> +
> +          drive-strength:
> +            enum: [ 2, 4, 8, 12 ]

Milliamperes? Then spell that out in a description:

> +  Voltage regulator supply the actual voltage to the GPIO bank while the syscon register
> +  configuration in bit [1:0] indicates the current voltage setting.
> +
> +  +------+------+-------------------+
> +  | Bit1 | Bit0 | Reference Voltage |
> +  +------+------+-------------------+
> +  |  0   |  0   |     3.3 V         |
> +  +------+------+-------------------+
> +  |  1   |  x   |     1.8 V         |
> +  +------+------+-------------------+
> +
> +  To increase the device voltage, set bit [1:0] to the new operating state first before
> +  raising the actual voltage to the higher operating point.
> +
> +  To decrease the device voltage, hold bit [1:0] to the current operating state until
> +  the actual voltage has stabilized at the lower operating point before changing the
> +  setting.
> +
> +  Alternatively, a device voltage change can always be initiated by first setting syscon
> +  register bit [1:0] = 0, the safe 3.3V startup condition, before changing the device
> +  voltage. Then once the actual voltage is changed and has stabilized at the new operating
> +  point, bit [1:0] can be reset as appropriate.

Actually: where is this information even used?

> +          slew-rate:
> +            maximum: 1

Milliseconds? Write unit in description please.

Yours,
Linus Walleij