Received-SPF: pass (google.com: domain of linux-kernel+bounces-8542-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20231221083622.3445726-1-yuklin.soo@starfivetech.com> <20231221083622.3445726-2-yuklin.soo@starfivetech.com>
In-Reply-To: <20231221083622.3445726-2-yuklin.soo@starfivetech.com>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Thu, 21 Dec 2023 14:57:14 +0100
Message-ID: <CACRpkdYL8wK2vX7P7p4QvU9VV2CPjRv_aXiLqO+07MMCCKKk4Q@mail.gmail.com>
Subject: Re: [RFC PATCH 1/6] dt-bindings: pinctrl: starfive: add JH8100
 pinctrl bindings
To: Alex Soo <yuklin.soo@starfivetech.com>
Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>, Hal Feng <hal.feng@starfivetech.com>, 
	Ley Foon Tan <leyfoon.tan@starfivetech.com>, 
	Jianlong Huang <jianlong.huang@starfivetech.com>, Emil Renner Berthing <kernel@esmil.dk>, 
	Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, 
	Conor Dooley <conor+dt@kernel.org>, Drew Fustini <drew@beagleboard.org>, linux-gpio@vger.kernel.org, 
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, 
	linux-riscv@lists.infradead.org, Paul Walmsley <paul.walmsley@sifive.com>, 
	Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Alex,

thanks for your patch!

On Thu, Dec 21, 2023 at 9:36=E2=80=AFAM Alex Soo <yuklin.soo@starfivetech.c=
om> wrote:

> Add dt-binding documentation and header file for JH8100 pinctrl
> driver.
>
> Signed-off-by: Alex Soo <yuklin.soo@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
(...)
> +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 selec=
t.
> +              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 sta=
te first before
> +  raising the actual voltage to the higher operating point.
> +
> +  To decrease the device voltage, hold bit [1:0] to the current operatin=
g 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 firs=
t setting syscon
> +  register bit [1:0] =3D 0, the safe 3.3V startup condition, before chan=
ging 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