Re: [PATCH v1 5/7] regulator: lp872x: Remove platform_data based probing

[Mark Brown]

On Tue, Oct 06, 2026 at 06:44:29PM +0300, Svyatoslav Ryhel wrote:

> The platform_data-based probing is tightly integrated into the lp872x
> driver structure itself; removing just the platform data support is
> impossible without remodeling major parts of the probe sequence.

>  static int lp872x_init_dvs(struct lp872x *lp)
>  {
> -	struct lp872x_dvs *dvs = lp->pdata ? lp->pdata->dvs : NULL;
> -	enum gpiod_flags pinstate;
> +	struct device *dev = lp->dev;
>  	u32 mask[] = { LP8720_EXT_DVS_M, LP8725_DVS1_M | LP8725_DVS2_M };
>  	u32 default_dvs_mode[] = { LP8720_DEFAULT_DVS, LP8725_DEFAULT_DVS };
> +	u8 dvs_state;

> +	device_property_read_u8(dev, "ti,dvs-vsel", (u8 *)&lp->vsel);

Since you're updating this we should fix to read into a local u8 and
assign the value - the cast won't work on big endian.

> +	device_property_read_u8(dev, "ti,dvs-state", &dvs_state);

If these properties are absent then dvs_state won't be updated...

> +	lp->dvs_pin = dvs_state ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;

...so this will be random.

> +	lp->dvs_gpio = devm_gpiod_get_optional(dev, "dvs", lp->dvs_pin);
> +	if (IS_ERR(lp->dvs_gpio)) {
> +		dev_err(dev, "gpio request err: %ld\n", PTR_ERR(lp->dvs_gpio));
> +		return PTR_ERR(lp->dvs_gpio);
>  	}

dev_err_probe()

>  static int lp872x_probe(struct i2c_client *cl)
>  {
> -	const struct i2c_device_id *id = i2c_client_get_device_id(cl);
> +	struct device *dev = &cl->dev;
>  	struct lp872x *lp;
> -	struct lp872x_platform_data *pdata;
> +	u8 general_config;

> +	device_property_read_u8(dev, "ti,general-config", &general_config);
> +	if (device_property_read_bool(dev, "ti,update-config")) {
> +		ret = regmap_write(lp->regmap, LP872X_GENERAL_CFG, general_config);

Same issue here with general_config being uninitialised if absent; this
is a buggy DT but we should still handle it better.

Attachment: signature.asc
Description: PGP signature