The last two are perhaps not strictly fixes, as they essentially add support for nested ATRs. The first one is a fix, thus stable is in cc.
Tomi
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ideasonboard.com --- Changes in v2: - Use mutext_init_with_key() - Rearrange the series so that the fix is the first patch - Link to v1: https://lore.kernel.org/r/20241122-i2c-atr-fixes-v1-0-62c51ce790be@ideasonbo...
--- Cosmin Tanislav (1): i2c: atr: Allow unmapped addresses from nested ATRs
Tomi Valkeinen (2): i2c: atr: Fix client detach i2c: atr: Fix lockdep for nested ATRs
drivers/i2c/i2c-atr.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) --- base-commit: adc218676eef25575469234709c2d87185ca223a change-id: 20241121-i2c-atr-fixes-6d52b9f5f0c1
Best regards,
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;
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?
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
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
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
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
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
On Fri, Nov 22, 2024 at 02:26:17PM +0200, Tomi Valkeinen wrote:
The last two are perhaps not strictly fixes, as they essentially add support for nested ATRs. The first one is a fix, thus stable is in cc.
Other than SoB chain issues, code wise LGTM, Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
Hi Andy,
On 22/11/2024 16:13, Andy Shevchenko wrote:
On Fri, Nov 22, 2024 at 02:26:17PM +0200, Tomi Valkeinen wrote:
The last two are perhaps not strictly fixes, as they essentially add support for nested ATRs. The first one is a fix, thus stable is in cc.
Other than SoB chain issues, code wise LGTM, Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
Thanks. I'll try to figure that out. Looks like even if the committer in my git tree is +renesas, b4 send still sends them with my normal email address.
It's not hard to add both SoBs, but... it feels a bit silly.
Tomi
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Luca Ceresoli -
Romain Gantois -
Tomi Valkeinen