Re: [PATCH v5 1/3] dt-bindings: mfd: rk8xx: add board level power hold GPIOs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: mfd: rk8xx: board power-hold GPIOs modeled as PMIC properties
--

commit 2f97d8ee4039bd6ee8ced1ebc89f3ae0764e0214
Author: Yaozhong Li <yaozhonguwl@gmail.com>

dt-bindings: mfd: rk8xx: add board level power hold GPIOs

This commit adds power-hold-gpios and power-hold-delay-ms properties to
all seven Rockchip RK8xx PMIC Devicetree binding schemas (rk801, rk805,
rk806, rk808, rk816, rk817, and rk818) to describe board-level power hold
lines driven by the SoC and released before PMIC shutdown.

> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml
> index 7c71447200ba4..bfba394b23f7c 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml
> @@ -29,6 +29,26 @@ properties:
>      description:
>        Telling whether or not this PMIC is controlling the system power.
>  
> +  power-hold-gpios:
> +    minItems: 1
> +    maxItems: 2
> +    description: >
> +      Board level power hold lines driven by the SoC. They have to be held
> +      asserted while the system is running and released before the PMIC
> +      shutdown request is written, otherwise the rails can drop and come
> +      back up instead of staying off. Only meaningful when this PMIC is the
> +      system power controller, as the lines are released as part of its
> +      power-off path.
> +
> +      The lines are released in the order listed, and power-hold-delay-ms
> +      elapses once, after the last of them.
> +
> +  power-hold-delay-ms:
> +    default: 0
> +    description:
> +      Time to wait after releasing power-hold-gpios before the shutdown
> +      request is written, to let the lines settle.

[Severity: Medium]
Should these board-level power circuitry GPIOs and OS shutdown sequencing
delays be modeled as properties on the RK8xx PMIC nodes across
Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml,
rockchip,rk805.yaml, rockchip,rk806.yaml, rockchip,rk808.yaml,
rockchip,rk816.yaml, rockchip,rk817.yaml, and rockchip,rk818.yaml?

As noted in the commit message and the power-hold-gpios description, these
lines are routed from the SoC into external board power circuitry rather
than into pins on the RK801, RK805, RK806, RK808, RK809, RK816, RK817, or
RK818 PMICs themselves.

In addition, adding these properties to all seven schemas because they share
the Linux drivers/mfd/rk8xx-core.c driver, bounding maxItems at 2 to match
the vendor kernel's pmic,hold-gpio and pmic,stby-gpio properties, and
encoding the release order and power-hold-delay-ms settle time prior to the
PMIC shutdown write appear to describe driver-specific shutdown sequencing
rather than the PMIC hardware.

Would it be more appropriate to describe the actual board-level power
switches or regulators in the Devicetree and handle their shutdown ordering
through standard kernel power-off or regulator mechanisms instead of adding
these properties to the PMIC bindings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027084511.1992-1-yaozhonguwl@gmail.com?part=1