> -----Original Message-----
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Sent: Thursday, December 21, 2023 9:57 PM
> To: Yuklin Soo <yuklin.soo@xxxxxxxxxxxxxxxx>
> Cc: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>; Hal Feng
> <hal.feng@xxxxxxxxxxxxxxxx>; Leyfoon Tan <leyfoon.tan@xxxxxxxxxxxxxxxx>;
> Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx>; Emil Renner Berthing
> <kernel@xxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> Drew Fustini <drew@xxxxxxxxxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> riscv@xxxxxxxxxxxxxxxxxxx; Paul Walmsley <paul.walmsley@xxxxxxxxxx>; Palmer
> Dabbelt <palmer@xxxxxxxxxxx>; Albert Ou <aou@xxxxxxxxxxxxxxxxx>
> Subject: Re: [RFC PATCH 1/6] dt-bindings: pinctrl: starfive: add JH8100 pinctrl
> bindings
>
> 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
This title will be updated as shown above to clarify the meaning of acronym AON.
>
> (...)
> > + 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:
Yes, the unit is milliamperes, “drive-strength:” will be changed to “drive-strength: Drive strength in mA”
>
> > + 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?
Each pin controller in JH8100 SoC has a set of regmap-based syscon registers to serve different purposes in respective pinctrl domain.
In AON (always-on) pinctrl, there are:
- syscon registers to indicate the I/O voltage level of eMMC, SD0, RGPIOs, and XSPI.
- syscon registers to control the GMAC settings.
The former will be used in the SD/eMMC device driver to indicate the change in voltage supply during voltage switching in the initialization process. The syscon register configuration provides information to indicate the device I/O voltage level. The device driver must make sure these syscon registers are configured properly. In case if setting syscon register to indicate device I/O voltage as 1.8V, but the actual voltage supply is 3.3V. The pads used for the device I/O interface will get damaged.
>
> > + slew-rate:
> > + maximum: 1
>
> Milliseconds? Write unit in description please.
The slew-rate is a single bit in the IO Pad configuration register. Bit value 0 means slow and 1 means fast.
slew-rate:
maximum: 1
will be changed to
slew-rate:
enum: [ 0, 1 ]
default: 0
description: |
0: slow (half frequency)
1: fast
Is that okay?
>
> Yours,
> Linus Walleij
