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

Cosmin Tanislav
2025-05-05T10:33:45Z

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, nitpick: I wouldn't add that empty line.
>

I can remove it I guess.

> Luca
>