* [PATCH v1 0/3] Add support for Nuvoton npcm845 i3c controller @ 2025-02-19 1:10 ` Stanley Chu 0 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu This patchset adds support for the Nuvoton npcm845 Board Management controller (BMC) SoC family. The Nuvoton npcm845 uses the same Silvico IP but an older version. This patchset adds fixes for the npcm845 specific hardware issues. Stanley Chu (3): i3c: master: svc: add support for Nuvoton npcm845 i3c i3c: master: svc: fix npcm845 FIFO empty issue i3c: master: svc: fix npcm845 invalid slvstart event drivers/i3c/master/svc-i3c-master.c | 72 +++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 9 deletions(-) -- 2.34.1 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 0/3] Add support for Nuvoton npcm845 i3c controller @ 2025-02-19 1:10 ` Stanley Chu 0 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu This patchset adds support for the Nuvoton npcm845 Board Management controller (BMC) SoC family. The Nuvoton npcm845 uses the same Silvico IP but an older version. This patchset adds fixes for the npcm845 specific hardware issues. Stanley Chu (3): i3c: master: svc: add support for Nuvoton npcm845 i3c i3c: master: svc: fix npcm845 FIFO empty issue i3c: master: svc: fix npcm845 invalid slvstart event drivers/i3c/master/svc-i3c-master.c | 72 +++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 9 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 1/3] i3c: master: svc: add support for Nuvoton npcm845 i3c 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 1:10 ` Stanley Chu -1 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu Nuvoton npcm845 SoC uses the same Silvico IP but an older version. Add a new comptaible string to distinguish between different hardware versions. Signed-off-by: Stanley Chu <yschu@nuvoton.com> --- drivers/i3c/master/svc-i3c-master.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index d6057d8c7dec..813839498b2b 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -1960,6 +1960,7 @@ static const struct dev_pm_ops svc_i3c_pm_ops = { static const struct of_device_id svc_i3c_master_of_match_tbl[] = { { .compatible = "silvaco,i3c-master-v1"}, + { .compatible = "nuvoton,npcm845-i3c" }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl); -- 2.34.1 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 1/3] i3c: master: svc: add support for Nuvoton npcm845 i3c @ 2025-02-19 1:10 ` Stanley Chu 0 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu Nuvoton npcm845 SoC uses the same Silvico IP but an older version. Add a new comptaible string to distinguish between different hardware versions. Signed-off-by: Stanley Chu <yschu@nuvoton.com> --- drivers/i3c/master/svc-i3c-master.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index d6057d8c7dec..813839498b2b 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -1960,6 +1960,7 @@ static const struct dev_pm_ops svc_i3c_pm_ops = { static const struct of_device_id svc_i3c_master_of_match_tbl[] = { { .compatible = "silvaco,i3c-master-v1"}, + { .compatible = "nuvoton,npcm845-i3c" }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl); -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/3] i3c: master: svc: add support for Nuvoton npcm845 i3c 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 20:43 ` Frank Li -1 siblings, 0 replies; 16+ messages in thread From: Frank Li @ 2025-02-19 20:43 UTC (permalink / raw) To: Stanley Chu Cc: miquel.raynal, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu On Wed, Feb 19, 2025 at 09:10:17AM +0800, Stanley Chu wrote: > Nuvoton npcm845 SoC uses the same Silvico IP but an older version. > Add a new comptaible string to distinguish between different > hardware versions. try wrap at 75 chars. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- > drivers/i3c/master/svc-i3c-master.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index d6057d8c7dec..813839498b2b 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -1960,6 +1960,7 @@ static const struct dev_pm_ops svc_i3c_pm_ops = { > > static const struct of_device_id svc_i3c_master_of_match_tbl[] = { > { .compatible = "silvaco,i3c-master-v1"}, > + { .compatible = "nuvoton,npcm845-i3c" }, Need update binding doc and cc linux-devicetree mail list. Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml binding doc change should be first patch. Frank > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl); > -- > 2.34.1 > -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/3] i3c: master: svc: add support for Nuvoton npcm845 i3c @ 2025-02-19 20:43 ` Frank Li 0 siblings, 0 replies; 16+ messages in thread From: Frank Li @ 2025-02-19 20:43 UTC (permalink / raw) To: Stanley Chu Cc: miquel.raynal, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu On Wed, Feb 19, 2025 at 09:10:17AM +0800, Stanley Chu wrote: > Nuvoton npcm845 SoC uses the same Silvico IP but an older version. > Add a new comptaible string to distinguish between different > hardware versions. try wrap at 75 chars. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- > drivers/i3c/master/svc-i3c-master.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index d6057d8c7dec..813839498b2b 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -1960,6 +1960,7 @@ static const struct dev_pm_ops svc_i3c_pm_ops = { > > static const struct of_device_id svc_i3c_master_of_match_tbl[] = { > { .compatible = "silvaco,i3c-master-v1"}, > + { .compatible = "nuvoton,npcm845-i3c" }, Need update binding doc and cc linux-devicetree mail list. Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml binding doc change should be first patch. Frank > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 1:10 ` Stanley Chu -1 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu npcm845 HW issue: I3C HW stalls the write transfer if the transmit FIFO becomes empty, when new data is written to FIFO, I3C HW resumes the transfer but the first transmitted data bit may have the wrong value. Workaround: Fill the FIFO in advance to prevent FIFO from becoming empty. Signed-off-by: Stanley Chu <yschu@nuvoton.com> --- drivers/i3c/master/svc-i3c-master.c | 58 ++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 813839498b2b..22cb1f1c1fdc 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -102,6 +102,7 @@ #define SVC_I3C_MDATACTRL_TXTRIG_FIFO_NOT_FULL GENMASK(5, 4) #define SVC_I3C_MDATACTRL_RXTRIG_FIFO_NOT_EMPTY 0 #define SVC_I3C_MDATACTRL_RXCOUNT(x) FIELD_GET(GENMASK(28, 24), (x)) +#define SVC_I3C_MDATACTRL_TXCOUNT(x) FIELD_GET(GENMASK(20, 16), (x)) #define SVC_I3C_MDATACTRL_TXFULL BIT(30) #define SVC_I3C_MDATACTRL_RXEMPTY BIT(31) @@ -133,6 +134,16 @@ #define SVC_I3C_EVENT_IBI GENMASK(7, 0) #define SVC_I3C_EVENT_HOTJOIN BIT(31) +/* + * SVC_I3C_QUIRK_FIFO_EMPTY: + * I3C HW stalls the write transfer if the transmit FIFO becomes empty, + * when new data is written to FIFO, I3C HW resumes the transfer but + * the first transmitted data bit may have the wrong value. + * Workaround: + * Fill the FIFO in advance to prevent FIFO from becoming empty. + */ +#define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) + struct svc_i3c_cmd { u8 addr; bool rnw; @@ -216,6 +227,7 @@ struct svc_i3c_master { struct mutex lock; u32 enabled_events; u32 mctrl_config; + u32 quirks; }; /** @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, unsigned int dev_nb = 0, last_addr = 0; u32 reg; int ret, i; + u32 dyn_addr; while (true) { /* clean SVC_I3C_MINT_IBIWON w1c bits */ @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, if (SVC_I3C_MSTATUS_RXPEND(reg)) { u8 data[6]; + /* + * SVC_I3C_QUIRK_FIFO_EMPTY fix: + * The TX FIFO should be ready before the TX transmission is starting. + */ + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); + if (dyn_addr < 0) + return dyn_addr; + writel(dyn_addr, master->regs + SVC_I3C_MWDATAB); + /* * We only care about the 48-bit provisioned ID yet to * be sure a device does not nack an address twice. @@ -1009,21 +1031,20 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, if (ret) break; - /* Give the slave device a suitable dynamic address */ - ret = i3c_master_get_free_addr(&master->base, last_addr + 1); - if (ret < 0) - break; - - addrs[dev_nb] = ret; + addrs[dev_nb] = dyn_addr; dev_dbg(master->dev, "DAA: device %d assigned to 0x%02x\n", dev_nb, addrs[dev_nb]); - - writel(addrs[dev_nb], master->regs + SVC_I3C_MWDATAB); last_addr = addrs[dev_nb++]; } /* Need manual issue STOP except for Complete condition */ svc_i3c_master_emit_stop(master); + /* + * Dynamic address is written to FIFO in advance for SVC_I3C_QUIRK_FIFO_EMPTY fix. + * Flush FIFO in the failed case. + */ + if (ret) + svc_i3c_master_flush_fifo(master); return ret; } @@ -1210,7 +1231,6 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, /* clean SVC_I3C_MINT_IBIWON w1c bits */ writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); - while (retry--) { writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | xfer_type | @@ -1220,6 +1240,22 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, SVC_I3C_MCTRL_RDTERM(*actual_len), master->regs + SVC_I3C_MCTRL); + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { + int i, count, space; + + reg = readl(master->regs + SVC_I3C_MDATACTRL); + space = SVC_I3C_FIFO_SIZE - SVC_I3C_MDATACTRL_TXCOUNT(reg); + count = xfer_len > space ? space : xfer_len; + for (i = 0; i < count; i++) { + if (i == xfer_len - 1) + writel(out[0], master->regs + SVC_I3C_MWDATABE); + else + writel(out[0], master->regs + SVC_I3C_MWDATAB); + out++; + } + xfer_len -= count; + } + ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); if (ret) @@ -1308,6 +1344,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, emit_stop: svc_i3c_master_emit_stop(master); svc_i3c_master_clear_merrwarn(master); + svc_i3c_master_flush_fifo(master); return ret; } @@ -1877,6 +1914,9 @@ static int svc_i3c_master_probe(struct platform_device *pdev) svc_i3c_master_reset(master); + if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; + /* Register the master */ ret = i3c_master_register(&master->base, &pdev->dev, &svc_i3c_master_ops, false); -- 2.34.1 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue @ 2025-02-19 1:10 ` Stanley Chu 0 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu npcm845 HW issue: I3C HW stalls the write transfer if the transmit FIFO becomes empty, when new data is written to FIFO, I3C HW resumes the transfer but the first transmitted data bit may have the wrong value. Workaround: Fill the FIFO in advance to prevent FIFO from becoming empty. Signed-off-by: Stanley Chu <yschu@nuvoton.com> --- drivers/i3c/master/svc-i3c-master.c | 58 ++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 813839498b2b..22cb1f1c1fdc 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -102,6 +102,7 @@ #define SVC_I3C_MDATACTRL_TXTRIG_FIFO_NOT_FULL GENMASK(5, 4) #define SVC_I3C_MDATACTRL_RXTRIG_FIFO_NOT_EMPTY 0 #define SVC_I3C_MDATACTRL_RXCOUNT(x) FIELD_GET(GENMASK(28, 24), (x)) +#define SVC_I3C_MDATACTRL_TXCOUNT(x) FIELD_GET(GENMASK(20, 16), (x)) #define SVC_I3C_MDATACTRL_TXFULL BIT(30) #define SVC_I3C_MDATACTRL_RXEMPTY BIT(31) @@ -133,6 +134,16 @@ #define SVC_I3C_EVENT_IBI GENMASK(7, 0) #define SVC_I3C_EVENT_HOTJOIN BIT(31) +/* + * SVC_I3C_QUIRK_FIFO_EMPTY: + * I3C HW stalls the write transfer if the transmit FIFO becomes empty, + * when new data is written to FIFO, I3C HW resumes the transfer but + * the first transmitted data bit may have the wrong value. + * Workaround: + * Fill the FIFO in advance to prevent FIFO from becoming empty. + */ +#define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) + struct svc_i3c_cmd { u8 addr; bool rnw; @@ -216,6 +227,7 @@ struct svc_i3c_master { struct mutex lock; u32 enabled_events; u32 mctrl_config; + u32 quirks; }; /** @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, unsigned int dev_nb = 0, last_addr = 0; u32 reg; int ret, i; + u32 dyn_addr; while (true) { /* clean SVC_I3C_MINT_IBIWON w1c bits */ @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, if (SVC_I3C_MSTATUS_RXPEND(reg)) { u8 data[6]; + /* + * SVC_I3C_QUIRK_FIFO_EMPTY fix: + * The TX FIFO should be ready before the TX transmission is starting. + */ + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); + if (dyn_addr < 0) + return dyn_addr; + writel(dyn_addr, master->regs + SVC_I3C_MWDATAB); + /* * We only care about the 48-bit provisioned ID yet to * be sure a device does not nack an address twice. @@ -1009,21 +1031,20 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, if (ret) break; - /* Give the slave device a suitable dynamic address */ - ret = i3c_master_get_free_addr(&master->base, last_addr + 1); - if (ret < 0) - break; - - addrs[dev_nb] = ret; + addrs[dev_nb] = dyn_addr; dev_dbg(master->dev, "DAA: device %d assigned to 0x%02x\n", dev_nb, addrs[dev_nb]); - - writel(addrs[dev_nb], master->regs + SVC_I3C_MWDATAB); last_addr = addrs[dev_nb++]; } /* Need manual issue STOP except for Complete condition */ svc_i3c_master_emit_stop(master); + /* + * Dynamic address is written to FIFO in advance for SVC_I3C_QUIRK_FIFO_EMPTY fix. + * Flush FIFO in the failed case. + */ + if (ret) + svc_i3c_master_flush_fifo(master); return ret; } @@ -1210,7 +1231,6 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, /* clean SVC_I3C_MINT_IBIWON w1c bits */ writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); - while (retry--) { writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | xfer_type | @@ -1220,6 +1240,22 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, SVC_I3C_MCTRL_RDTERM(*actual_len), master->regs + SVC_I3C_MCTRL); + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { + int i, count, space; + + reg = readl(master->regs + SVC_I3C_MDATACTRL); + space = SVC_I3C_FIFO_SIZE - SVC_I3C_MDATACTRL_TXCOUNT(reg); + count = xfer_len > space ? space : xfer_len; + for (i = 0; i < count; i++) { + if (i == xfer_len - 1) + writel(out[0], master->regs + SVC_I3C_MWDATABE); + else + writel(out[0], master->regs + SVC_I3C_MWDATAB); + out++; + } + xfer_len -= count; + } + ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); if (ret) @@ -1308,6 +1344,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, emit_stop: svc_i3c_master_emit_stop(master); svc_i3c_master_clear_merrwarn(master); + svc_i3c_master_flush_fifo(master); return ret; } @@ -1877,6 +1914,9 @@ static int svc_i3c_master_probe(struct platform_device *pdev) svc_i3c_master_reset(master); + if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; + /* Register the master */ ret = i3c_master_register(&master->base, &pdev->dev, &svc_i3c_master_ops, false); -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 8:28 ` Miquel Raynal -1 siblings, 0 replies; 16+ messages in thread From: Miquel Raynal @ 2025-02-19 8:28 UTC (permalink / raw) To: Stanley Chu Cc: frank.li, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu Hello Stanley, On 19/02/2025 at 09:10:18 +08, Stanley Chu <stanley.chuys@gmail.com> wrote: > npcm845 HW issue: > I3C HW stalls the write transfer if the transmit FIFO becomes empty, > when new data is written to FIFO, I3C HW resumes the transfer but > the first transmitted data bit may have the wrong value. > Workaround: > Fill the FIFO in advance to prevent FIFO from becoming empty. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- ... > @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > unsigned int dev_nb = 0, last_addr = 0; > u32 reg; > int ret, i; > + u32 dyn_addr; > > while (true) { > /* clean SVC_I3C_MINT_IBIWON w1c bits */ > @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (SVC_I3C_MSTATUS_RXPEND(reg)) { > u8 data[6]; > > + /* > + * SVC_I3C_QUIRK_FIFO_EMPTY fix: > + * The TX FIFO should be ready before the TX transmission is starting. > + */ > + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); > + if (dyn_addr < 0) > + return dyn_addr; > + writel(dyn_addr, master->regs + SVC_I3C_MWDATAB); Maybe Frank can test, but it feels like there are chances it will break the other controller. Perhaps the other quirk applies to the other controller though. Regarding your series, patch 1 does not make much sense alone, and you should anyway write a binding patch for your new compatible (first in the series). Thanks, Miquèl -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue @ 2025-02-19 8:28 ` Miquel Raynal 0 siblings, 0 replies; 16+ messages in thread From: Miquel Raynal @ 2025-02-19 8:28 UTC (permalink / raw) To: Stanley Chu Cc: frank.li, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu Hello Stanley, On 19/02/2025 at 09:10:18 +08, Stanley Chu <stanley.chuys@gmail.com> wrote: > npcm845 HW issue: > I3C HW stalls the write transfer if the transmit FIFO becomes empty, > when new data is written to FIFO, I3C HW resumes the transfer but > the first transmitted data bit may have the wrong value. > Workaround: > Fill the FIFO in advance to prevent FIFO from becoming empty. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- ... > @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > unsigned int dev_nb = 0, last_addr = 0; > u32 reg; > int ret, i; > + u32 dyn_addr; > > while (true) { > /* clean SVC_I3C_MINT_IBIWON w1c bits */ > @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (SVC_I3C_MSTATUS_RXPEND(reg)) { > u8 data[6]; > > + /* > + * SVC_I3C_QUIRK_FIFO_EMPTY fix: > + * The TX FIFO should be ready before the TX transmission is starting. > + */ > + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); > + if (dyn_addr < 0) > + return dyn_addr; > + writel(dyn_addr, master->regs + SVC_I3C_MWDATAB); Maybe Frank can test, but it feels like there are chances it will break the other controller. Perhaps the other quirk applies to the other controller though. Regarding your series, patch 1 does not make much sense alone, and you should anyway write a binding patch for your new compatible (first in the series). Thanks, Miquèl ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 21:09 ` Frank Li -1 siblings, 0 replies; 16+ messages in thread From: Frank Li @ 2025-02-19 21:09 UTC (permalink / raw) To: Stanley Chu Cc: miquel.raynal, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu On Wed, Feb 19, 2025 at 09:10:18AM +0800, Stanley Chu wrote: > npcm845 HW issue: > I3C HW stalls the write transfer if the transmit FIFO becomes empty, > when new data is written to FIFO, I3C HW resumes the transfer but > the first transmitted data bit may have the wrong value. > Workaround: > Fill the FIFO in advance to prevent FIFO from becoming empty. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- > drivers/i3c/master/svc-i3c-master.c | 58 ++++++++++++++++++++++++----- > 1 file changed, 49 insertions(+), 9 deletions(-) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index 813839498b2b..22cb1f1c1fdc 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -102,6 +102,7 @@ > #define SVC_I3C_MDATACTRL_TXTRIG_FIFO_NOT_FULL GENMASK(5, 4) > #define SVC_I3C_MDATACTRL_RXTRIG_FIFO_NOT_EMPTY 0 > #define SVC_I3C_MDATACTRL_RXCOUNT(x) FIELD_GET(GENMASK(28, 24), (x)) > +#define SVC_I3C_MDATACTRL_TXCOUNT(x) FIELD_GET(GENMASK(20, 16), (x)) > #define SVC_I3C_MDATACTRL_TXFULL BIT(30) > #define SVC_I3C_MDATACTRL_RXEMPTY BIT(31) > > @@ -133,6 +134,16 @@ > #define SVC_I3C_EVENT_IBI GENMASK(7, 0) > #define SVC_I3C_EVENT_HOTJOIN BIT(31) > > +/* > + * SVC_I3C_QUIRK_FIFO_EMPTY: > + * I3C HW stalls the write transfer if the transmit FIFO becomes empty, > + * when new data is written to FIFO, I3C HW resumes the transfer but > + * the first transmitted data bit may have the wrong value. > + * Workaround: > + * Fill the FIFO in advance to prevent FIFO from becoming empty. > + */ > +#define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) > + > struct svc_i3c_cmd { > u8 addr; > bool rnw; > @@ -216,6 +227,7 @@ struct svc_i3c_master { > struct mutex lock; > u32 enabled_events; > u32 mctrl_config; > + u32 quirks; > }; > > /** > @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > unsigned int dev_nb = 0, last_addr = 0; > u32 reg; > int ret, i; > + u32 dyn_addr; > > while (true) { > /* clean SVC_I3C_MINT_IBIWON w1c bits */ > @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (SVC_I3C_MSTATUS_RXPEND(reg)) { > u8 data[6]; > > + /* > + * SVC_I3C_QUIRK_FIFO_EMPTY fix: > + * The TX FIFO should be ready before the TX transmission is starting. > + */ > + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); > + if (dyn_addr < 0) > + return dyn_addr; > + writel(dyn_addr, master->regs + SVC_I3C_MWDATAB); > + I think it should work for new svc controller. Fill data advance to avoid stall SCL clock. If it is true, comments/commit message need be updated. > /* > * We only care about the 48-bit provisioned ID yet to > * be sure a device does not nack an address twice. > @@ -1009,21 +1031,20 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (ret) > break; > > - /* Give the slave device a suitable dynamic address */ > - ret = i3c_master_get_free_addr(&master->base, last_addr + 1); > - if (ret < 0) > - break; > - > - addrs[dev_nb] = ret; > + addrs[dev_nb] = dyn_addr; > dev_dbg(master->dev, "DAA: device %d assigned to 0x%02x\n", > dev_nb, addrs[dev_nb]); > - > - writel(addrs[dev_nb], master->regs + SVC_I3C_MWDATAB); > last_addr = addrs[dev_nb++]; > } > > /* Need manual issue STOP except for Complete condition */ > svc_i3c_master_emit_stop(master); > + /* > + * Dynamic address is written to FIFO in advance for SVC_I3C_QUIRK_FIFO_EMPTY fix. > + * Flush FIFO in the failed case. > + */ > + if (ret) > + svc_i3c_master_flush_fifo(master); Needn't check ret, flash fifo unconditional should be fine and avoid left any data in fifo. > return ret; > } > > @@ -1210,7 +1231,6 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > /* clean SVC_I3C_MINT_IBIWON w1c bits */ > writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); > > - > while (retry--) { > writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | > xfer_type | > @@ -1220,6 +1240,22 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > SVC_I3C_MCTRL_RDTERM(*actual_len), > master->regs + SVC_I3C_MCTRL); > > + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { > + int i, count, space; > + > + reg = readl(master->regs + SVC_I3C_MDATACTRL); > + space = SVC_I3C_FIFO_SIZE - SVC_I3C_MDATACTRL_TXCOUNT(reg); > + count = xfer_len > space ? space : xfer_len; > + for (i = 0; i < count; i++) { > + if (i == xfer_len - 1) > + writel(out[0], master->regs + SVC_I3C_MWDATABE); > + else > + writel(out[0], master->regs + SVC_I3C_MWDATAB); > + out++; > + } > + xfer_len -= count; I think you can call svc_i3c_master_write(), or improve svc_i3c_master_write(..., bool b_max_space) ret = svc_i3c_master_write(master, out, count); xfer_len -= ret; out += ret; > + } > + > ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, > SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); > if (ret) > @@ -1308,6 +1344,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > emit_stop: > svc_i3c_master_emit_stop(master); > svc_i3c_master_clear_merrwarn(master); > + svc_i3c_master_flush_fifo(master); > > return ret; > } > @@ -1877,6 +1914,9 @@ static int svc_i3c_master_probe(struct platform_device *pdev) > > svc_i3c_master_reset(master); > > + if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) > + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; > + Generally, it'd better to add drvdata such as struct svc_i3c_drvdata { u32 quirks; }; const struct svc_i3c_drvdata nutoton = { .quirks = SVC_I3C_QUIRK_FIFO_EMPTY; } static const struct of_device_id svc_i3c_master_of_match_tbl[] = { ... { .compatible = "nuvoton,npcm845-i3c", data = &nutoton }, ... } Frank > /* Register the master */ > ret = i3c_master_register(&master->base, &pdev->dev, > &svc_i3c_master_ops, false); > -- > 2.34.1 > -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue @ 2025-02-19 21:09 ` Frank Li 0 siblings, 0 replies; 16+ messages in thread From: Frank Li @ 2025-02-19 21:09 UTC (permalink / raw) To: Stanley Chu Cc: miquel.raynal, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu On Wed, Feb 19, 2025 at 09:10:18AM +0800, Stanley Chu wrote: > npcm845 HW issue: > I3C HW stalls the write transfer if the transmit FIFO becomes empty, > when new data is written to FIFO, I3C HW resumes the transfer but > the first transmitted data bit may have the wrong value. > Workaround: > Fill the FIFO in advance to prevent FIFO from becoming empty. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- > drivers/i3c/master/svc-i3c-master.c | 58 ++++++++++++++++++++++++----- > 1 file changed, 49 insertions(+), 9 deletions(-) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index 813839498b2b..22cb1f1c1fdc 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -102,6 +102,7 @@ > #define SVC_I3C_MDATACTRL_TXTRIG_FIFO_NOT_FULL GENMASK(5, 4) > #define SVC_I3C_MDATACTRL_RXTRIG_FIFO_NOT_EMPTY 0 > #define SVC_I3C_MDATACTRL_RXCOUNT(x) FIELD_GET(GENMASK(28, 24), (x)) > +#define SVC_I3C_MDATACTRL_TXCOUNT(x) FIELD_GET(GENMASK(20, 16), (x)) > #define SVC_I3C_MDATACTRL_TXFULL BIT(30) > #define SVC_I3C_MDATACTRL_RXEMPTY BIT(31) > > @@ -133,6 +134,16 @@ > #define SVC_I3C_EVENT_IBI GENMASK(7, 0) > #define SVC_I3C_EVENT_HOTJOIN BIT(31) > > +/* > + * SVC_I3C_QUIRK_FIFO_EMPTY: > + * I3C HW stalls the write transfer if the transmit FIFO becomes empty, > + * when new data is written to FIFO, I3C HW resumes the transfer but > + * the first transmitted data bit may have the wrong value. > + * Workaround: > + * Fill the FIFO in advance to prevent FIFO from becoming empty. > + */ > +#define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) > + > struct svc_i3c_cmd { > u8 addr; > bool rnw; > @@ -216,6 +227,7 @@ struct svc_i3c_master { > struct mutex lock; > u32 enabled_events; > u32 mctrl_config; > + u32 quirks; > }; > > /** > @@ -891,6 +903,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > unsigned int dev_nb = 0, last_addr = 0; > u32 reg; > int ret, i; > + u32 dyn_addr; > > while (true) { > /* clean SVC_I3C_MINT_IBIWON w1c bits */ > @@ -931,6 +944,15 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (SVC_I3C_MSTATUS_RXPEND(reg)) { > u8 data[6]; > > + /* > + * SVC_I3C_QUIRK_FIFO_EMPTY fix: > + * The TX FIFO should be ready before the TX transmission is starting. > + */ > + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); > + if (dyn_addr < 0) > + return dyn_addr; > + writel(dyn_addr, master->regs + SVC_I3C_MWDATAB); > + I think it should work for new svc controller. Fill data advance to avoid stall SCL clock. If it is true, comments/commit message need be updated. > /* > * We only care about the 48-bit provisioned ID yet to > * be sure a device does not nack an address twice. > @@ -1009,21 +1031,20 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (ret) > break; > > - /* Give the slave device a suitable dynamic address */ > - ret = i3c_master_get_free_addr(&master->base, last_addr + 1); > - if (ret < 0) > - break; > - > - addrs[dev_nb] = ret; > + addrs[dev_nb] = dyn_addr; > dev_dbg(master->dev, "DAA: device %d assigned to 0x%02x\n", > dev_nb, addrs[dev_nb]); > - > - writel(addrs[dev_nb], master->regs + SVC_I3C_MWDATAB); > last_addr = addrs[dev_nb++]; > } > > /* Need manual issue STOP except for Complete condition */ > svc_i3c_master_emit_stop(master); > + /* > + * Dynamic address is written to FIFO in advance for SVC_I3C_QUIRK_FIFO_EMPTY fix. > + * Flush FIFO in the failed case. > + */ > + if (ret) > + svc_i3c_master_flush_fifo(master); Needn't check ret, flash fifo unconditional should be fine and avoid left any data in fifo. > return ret; > } > > @@ -1210,7 +1231,6 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > /* clean SVC_I3C_MINT_IBIWON w1c bits */ > writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); > > - > while (retry--) { > writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | > xfer_type | > @@ -1220,6 +1240,22 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > SVC_I3C_MCTRL_RDTERM(*actual_len), > master->regs + SVC_I3C_MCTRL); > > + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { > + int i, count, space; > + > + reg = readl(master->regs + SVC_I3C_MDATACTRL); > + space = SVC_I3C_FIFO_SIZE - SVC_I3C_MDATACTRL_TXCOUNT(reg); > + count = xfer_len > space ? space : xfer_len; > + for (i = 0; i < count; i++) { > + if (i == xfer_len - 1) > + writel(out[0], master->regs + SVC_I3C_MWDATABE); > + else > + writel(out[0], master->regs + SVC_I3C_MWDATAB); > + out++; > + } > + xfer_len -= count; I think you can call svc_i3c_master_write(), or improve svc_i3c_master_write(..., bool b_max_space) ret = svc_i3c_master_write(master, out, count); xfer_len -= ret; out += ret; > + } > + > ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, > SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); > if (ret) > @@ -1308,6 +1344,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > emit_stop: > svc_i3c_master_emit_stop(master); > svc_i3c_master_clear_merrwarn(master); > + svc_i3c_master_flush_fifo(master); > > return ret; > } > @@ -1877,6 +1914,9 @@ static int svc_i3c_master_probe(struct platform_device *pdev) > > svc_i3c_master_reset(master); > > + if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) > + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; > + Generally, it'd better to add drvdata such as struct svc_i3c_drvdata { u32 quirks; }; const struct svc_i3c_drvdata nutoton = { .quirks = SVC_I3C_QUIRK_FIFO_EMPTY; } static const struct of_device_id svc_i3c_master_of_match_tbl[] = { ... { .compatible = "nuvoton,npcm845-i3c", data = &nutoton }, ... } Frank > /* Register the master */ > ret = i3c_master_register(&master->base, &pdev->dev, > &svc_i3c_master_ops, false); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 3/3] i3c: master: svc: fix npcm845 invalid slvstart event 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 1:10 ` Stanley Chu -1 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu I3C HW may generate an invalid SlvStart event when emitting a STOP. If it is a true SlvStart, the MSTATUS state should be SLVREQ. Check the MSTATUS state to ignore the false event. Signed-off-by: Stanley Chu <yschu@nuvoton.com> --- drivers/i3c/master/svc-i3c-master.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 22cb1f1c1fdc..14cedcb81c52 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -59,6 +59,7 @@ #define SVC_I3C_MSTATUS_STATE(x) FIELD_GET(GENMASK(2, 0), (x)) #define SVC_I3C_MSTATUS_STATE_DAA(x) (SVC_I3C_MSTATUS_STATE(x) == 5) #define SVC_I3C_MSTATUS_STATE_IDLE(x) (SVC_I3C_MSTATUS_STATE(x) == 0) +#define SVC_I3C_MSTATUS_STATE_SLVREQ(x) (SVC_I3C_MSTATUS_STATE(x) == 1) #define SVC_I3C_MSTATUS_BETWEEN(x) FIELD_GET(BIT(4), (x)) #define SVC_I3C_MSTATUS_NACKED(x) FIELD_GET(BIT(5), (x)) #define SVC_I3C_MSTATUS_IBITYPE(x) FIELD_GET(GENMASK(7, 6), (x)) @@ -143,6 +144,12 @@ * Fill the FIFO in advance to prevent FIFO from becoming empty. */ #define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) +/* + * SVC_I3C_QUIRK_FLASE_SLVSTART: + * I3C HW may generate an invalid SlvStart event when emitting a STOP. + * If it is a true SlvStart, the MSTATUS state should be SLVREQ. + */ +#define SVC_I3C_QUIRK_FALSE_SLVSTART BIT(1) struct svc_i3c_cmd { u8 addr; @@ -576,6 +583,11 @@ static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id) /* Clear the interrupt status */ writel(SVC_I3C_MINT_SLVSTART, master->regs + SVC_I3C_MSTATUS); + /* Ignore the false event */ + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && + !SVC_I3C_MSTATUS_STATE_SLVREQ(active)) + return IRQ_HANDLED; + svc_i3c_master_disable_interrupts(master); /* Handle the interrupt in a non atomic context */ @@ -1915,7 +1927,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev) svc_i3c_master_reset(master); if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) - master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY | + SVC_I3C_QUIRK_FALSE_SLVSTART; /* Register the master */ ret = i3c_master_register(&master->base, &pdev->dev, -- 2.34.1 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 3/3] i3c: master: svc: fix npcm845 invalid slvstart event @ 2025-02-19 1:10 ` Stanley Chu 0 siblings, 0 replies; 16+ messages in thread From: Stanley Chu @ 2025-02-19 1:10 UTC (permalink / raw) To: frank.li, miquel.raynal, alexandre.belloni, linux-i3c Cc: linux-kernel, tomer.maimon, kwliu, yschu I3C HW may generate an invalid SlvStart event when emitting a STOP. If it is a true SlvStart, the MSTATUS state should be SLVREQ. Check the MSTATUS state to ignore the false event. Signed-off-by: Stanley Chu <yschu@nuvoton.com> --- drivers/i3c/master/svc-i3c-master.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 22cb1f1c1fdc..14cedcb81c52 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -59,6 +59,7 @@ #define SVC_I3C_MSTATUS_STATE(x) FIELD_GET(GENMASK(2, 0), (x)) #define SVC_I3C_MSTATUS_STATE_DAA(x) (SVC_I3C_MSTATUS_STATE(x) == 5) #define SVC_I3C_MSTATUS_STATE_IDLE(x) (SVC_I3C_MSTATUS_STATE(x) == 0) +#define SVC_I3C_MSTATUS_STATE_SLVREQ(x) (SVC_I3C_MSTATUS_STATE(x) == 1) #define SVC_I3C_MSTATUS_BETWEEN(x) FIELD_GET(BIT(4), (x)) #define SVC_I3C_MSTATUS_NACKED(x) FIELD_GET(BIT(5), (x)) #define SVC_I3C_MSTATUS_IBITYPE(x) FIELD_GET(GENMASK(7, 6), (x)) @@ -143,6 +144,12 @@ * Fill the FIFO in advance to prevent FIFO from becoming empty. */ #define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) +/* + * SVC_I3C_QUIRK_FLASE_SLVSTART: + * I3C HW may generate an invalid SlvStart event when emitting a STOP. + * If it is a true SlvStart, the MSTATUS state should be SLVREQ. + */ +#define SVC_I3C_QUIRK_FALSE_SLVSTART BIT(1) struct svc_i3c_cmd { u8 addr; @@ -576,6 +583,11 @@ static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id) /* Clear the interrupt status */ writel(SVC_I3C_MINT_SLVSTART, master->regs + SVC_I3C_MSTATUS); + /* Ignore the false event */ + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && + !SVC_I3C_MSTATUS_STATE_SLVREQ(active)) + return IRQ_HANDLED; + svc_i3c_master_disable_interrupts(master); /* Handle the interrupt in a non atomic context */ @@ -1915,7 +1927,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev) svc_i3c_master_reset(master); if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) - master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY | + SVC_I3C_QUIRK_FALSE_SLVSTART; /* Register the master */ ret = i3c_master_register(&master->base, &pdev->dev, -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/3] i3c: master: svc: fix npcm845 invalid slvstart event 2025-02-19 1:10 ` Stanley Chu @ 2025-02-19 21:11 ` Frank Li -1 siblings, 0 replies; 16+ messages in thread From: Frank Li @ 2025-02-19 21:11 UTC (permalink / raw) To: Stanley Chu Cc: miquel.raynal, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu On Wed, Feb 19, 2025 at 09:10:19AM +0800, Stanley Chu wrote: > I3C HW may generate an invalid SlvStart event when emitting a STOP. > If it is a true SlvStart, the MSTATUS state should be SLVREQ. > Check the MSTATUS state to ignore the false event. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- > drivers/i3c/master/svc-i3c-master.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index 22cb1f1c1fdc..14cedcb81c52 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -59,6 +59,7 @@ > #define SVC_I3C_MSTATUS_STATE(x) FIELD_GET(GENMASK(2, 0), (x)) > #define SVC_I3C_MSTATUS_STATE_DAA(x) (SVC_I3C_MSTATUS_STATE(x) == 5) > #define SVC_I3C_MSTATUS_STATE_IDLE(x) (SVC_I3C_MSTATUS_STATE(x) == 0) > +#define SVC_I3C_MSTATUS_STATE_SLVREQ(x) (SVC_I3C_MSTATUS_STATE(x) == 1) > #define SVC_I3C_MSTATUS_BETWEEN(x) FIELD_GET(BIT(4), (x)) > #define SVC_I3C_MSTATUS_NACKED(x) FIELD_GET(BIT(5), (x)) > #define SVC_I3C_MSTATUS_IBITYPE(x) FIELD_GET(GENMASK(7, 6), (x)) > @@ -143,6 +144,12 @@ > * Fill the FIFO in advance to prevent FIFO from becoming empty. > */ > #define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) > +/* > + * SVC_I3C_QUIRK_FLASE_SLVSTART: > + * I3C HW may generate an invalid SlvStart event when emitting a STOP. > + * If it is a true SlvStart, the MSTATUS state should be SLVREQ. > + */ > +#define SVC_I3C_QUIRK_FALSE_SLVSTART BIT(1) > > struct svc_i3c_cmd { > u8 addr; > @@ -576,6 +583,11 @@ static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id) > /* Clear the interrupt status */ > writel(SVC_I3C_MINT_SLVSTART, master->regs + SVC_I3C_MSTATUS); > > + /* Ignore the false event */ > + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && You should check SVC_I3C_QUIRK_FALSE_SLVSTART, not SVC_I3C_QUIRK_FIFO_EMPTY Frank > + !SVC_I3C_MSTATUS_STATE_SLVREQ(active)) > + return IRQ_HANDLED; > + > svc_i3c_master_disable_interrupts(master); > > /* Handle the interrupt in a non atomic context */ > @@ -1915,7 +1927,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev) > svc_i3c_master_reset(master); > > if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) > - master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; > + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY | > + SVC_I3C_QUIRK_FALSE_SLVSTART; > > /* Register the master */ > ret = i3c_master_register(&master->base, &pdev->dev, > -- > 2.34.1 > -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 3/3] i3c: master: svc: fix npcm845 invalid slvstart event @ 2025-02-19 21:11 ` Frank Li 0 siblings, 0 replies; 16+ messages in thread From: Frank Li @ 2025-02-19 21:11 UTC (permalink / raw) To: Stanley Chu Cc: miquel.raynal, alexandre.belloni, linux-i3c, linux-kernel, tomer.maimon, kwliu, yschu On Wed, Feb 19, 2025 at 09:10:19AM +0800, Stanley Chu wrote: > I3C HW may generate an invalid SlvStart event when emitting a STOP. > If it is a true SlvStart, the MSTATUS state should be SLVREQ. > Check the MSTATUS state to ignore the false event. > > Signed-off-by: Stanley Chu <yschu@nuvoton.com> > --- > drivers/i3c/master/svc-i3c-master.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index 22cb1f1c1fdc..14cedcb81c52 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -59,6 +59,7 @@ > #define SVC_I3C_MSTATUS_STATE(x) FIELD_GET(GENMASK(2, 0), (x)) > #define SVC_I3C_MSTATUS_STATE_DAA(x) (SVC_I3C_MSTATUS_STATE(x) == 5) > #define SVC_I3C_MSTATUS_STATE_IDLE(x) (SVC_I3C_MSTATUS_STATE(x) == 0) > +#define SVC_I3C_MSTATUS_STATE_SLVREQ(x) (SVC_I3C_MSTATUS_STATE(x) == 1) > #define SVC_I3C_MSTATUS_BETWEEN(x) FIELD_GET(BIT(4), (x)) > #define SVC_I3C_MSTATUS_NACKED(x) FIELD_GET(BIT(5), (x)) > #define SVC_I3C_MSTATUS_IBITYPE(x) FIELD_GET(GENMASK(7, 6), (x)) > @@ -143,6 +144,12 @@ > * Fill the FIFO in advance to prevent FIFO from becoming empty. > */ > #define SVC_I3C_QUIRK_FIFO_EMPTY BIT(0) > +/* > + * SVC_I3C_QUIRK_FLASE_SLVSTART: > + * I3C HW may generate an invalid SlvStart event when emitting a STOP. > + * If it is a true SlvStart, the MSTATUS state should be SLVREQ. > + */ > +#define SVC_I3C_QUIRK_FALSE_SLVSTART BIT(1) > > struct svc_i3c_cmd { > u8 addr; > @@ -576,6 +583,11 @@ static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id) > /* Clear the interrupt status */ > writel(SVC_I3C_MINT_SLVSTART, master->regs + SVC_I3C_MSTATUS); > > + /* Ignore the false event */ > + if ((master->quirks & SVC_I3C_QUIRK_FIFO_EMPTY) && You should check SVC_I3C_QUIRK_FALSE_SLVSTART, not SVC_I3C_QUIRK_FIFO_EMPTY Frank > + !SVC_I3C_MSTATUS_STATE_SLVREQ(active)) > + return IRQ_HANDLED; > + > svc_i3c_master_disable_interrupts(master); > > /* Handle the interrupt in a non atomic context */ > @@ -1915,7 +1927,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev) > svc_i3c_master_reset(master); > > if (device_is_compatible(master->dev, "nuvoton,npcm845-i3c")) > - master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY; > + master->quirks = SVC_I3C_QUIRK_FIFO_EMPTY | > + SVC_I3C_QUIRK_FALSE_SLVSTART; > > /* Register the master */ > ret = i3c_master_register(&master->base, &pdev->dev, > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-02-19 21:18 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-19 1:10 [PATCH v1 0/3] Add support for Nuvoton npcm845 i3c controller Stanley Chu 2025-02-19 1:10 ` Stanley Chu 2025-02-19 1:10 ` [PATCH v1 1/3] i3c: master: svc: add support for Nuvoton npcm845 i3c Stanley Chu 2025-02-19 1:10 ` Stanley Chu 2025-02-19 20:43 ` Frank Li 2025-02-19 20:43 ` Frank Li 2025-02-19 1:10 ` [PATCH v1 2/3] i3c: master: svc: fix npcm845 FIFO empty issue Stanley Chu 2025-02-19 1:10 ` Stanley Chu 2025-02-19 8:28 ` Miquel Raynal 2025-02-19 8:28 ` Miquel Raynal 2025-02-19 21:09 ` Frank Li 2025-02-19 21:09 ` Frank Li 2025-02-19 1:10 ` [PATCH v1 3/3] i3c: master: svc: fix npcm845 invalid slvstart event Stanley Chu 2025-02-19 1:10 ` Stanley Chu 2025-02-19 21:11 ` Frank Li 2025-02-19 21:11 ` Frank Li
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.