Re: [PATCH v2 1/2] regulator: dt-bindings: add hi3798cv200 regulator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dt-bindings: regulator: missing hardware calibration properties causes overvoltage
- [Medium] dt-bindings: compatible restricts to single SoC despite supporting multiple variants
--

commit e41927e1676c51346b08990a591c57b883a2a88d
Author: David Yang <mmyangfl@gmail.com>

regulator: dt-bindings: add hi3798cv200 regulator

The PMOC block of the HiSilicon STB SoCs (Hi3798cv200/mv100/mv200)
drives an external converter with a PWM signal. Add dt-binding doc.

> diff --git a/Documentation/devicetree/bindings/regulator/hisilicon,hi3798cv200-regulator.yaml b/Documentation/devicetree/bindings/regulator/hisilicon,hi3798cv200-regulator.yaml
> new file mode 100644
> index 0000000000000..812d94aca1f59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/hisilicon,hi3798cv200-regulator.yaml

[ ... ]

> +description: |
> +  HiSilicon STB SoCs (Hi3798cv200/mv100/mv200) provide PWM outputs in the PMOC
> +  block which drive an external converter.
> +
> +  The voltage range depends on the board design: reference designs specify
> +  0.65 V to 1.15 V for the Hi3798cv200, and 0.7 V to 1.25 V for the Hi3798mv200;
> +  a customized board has to figure out its own. The regulator itself has no
> +  other source of calibration.
> +
> +properties:
> +  compatible:
> +    const: hisilicon,hi3798cv200-regulator

[Severity: Medium]
Will this strict constraint reject device trees for the other supported SoCs?

The description explicitly mentions support for multiple HiSilicon STB SoCs
(Hi3798cv200/mv100/mv200) which have different voltage reference designs.
However, restricting the compatible property to a single exact value
(hisilicon,hi3798cv200-regulator) rejects device trees that use SoC-specific
compatible strings (e.g., hisilicon,hi3798mv200-regulator) or fallback lists.

This prevents the proper representation of the other supported hardware variants
without failing schema validation (dt_binding_check).

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - regulator-min-microvolt
> +  - regulator-max-microvolt
> +
> +allOf:
> +  - $ref: regulator.yaml#
> +
> +unevaluatedProperties: false

[Severity: High]
Does the lack of hardware calibration properties here cause overvoltage
in the driver?

By not including properties to describe the physical hardware bounds (e.g.,
duty-cycle mappings or pwm-regulator integration), strict validation prevents
defining these board-specific calibration values in the device tree.

As a result, the follow-up commit "regulator: histb: add voltage regulator
driver" relies on the standard OS policy constraints (regulator-min-microvolt
and regulator-max-microvolt) as physical 100% and 0% duty cycle calibration
boundaries:

drivers/regulator/histb-regulator.c:histb_regulator_probe() {
    ...
	priv->min_uV = init_data->constraints.min_uV;
	priv->max_uV = init_data->constraints.max_uV;
    ...
}

If a device tree author specifies a tighter policy constraint to protect
hardware (e.g., a maximum of 1.0V when the physical maximum is 1.15V),
a legitimate request for 1.0V outputs a 0% duty cycle:

drivers/regulator/histb-regulator.c:histb_regulator_set_voltage() {
    ...
	if (min_uV >= priv->max_uV)
		duty = 0;
    ...
}

This configures the hardware to physically deliver the unconstrained maximum
of 1.15V, completely bypassing the intended safety bounds and potentially
damaging connected components.

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