Re: [PATCH v4 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 f8b9688233f1d3f9facd6db797983e937aae95db
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 RK8xx PMIC device tree schemas to describe board-level power
hold lines that must be released before a PMIC shutdown request.

> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml
> index 7c71447200ba4..2f9554d0052ae 100644
> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk801.yaml
> @@ -29,6 +29,25 @@ 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 external board-level power circuitry and Linux shutdown sequencing
be modeled as properties on the RK8xx PMIC nodes?

As noted in Documentation/devicetree/bindings/writing-bindings.rst,
bindings should be based on what the hardware has rather than what an OS
and driver currently support. Unlike dvs-gpios in rockchip,rk808.yaml
(which describes actual DVS input pins on the RK808), none of the RK8xx
PMICs have dedicated SoC-driven power-hold pins; the commit message and
property descriptions state that these are board-level lines routed from
the SoC into the board's power circuitry.

Furthermore, power-hold-gpios (with maxItems: 2 derived from the Firefly
vendor BSP pmic,hold-gpio and pmic,stby-gpio properties) and
power-hold-delay-ms are added to all seven RK8xx schemas (rockchip,rk801,
rockchip,rk805, rockchip,rk806, rockchip,rk808, rockchip,rk816,
rockchip,rk817, and rockchip,rk818) because rk8xx_probe() in
drivers/mfd/rk8xx-core.c reads them in shared code when
system-power-controller is present.

Would it be more appropriate to model the board's power hold/latch
circuitry or upstream supply rails as separate board-level DT nodes (such
as a regulator or dedicated board power controller node) sequenced via
sys_off_handler priorities, rather than placing procedural GPIO arrays and
settle delays on the PMIC MFD node?

[ ... ]

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