1. Patchew
  2. linux
  3. [PATCH v2 0/3] i2c: atr: A few i2c-atr fixes
  4. View patch

[PATCH v2 1/3] i2c: atr: Fix client detach

Tomi Valkeinen posted 3 patches 1 year, 2 months ago
[PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Tomi Valkeinen 1 year, 2 months ago 
From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

i2c-atr catches the BUS_NOTIFY_DEL_DEVICE event on the bus and removes
the translation by calling i2c_atr_detach_client().

However, BUS_NOTIFY_DEL_DEVICE happens when the device is about to be
removed from this bus, i.e. before removal, and thus before calling
.remove() on the driver. If the driver happens to do any i2c
transactions in its remove(), they will fail.

Fix this by catching BUS_NOTIFY_REMOVED_DEVICE instead, thus removing
the translation only after the device is actually removed.

Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")
Cc: stable@vger.kernel.org
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/i2c/i2c-atr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
index f21475ae5921..0d54d0b5e327 100644
--- a/drivers/i2c/i2c-atr.c
+++ b/drivers/i2c/i2c-atr.c
@@ -412,7 +412,7 @@ static int i2c_atr_bus_notifier_call(struct notifier_block *nb,
 				dev_name(dev), ret);
 		break;
 
-	case BUS_NOTIFY_DEL_DEVICE:
+	case BUS_NOTIFY_REMOVED_DEVICE:
 		i2c_atr_detach_client(client->adapter, client);
 		break;
 

-- 
2.43.0
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Wolfram Sang 1 year, 1 month ago 
On Fri, Nov 22, 2024 at 02:26:18PM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> i2c-atr catches the BUS_NOTIFY_DEL_DEVICE event on the bus and removes
> the translation by calling i2c_atr_detach_client().
> 
> However, BUS_NOTIFY_DEL_DEVICE happens when the device is about to be
> removed from this bus, i.e. before removal, and thus before calling
> .remove() on the driver. If the driver happens to do any i2c
> transactions in its remove(), they will fail.
> 
> Fix this by catching BUS_NOTIFY_REMOVED_DEVICE instead, thus removing
> the translation only after the device is actually removed.
> 
> Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Applied to for-current, thanks!
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Tomi Valkeinen 1 year, 2 months ago 
Hi Wolfram,

On 22/11/2024 14:26, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> i2c-atr catches the BUS_NOTIFY_DEL_DEVICE event on the bus and removes
> the translation by calling i2c_atr_detach_client().
> 
> However, BUS_NOTIFY_DEL_DEVICE happens when the device is about to be
> removed from this bus, i.e. before removal, and thus before calling
> .remove() on the driver. If the driver happens to do any i2c
> transactions in its remove(), they will fail.
> 
> Fix this by catching BUS_NOTIFY_REMOVED_DEVICE instead, thus removing
> the translation only after the device is actually removed.
> 
> Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>   drivers/i2c/i2c-atr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> index f21475ae5921..0d54d0b5e327 100644
> --- a/drivers/i2c/i2c-atr.c
> +++ b/drivers/i2c/i2c-atr.c
> @@ -412,7 +412,7 @@ static int i2c_atr_bus_notifier_call(struct notifier_block *nb,
>   				dev_name(dev), ret);
>   		break;
>   
> -	case BUS_NOTIFY_DEL_DEVICE:
> +	case BUS_NOTIFY_REMOVED_DEVICE:
>   		i2c_atr_detach_client(client->adapter, client);
>   		break;
>   
> 

Can you pick this one up (ignore the two other patches in this series), 
or should I send it as a separate patch?

  Tomi
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Romain Gantois 1 year, 2 months ago 
On vendredi 22 novembre 2024 13:26:18 heure normale d’Europe centrale Tomi 
Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> i2c-atr catches the BUS_NOTIFY_DEL_DEVICE event on the bus and removes
> the translation by calling i2c_atr_detach_client().
> 
> However, BUS_NOTIFY_DEL_DEVICE happens when the device is about to be
> removed from this bus, i.e. before removal, and thus before calling
> .remove() on the driver. If the driver happens to do any i2c
> transactions in its remove(), they will fail.
> 
> Fix this by catching BUS_NOTIFY_REMOVED_DEVICE instead, thus removing
> the translation only after the device is actually removed.
> 
> Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/i2c/i2c-atr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> index f21475ae5921..0d54d0b5e327 100644
> --- a/drivers/i2c/i2c-atr.c
> +++ b/drivers/i2c/i2c-atr.c
> @@ -412,7 +412,7 @@ static int i2c_atr_bus_notifier_call(struct
> notifier_block *nb, dev_name(dev), ret);
>  		break;
> 
> -	case BUS_NOTIFY_DEL_DEVICE:
> +	case BUS_NOTIFY_REMOVED_DEVICE:
>  		i2c_atr_detach_client(client->adapter, client);
>  		break;

LGTM, tested on a TI FPC202 ATR.

Reviewed-by: Romain Gantois <romain.gantois@bootlin.com>
Tested-by: Romain Gantois <romain.gantois@bootlin.com>

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Luca Ceresoli 1 year, 2 months ago 
Hello Tomi,

+Cc: Romain who is doing a different kind of sorcery on i2c-atr.c, so
he is aware of this series.

On Fri, 22 Nov 2024 14:26:18 +0200
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:

> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> i2c-atr catches the BUS_NOTIFY_DEL_DEVICE event on the bus and removes
> the translation by calling i2c_atr_detach_client().
> 
> However, BUS_NOTIFY_DEL_DEVICE happens when the device is about to be
> removed from this bus, i.e. before removal, and thus before calling
> .remove() on the driver. If the driver happens to do any i2c
> transactions in its remove(), they will fail.
> 
> Fix this by catching BUS_NOTIFY_REMOVED_DEVICE instead, thus removing
> the translation only after the device is actually removed.
> 
> Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Looks good:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Tomi Valkeinen 1 year, 2 months ago 
Hi Romain,

On 26/11/2024 10:14, Luca Ceresoli wrote:
> Hello Tomi,
> 
> +Cc: Romain who is doing a different kind of sorcery on i2c-atr.c, so
> he is aware of this series.
> 
> On Fri, 22 Nov 2024 14:26:18 +0200
> Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote:
> 
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> i2c-atr catches the BUS_NOTIFY_DEL_DEVICE event on the bus and removes
>> the translation by calling i2c_atr_detach_client().
>>
>> However, BUS_NOTIFY_DEL_DEVICE happens when the device is about to be
>> removed from this bus, i.e. before removal, and thus before calling
>> .remove() on the driver. If the driver happens to do any i2c
>> transactions in its remove(), they will fail.
>>
>> Fix this by catching BUS_NOTIFY_REMOVED_DEVICE instead, thus removing
>> the translation only after the device is actually removed.
>>
>> Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Looks good:
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 

Can you test this one with your setup, and give your Rb/Tb?

I think it's an obvious fix, and could be merged separately from the 
rest, which still need discussion.

  Tomi
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Andy Shevchenko 1 year, 2 months ago 
On Fri, Nov 22, 2024 at 02:26:18PM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

We (used to?) have a check in Linux Next against missing SoB of the committer,
wouldn't this trap into it?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2 1/3] i2c: atr: Fix client detach
Posted by Tomi Valkeinen 1 year, 2 months ago 
Hi,

On 22/11/2024 16:07, Andy Shevchenko wrote:
> On Fri, Nov 22, 2024 at 02:26:18PM +0200, Tomi Valkeinen wrote:
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> We (used to?) have a check in Linux Next against missing SoB of the committer,
> wouldn't this trap into it?

I don't think linux-next can check that. Or rather, it can, but the 
committer there (which is the subsystem maintainer, probably) is not 
related to the sender of this mail, so I don't think linux-next can 
complain about this particular issue.

I didn't right away figure out how to easily change the From address for 
the email with the standard tooling. As this is such a clear case of the 
sender and the author being the same person, I'll just ignore this point 
for now (although strictly speaking I think you're right).

  Tomi

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.