Re: [PATCH v4 2/3] regulator: Add RPMI voltage service

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] regulator: mpxy_reg_set_voltage_sel() rejects 0 uV and masks list_voltage() error codes
--

commit cbc4d4861f2f3917f562d79d42a2f9d2a4373b1c
Author: Joshua Yeong <joshua.yeong@starfivetech.com>

regulator: Add RPMI voltage service

This commit adds a mailbox client based regulator driver for the RISC-V 
RPMI voltage service group. It enumerates voltage domains advertised by
the platform microcontroller and registers them as regulators.

> diff --git a/drivers/regulator/riscv-rpmi-regulator.c b/drivers/regulator/riscv-rpmi-regulator.c
> new file mode 100644
> index 0000000000000..46119297e7de9
> --- /dev/null
> +++ b/drivers/regulator/riscv-rpmi-regulator.c

[ ... ]

> +static int mpxy_reg_set_voltage_sel(struct regulator_dev *rdev, unsigned int selector)
> +{
> +	struct rpmi_reg_domain *mpxy_reg = rdev_get_drvdata(rdev);
> +	struct rpmi_set_level_tx tx;
> +	struct rpmi_set_level_rx rx = { };
> +	struct rpmi_mbox_message msg;
> +	s32 volt_uV;
> +	int ret;
> +
> +	volt_uV = mpxy_reg->desc.ops->list_voltage(rdev, selector);
> +	if (volt_uV <= 0)
> +		return -EINVAL;

[Severity: Medium]
Does this check incorrectly drop specific error codes from the regulator
core and prevent setting a 0 uV target?

If list_voltage() returns a specific error code (such as -ENOTSUPP), it
is masked by -EINVAL here, which degrades error reporting upstream.
Additionally, if the hardware explicitly supports and advertises a 0 uV
level (which is valid for regulators that support turning off the
output via voltage selection), the driver will falsely reject it.

The standard regulator subsystem pattern is to check:

	if (volt_uV < 0)
		return volt_uV;

> +
> +	tx.domain_id = cpu_to_le32(mpxy_reg->id);
> +	tx.level = cpu_to_le32(volt_uV);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007100022.2512187-1-joshua.yeong@starfivetech.com?part=2