Re: [PATCH v3 3/3] regulator: Add TI LM51772 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] regulator: lm51772: mark LM51772_REG_CLEAR_FAULTS as precious in regmap_config
--

commit e3ae9ce00ffd3d8ace5d736bc8d42d9be1358656
Author: Paul Louvel <paul.louvel@bootlin.com>

regulator: Add TI LM51772 support

This commit adds a regulator driver for the TI LM51772 I2C buck-boost
voltage regulator and its auxiliary 5 V LDO output.

> diff --git a/drivers/regulator/lm51772-regulator.c b/drivers/regulator/lm51772-regulator.c
> new file mode 100644
> index 0000000000000..a78a55c9da262
> --- /dev/null
> +++ b/drivers/regulator/lm51772-regulator.c

[ ... ]

> +static int lm51772_irq_map_event(int irq, struct regulator_irq_data *rid, unsigned long *dev_mask)
> +{
> +	struct regmap *regmap = rid->data;
> +	unsigned int status, reg, errors;
> +	size_t bit;
> +	int ret;
> +
> +	ret = regmap_read(regmap, LM51772_REG_STATUS_BYTE, &status);
> +	if (ret)
> +		return REGULATOR_FAILED_RETRY;
> +
> +	ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, &reg);
> +	if (ret)
> +		return REGULATOR_FAILED_RETRY;

[ ... ]

> +static const struct regmap_range lm51772_readable_ranges[] = {
> +	regmap_reg_range(LM51772_REG_CLEAR_FAULTS, LM51772_REG_CLEAR_FAULTS),
> +	regmap_reg_range(LM51772_REG_ILIM_THRESHOLD, LM51772_REG_ILIM_THRESHOLD),
> +	regmap_reg_range(LM51772_REG_VOUT_TARGET1_LSB, LM51772_REG_VOUT_TARGET1_MSB),
> +	regmap_reg_range(LM51772_REG_USB_PD_STATUS_0, LM51772_REG_USB_PD_STATUS_0),
> +	regmap_reg_range(LM51772_REG_STATUS_BYTE, LM51772_REG_STATUS_BYTE),
> +	regmap_reg_range(LM51772_REG_USB_PD_CONTROL_0, LM51772_REG_USB_PD_CONTROL_0),
> +	regmap_reg_range(LM51772_REG_MFR_SPECIFIC_D0, LM51772_REG_IVP_VOLTAGE)
> +};
> +
> +static const struct regmap_access_table lm51772_readable_table = {
> +	.yes_ranges = lm51772_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(lm51772_readable_ranges)
> +};
> +
> +static const struct regmap_range lm51772_writeable_ranges[] = {
> +	regmap_reg_range(LM51772_REG_CLEAR_FAULTS, LM51772_REG_CLEAR_FAULTS),
> +	regmap_reg_range(LM51772_REG_STATUS_BYTE, LM51772_REG_STATUS_BYTE),
> +	regmap_reg_range(LM51772_REG_ILIM_THRESHOLD, LM51772_REG_ILIM_THRESHOLD),
> +	regmap_reg_range(LM51772_REG_VOUT_TARGET1_LSB, LM51772_REG_VOUT_TARGET1_MSB),
> +	regmap_reg_range(LM51772_REG_USB_PD_CONTROL_0, LM51772_REG_USB_PD_CONTROL_0),
> +	regmap_reg_range(LM51772_REG_MFR_SPECIFIC_D0, LM51772_REG_IVP_VOLTAGE)
> +};
> +
> +static const struct regmap_access_table lm51772_writeable_table = {
> +	.yes_ranges = lm51772_writeable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(lm51772_writeable_ranges)
> +};
> +
> +static const struct regmap_range lm51772_volatile_ranges[] = {
> +	regmap_reg_range(LM51772_REG_USB_PD_STATUS_0, LM51772_REG_USB_PD_STATUS_0),
> +	regmap_reg_range(LM51772_REG_STATUS_BYTE, LM51772_REG_STATUS_BYTE),
> +	regmap_reg_range(LM51772_REG_CLEAR_FAULTS, LM51772_REG_CLEAR_FAULTS)
> +};
> +
> +static const struct regmap_access_table lm51772_volatile_table = {
> +	.yes_ranges = lm51772_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(lm51772_volatile_ranges)
> +};
> +
> +static const struct regmap_config lm51772_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.rd_table = &lm51772_readable_table,
> +	.wr_table = &lm51772_writeable_table,
> +	.volatile_table = &lm51772_volatile_table,
> +	.max_register = LM51772_REG_IVP_VOLTAGE,
> +	.cache_type = REGCACHE_MAPLE
> +};

[Severity: Medium]
Should LM51772_REG_CLEAR_FAULTS be marked as precious in
lm51772_regmap_config?

As shown in lm51772_irq_map_event() and lm51772_irq_renable(), reading
LM51772_REG_CLEAR_FAULTS (0x03) clears latched fault status bits in
LM51772_REG_STATUS_BYTE (0x78) on the device.

Because LM51772_REG_CLEAR_FAULTS is included in lm51772_readable_ranges
without a precious_table or precious_reg in lm51772_regmap_config, reading
the regmap debugfs registers file causes regmap_read_debugfs() to read
LM51772_REG_CLEAR_FAULTS first, clearing active hardware fault flags and
deasserting the fault interrupt before LM51772_REG_STATUS_BYTE is read.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-paul-lm51772-v3-0-0de62c22f342@bootlin.com?part=3