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

[Mark Brown]

> Add driver for the TI LM51772 buck-boost voltage regulator.

> @@ -0,0 +1,903 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI LM51772 I2C regulator driver
> + *
> + * Datasheet: SNVSC22D (September 2025)
> + *
> + * Copyright (c) 2026 Bootlin
> + */

Please make the entire comment block a C++ one so things look more
intentional.

> +static int lm51772_irq_map_event(int irq, struct regulator_irq_data *rid, unsigned long *dev_mask)
> +{

> +	ret = regmap_read(regmap, LM51772_REG_CLEAR_FAULTS, &reg);
> +	if (ret)
> +		return REGULATOR_FAILED_RETRY;

This register appears to be clear on read so it should be marked as
precious, otherwise debugfs will eat notifications.

> +/*
> + * For < 24V, voltages are set in 10mV monotonous steps.
> + * For >= 24V, voltages are set in 20mV monotonous steps.
> + */
> +static const struct linear_range lm51772_voltage_ranges[] = {
> +	REGULATOR_LINEAR_RANGE(LM51772_DIV10_MIN_UV, 0, 99, 0),
> +	REGULATOR_LINEAR_RANGE(LM51772_DIV10_MIN_UV, 100, 2399, LM51772_DIV10_STEP_UV),

Looks like this ought to cover 2400 too since there's overlap between
the two ranges?  If we don't have it then the driver will get upset if
it finds the hardware with that configuration when it starts.

> +	  .active_discharge_mask = LM51772_FORCE_DISCH,
> +	  .active_discharge_on = FIELD_PREP_CONST(LM51772_FORCE_DISCH, 1),
> +	  .active_discharge_off = FIELD_PREP_CONST(LM51772_FORCE_DISCH, 0),

It looks like this is an always on load on the output rather than
something only enabled when the regulator has been disabled so we'll end
up with an extra power drain for no reason.  Possibly we should have
core support for hardware that needs such manual management, though the
datasheet suggests DICHARG_CONFIG0/1 might be more appropriate for what
the API is looking for here?

-- 
Mark Brown <broonie@xxxxxxxxxx>