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

Luca Ceresoli
2025-05-05T11:32:39Z

On Mon, 5 May 2025 13:33:39 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:

> On 4/30/25 5:33 PM, Luca Ceresoli wrote:
> > On Mon, 28 Apr 2025 13:25:11 +0300
> > Cosmin Tanislav <demonsingur@gmail.com> 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@gmail.com>
> >> ---
> >> 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, I see now. Makes sense. Thanks for the clarification!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering