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

[Svyatoslav Ryhel]

ср, 7 жовт. 2026 р. о 11:39 Mark Brown <broonie@xxxxxxxxxx> пише:
>
> 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.
>

Noted

> > +     device_property_read_u8(dev, "ti,dvs-state", &dvs_state);
>
> If these properties are absent then dvs_state won't be updated...
>

I will init dvs_state above then.

> > +     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()
>

Noted

> >  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.

Noted, I will check default values in datasheet then, thanks.