Re: [PATCH v4 6/9] i2c: atr: allow replacing mappings in attach_addr()

[Cosmin Tanislav]

On 4/30/25 5:33 PM, Luca Ceresoli wrote:
> On Mon, 28 Apr 2025 13:25:11 +0300
> Cosmin Tanislav <demonsingur@xxxxxxxxx> wrote:
>
> > It is possible for aliases to be exhausted while we are still attaching
> > children.
> >
> > Allow replacing mapping on attach by calling
> > i2c_atr_replace_mapping_by_addr() if i2c_atr_create_mapping_by_addr()
> > fails.
> >
> > Signed-off-by: Cosmin Tanislav <demonsingur@xxxxxxxxx>
> > ---
> >   drivers/i2c/i2c-atr.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> > index bf7b2ac5e9cf..7214a59ddf15 100644
> > --- a/drivers/i2c/i2c-atr.c
> > +++ b/drivers/i2c/i2c-atr.c
> > @@ -543,6 +543,9 @@ static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
> >   	mutex_lock(&chan->alias_pairs_lock);
> >   
> >   	c2a = i2c_atr_create_mapping_by_addr(chan, addr);
> > +	if (!c2a)
> > +		c2a = i2c_atr_replace_mapping_by_addr(chan, addr);
> > +
> >   	if (!c2a) {
> >   		dev_err(atr->dev, "failed to find a free alias\n");
> >   		ret = -EBUSY;
>
> Looks like this should be squashed into patch 5, no? I might be
> wrong, but IIUC the change in patch 5 is introducing a "bug" ("It is
> possible for aliases to be exhausted while we are still attaching
> children") and this patch fixes it.

Patch 5 doesn't introduce a bug, this is just how things were before.
If you look at the diff in patch 5, the error case of
i2c_atr_reserve_alias() just returns an error.

The logic in i2c_atr_attach_addr() didn't handle the case where a
mapping is not able to be created, matching the logic in
i2c_atr_create_mapping_by_addr().

This patch adds handling for that case.

> Ah, nitpick: I wouldn't add that empty line.

I can remove it I guess.

> Luca