| Message ID | 20241023055118.1400286-3-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce initial support for the AMD I3C (non-HCI) to DW driver | expand |
On 10/23/24 8:51 AM, Shyam Sundar S K wrote: > On AMD platforms, the IRQ lines are shared between two instances of I3C. > Add IRQF_SHARED flag during the interrupt registration process. > > Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com> > Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/i3c/master/dw-i3c-master.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c > index 1a7c300b6d45..fd58a95ae1c3 100644 > --- a/drivers/i3c/master/dw-i3c-master.c > +++ b/drivers/i3c/master/dw-i3c-master.c > @@ -1578,7 +1578,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > writel(INTR_ALL, master->regs + INTR_STATUS); > irq = platform_get_irq(pdev, 0); > ret = devm_request_irq(&pdev->dev, irq, > - dw_i3c_master_irq_handler, 0, > + dw_i3c_master_irq_handler, IRQF_SHARED, > dev_name(&pdev->dev), master); dw_i3c_master_irq_handler() seems to be otherwise ready for shared interrupts but reminded me it might have a similar issue than drivers/i2c/busses/i2c-designware-master.c had [1] because both are runtime PM managed. To me it looks dw_i3c_master_irq_handler() may incorrectly process interrupt from other device if register reads return all bits one when device is suspended. Worth to check. 1. Commit cdbd2f169bf1 ("i2c: designware: Do not process interrupt when device is suspended")
On 11/4/2024 20:04, Jarkko Nikula wrote: > On 10/23/24 8:51 AM, Shyam Sundar S K wrote: >> On AMD platforms, the IRQ lines are shared between two instances of >> I3C. >> Add IRQF_SHARED flag during the interrupt registration process. >> >> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com> >> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >> --- >> drivers/i3c/master/dw-i3c-master.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/i3c/master/dw-i3c-master.c >> b/drivers/i3c/master/dw-i3c-master.c >> index 1a7c300b6d45..fd58a95ae1c3 100644 >> --- a/drivers/i3c/master/dw-i3c-master.c >> +++ b/drivers/i3c/master/dw-i3c-master.c >> @@ -1578,7 +1578,7 @@ int dw_i3c_common_probe(struct dw_i3c_master >> *master, >> writel(INTR_ALL, master->regs + INTR_STATUS); >> irq = platform_get_irq(pdev, 0); >> ret = devm_request_irq(&pdev->dev, irq, >> - dw_i3c_master_irq_handler, 0, >> + dw_i3c_master_irq_handler, IRQF_SHARED, >> dev_name(&pdev->dev), master); > > dw_i3c_master_irq_handler() seems to be otherwise ready for shared > interrupts but reminded me it might have a similar issue than > drivers/i2c/busses/i2c-designware-master.c had [1] because both are > runtime PM managed. > > To me it looks dw_i3c_master_irq_handler() may incorrectly process > interrupt from other device if register reads return all bits one when > device is suspended. Worth to check. > > 1. Commit cdbd2f169bf1 ("i2c: designware: Do not process interrupt > when device is suspended") Makes sense. I will remove this change in the next revision. Thanks, Shyam
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index 1a7c300b6d45..fd58a95ae1c3 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -1578,7 +1578,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, writel(INTR_ALL, master->regs + INTR_STATUS); irq = platform_get_irq(pdev, 0); ret = devm_request_irq(&pdev->dev, irq, - dw_i3c_master_irq_handler, 0, + dw_i3c_master_irq_handler, IRQF_SHARED, dev_name(&pdev->dev), master); if (ret) goto err_assert_rst;