Hello Oleksij,
On Tue, 8 Apr 2025 20:09:36 +0200
Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:
> Hi Kory,
>
> here are some points
>
> On Tue, Apr 08, 2025 at 04:32:13PM +0200, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@xxxxxxxxxxx>
> > +static struct pse_power_domain *devm_pse_alloc_pw_d(struct device *dev)
> > +{
> > + struct pse_power_domain *pw_d;
> > + int index, ret;
> > +
> > + pw_d = devm_kzalloc(dev, sizeof(*pw_d), GFP_KERNEL);
> > + if (!pw_d)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ret = xa_alloc(&pse_pw_d_map, &index, pw_d, XA_LIMIT(1,
> > PSE_PW_D_LIMIT),
> > + GFP_KERNEL);
> > + if (ret)
> > + return ERR_PTR(ret);
>
> Missing "kref_init(&pw_d->refcnt);" ?
Oh yes, indeed thanks.
> > +
> > + pw_d = devm_pse_alloc_pw_d(pcdev->dev);
> > + if (IS_ERR_OR_NULL(pw_d)) {
>
> s/IS_ERR_OR_NULL/IS_ERR
>
> devm_pse_alloc_pw_d() is not returning NULL.
Yes, that's right!
> > + ret = PTR_ERR(pw_d);
> > + goto out;
> > + }
> > +
> > + supply = regulator_get(&rdev->dev, rdev->supply_name);
> > + if (IS_ERR(supply)) {
> > + xa_erase(&pse_pw_d_map, pw_d->id);
> > + ret = PTR_ERR(supply);
>
> Here:
> Either we need to ensure pse_flush_pw_ds() handles incomplete setups
> or immediately clean up earlier entries in the loop when an error
> occurs.
The pw_d has not yet been saved to pcdev->pi[i].pw_d so we need to remove the
pw_d from the xarray here. pse_flush_pw_ds will deal with all the pw_d entries
in pcdev->pi[x].
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
