Re: [PATCH 4/5] regulator: Support regulators where the voltage selector spans two registers

[Paul Louvel]

On Thu Oct 22, 2026 at 7:21 PM CEST, Mark Brown wrote:
> On Wed, Oct 21, 2026 at 12:27:45PM +0200, Paul Louvel wrote:
>> Some devices uses 8-bit registers to store the voltage selector across
>> two registers: one for the least significant bits and one for the most
>> significant bits.
>
>> +static int set_oversize_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
>> +{
>> +	int val_bytes = regmap_get_val_bytes(rdev->regmap);
>> +	__le16 val;
>> +
>> +	if (val_bytes != 1)
>> +		return -EINVAL;
>> +
>> +	val = cpu_to_le16(sel);
>> +
>> +	return regmap_bulk_write(rdev->regmap, rdev->desc->vsel_reg, &val, 2);
>> +}
>
> This ignores the shift and mask but the get still pays attention to them
> due to code sharing.  Either there should be checks that reject them
> like we have for val_bytes or there should be a read/modify/write here,
> obviously the former is much easier and that'd be totally fine.  IIRC
> there's some existing drivers we could convert but that's a separate
> thing.

set_oversize_voltage_sel() does not apply the shift itself because both callers
already shift sel into position before calling it [1][2].

Using two separate regmap_update_bits() calls, one per register, would be
equally simple, and it would guarantee that reserved bits outside vsel_mask are
not overwritten on devices that are sensitive (not the case for the LM51772).

[1]: https://elixir.bootlin.com/linux/v7.3/source/drivers/regulator/helpers.c#L222
[2]: https://elixir.bootlin.com/linux/v7.3/source/drivers/regulator/helpers.c#L285

Thanks.
Best regards,
-- 
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com