Re: [PATCH v3 1/4] regulator: dt-bindings: Add fan53555 allowed modes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The device tree binding introduces the `regulator-initial-mode` property, but the corresponding driver lacks the `.of_map_mode` callback required to use it.
- [Low] The patch defines constraints for `regulator-initial-mode` but fails to define constraints for `regulator-allowed-modes`.

Pre-existing issues:
- [High] A preexisting bug in `fan53555_set_mode()` corrupts the voltage register when setting `REGULATOR_MODE_NORMAL` on certain vendor variants like RK8602.
--

--- Patch [1]: [PATCH v3 1/4] regulator: dt-bindings: Add fan53555 allowed modes ---
commit 23beca6378b28eed5bc10d3625b311119ecc5eb7
Author: Victor Krawiec <victor.krawiec@arturia.com>

regulator: dt-bindings: Add fan53555 allowed modes

This commit adds device tree binding constants and schema properties to
specify the initial operation mode (Auto-PFM or Forced PWM) for the
FAN53555 PMIC regulator.

> diff --git a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
> index 69bae90fc4b28..9a18891f721e6 100644
> --- a/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
> +++ b/Documentation/devicetree/bindings/regulator/fcs,fan53555.yaml
> @@ -48,6 +48,12 @@ properties:
>        VSEL0 register. When this pin is HIGH, VOUT is set by the VSEL1 register.
>      maxItems: 1
>  
> +  regulator-initial-mode:
> +    enum:
> +      [ 1, 2 ]
> +    description:
> +      Defined in include/dt-bindings/regulator/fcs,fan53555-regulator.h
> +

[Severity: Low]
The commit message mentions adding allowed modes, but the schema only defines
constraints for regulator-initial-mode. Does this leave
regulator-allowed-modes unconstrained, allowing device trees to pass
dt_binding_check even if they specify invalid integers like
regulator-allowed-modes = <99>?

[Severity: Medium]
Will the regulator core actually apply this initial mode?

The fan53555 driver currently lacks the .of_map_mode callback:

drivers/regulator/fan53555.c:fan53555_regulator_register() {
    ...
    rdesc->vsel_mask = BIT(fls(di->vsel_count - 1)) - 1;
    rdesc->owner = THIS_MODULE;

    rdev = devm_regulator_register(di->dev, &di->desc, config);
    ...
}

Without an .of_map_mode implementation to translate the DT integer into a
standard REGULATOR_MODE_* constant, won't the regulator core ignore this
property completely and fail to set the mode?

[ ... ]

> diff --git a/include/dt-bindings/regulator/fcs,fan53555-regulator.h b/include/dt-bindings/regulator/fcs,fan53555-regulator.h
> new file mode 100644
> index 0000000000000..ce3d6c63a0f1f
> --- /dev/null
> +++ b/include/dt-bindings/regulator/fcs,fan53555-regulator.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2026 Arturia - All rights reserved.
> + *
> + * Device Tree binding constants for the FAN53555 PMIC regulator
> + */
> +
> +#ifndef _DT_BINDINGS_REGULATOR_FAN53555_H
> +#define _DT_BINDINGS_REGULATOR_FAN53555_H
> +
> +/*
> + * Constants to specify regulator modes in device tree for SYR82X regulators
> + * FAN53555_REGULATOR_MODE_FORCE_PWM:	Force fixed PWM mode
> + * FAN53555_REGULATOR_MODE_AUTO:	Allow auto-PFM mode during light load
> + */
> +
> +#define FAN53555_REGULATOR_MODE_FORCE_PWM	1
> +#define FAN53555_REGULATOR_MODE_AUTO		2

[Severity: High]
This is a pre-existing issue, but I noticed a potential hardware register
corruption in the driver when setting modes.

In the fan53555 driver, when setting REGULATOR_MODE_NORMAL, does the code
mistakenly clear the mode mask from the voltage register instead of the mode
register?

drivers/regulator/fan53555.c:fan53555_set_mode() {
    ...
    case REGULATOR_MODE_NORMAL:
        regmap_update_bits(rdev->regmap, di->vol_reg, di->mode_mask, 0);
        break;
    ...
}

For variants like the RK8602 where mode_reg and vol_reg are distinct
hardware registers, clearing di->mode_mask (e.g. BIT(6)) in di->vol_reg
could silently drop the output voltage by 64 steps (400mV). Could this lead
to severe undervolting and system crashes if a consumer attempts to change
the mode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723094001.120264-1-victor.krawiec@arturia.com?part=1