| Message ID | 20250227060131.2206860-4-yschu@nuvoton.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Nuvoton npcm845 i3c controller | expand |
On Thu, Feb 27, 2025 at 02:01:29PM +0800, Stanley Chu wrote: > From: Stanley Chu <yschu@nuvoton.com> > > 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. > 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 | 52 ++++++++++++++++++++++++----- > 1 file changed, 43 insertions(+), 9 deletions(-) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index 9143a079de53..a0c6d8053a25 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -114,6 +114,7 @@ > #define SVC_I3C_MWDATAHE 0x0BC > #define SVC_I3C_MRDATAB 0x0C0 > #define SVC_I3C_MRDATAH 0x0C8 > +#define SVC_I3C_MWDATAB1 0x0CC > #define SVC_I3C_MWMSG_SDR 0x0D0 > #define SVC_I3C_MRMSG_SDR 0x0D4 > #define SVC_I3C_MWMSG_DDR 0x0D8 > @@ -940,7 +941,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > u8 *addrs, unsigned int *count) > { > u64 prov_id[SVC_I3C_MAX_DEVS] = {}, nacking_prov_id = 0; > - unsigned int dev_nb = 0, last_addr = 0; > + unsigned int dev_nb = 0, last_addr = 0, dyn_addr; > u32 reg; > int ret, i; > > @@ -983,6 +984,25 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > if (SVC_I3C_MSTATUS_RXPEND(reg)) { > u8 data[6]; > > + /* > + * One slave sends its ID to request for address assignment, > + * prefilling the dynamic address can reduce SCL clock stalls > + * and also fix the SVC_I3C_QUIRK_FIFO_EMPTY quirk. > + * > + * Ideally, prefilling before the processDAA command is better. > + * However, it requires an additional check to write the dyn_addr > + * at the right time because the driver needs to write the processDAA > + * command twice for one assignment. > + * Prefilling here is safe and efficient because the FIFO starts > + * filling within a few hundred nanoseconds, which is significantly > + * faster compared to the 64 SCL clock cycles. > + */ > + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); > + if (dyn_addr < 0) > + return -ENOSPC; > + > + 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. > @@ -1061,21 +1081,16 @@ 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); > + svc_i3c_master_flush_fifo(master); > + > return ret; > } > > @@ -1272,6 +1287,24 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > SVC_I3C_MCTRL_RDTERM(*actual_len), > master->regs + SVC_I3C_MCTRL); > > + /* > + * The entire transaction can consist of multiple write transfers. > + * Prefilling before EmitStartAddr causes the data to be emitted > + * immediately, becoming part of the previous transfer. > + * The only way to work around this hardware issue is to let the > + * FIFO start filling as soon as possible after EmitStartAddr. > + */ > + if (svc_has_quirk(master, SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { > + u32 end = xfer_len > SVC_I3C_FIFO_SIZE ? 0 : BIT(8); Define BIT(8) as macro Let me find a time at next week to do simple test for this. > + u32 len = min_t(u32, xfer_len, SVC_I3C_FIFO_SIZE); > + > + writesb(master->regs + SVC_I3C_MWDATAB1, out, len - 1); > + /* Mark END bit if this is the last byte */ > + writel(out[len - 1] | end, master->regs + SVC_I3C_MWDATAB); > + xfer_len -= len; > + out += len; > + } > + > ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, > SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); > if (ret) > @@ -1360,6 +1393,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; > } > -- > 2.34.1 >
On Thu, Feb 27, 2025 at 12:23:18PM -0500, Frank Li wrote: > On Thu, Feb 27, 2025 at 02:01:29PM +0800, Stanley Chu wrote: > > From: Stanley Chu <yschu@nuvoton.com> > > > > 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. > > 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 | 52 ++++++++++++++++++++++++----- > > 1 file changed, 43 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > > index 9143a079de53..a0c6d8053a25 100644 > > --- a/drivers/i3c/master/svc-i3c-master.c > > +++ b/drivers/i3c/master/svc-i3c-master.c > > @@ -114,6 +114,7 @@ > > #define SVC_I3C_MWDATAHE 0x0BC > > #define SVC_I3C_MRDATAB 0x0C0 > > #define SVC_I3C_MRDATAH 0x0C8 > > +#define SVC_I3C_MWDATAB1 0x0CC > > #define SVC_I3C_MWMSG_SDR 0x0D0 > > #define SVC_I3C_MRMSG_SDR 0x0D4 > > #define SVC_I3C_MWMSG_DDR 0x0D8 > > @@ -940,7 +941,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > > u8 *addrs, unsigned int *count) > > { > > u64 prov_id[SVC_I3C_MAX_DEVS] = {}, nacking_prov_id = 0; > > - unsigned int dev_nb = 0, last_addr = 0; > > + unsigned int dev_nb = 0, last_addr = 0, dyn_addr; > > u32 reg; > > int ret, i; > > > > @@ -983,6 +984,25 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, > > if (SVC_I3C_MSTATUS_RXPEND(reg)) { > > u8 data[6]; > > > > + /* > > + * One slave sends its ID to request for address assignment, > > + * prefilling the dynamic address can reduce SCL clock stalls > > + * and also fix the SVC_I3C_QUIRK_FIFO_EMPTY quirk. > > + * > > + * Ideally, prefilling before the processDAA command is better. > > + * However, it requires an additional check to write the dyn_addr > > + * at the right time because the driver needs to write the processDAA > > + * command twice for one assignment. > > + * Prefilling here is safe and efficient because the FIFO starts > > + * filling within a few hundred nanoseconds, which is significantly > > + * faster compared to the 64 SCL clock cycles. > > + */ > > + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); > > + if (dyn_addr < 0) > > + return -ENOSPC; > > + > > + 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. > > @@ -1061,21 +1081,16 @@ 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); > > + svc_i3c_master_flush_fifo(master); > > + > > return ret; > > } > > > > @@ -1272,6 +1287,24 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > > SVC_I3C_MCTRL_RDTERM(*actual_len), > > master->regs + SVC_I3C_MCTRL); > > > > + /* > > + * The entire transaction can consist of multiple write transfers. > > + * Prefilling before EmitStartAddr causes the data to be emitted > > + * immediately, becoming part of the previous transfer. > > + * The only way to work around this hardware issue is to let the > > + * FIFO start filling as soon as possible after EmitStartAddr. > > + */ > > + if (svc_has_quirk(master, SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { > > + u32 end = xfer_len > SVC_I3C_FIFO_SIZE ? 0 : BIT(8); > > Define BIT(8) as macro > > Let me find a time at next week to do simple test for this. I run basic test, it is fine! Reviewed-by: Frank Li <Frank.Li@nxp.com> > > > + u32 len = min_t(u32, xfer_len, SVC_I3C_FIFO_SIZE); > > + > > + writesb(master->regs + SVC_I3C_MWDATAB1, out, len - 1); > > + /* Mark END bit if this is the last byte */ > > + writel(out[len - 1] | end, master->regs + SVC_I3C_MWDATAB); > > + xfer_len -= len; > > + out += len; > > + } > > + > > ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, > > SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); > > if (ret) > > @@ -1360,6 +1393,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; > > } > > -- > > 2.34.1 > >
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index 9143a079de53..a0c6d8053a25 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -114,6 +114,7 @@ #define SVC_I3C_MWDATAHE 0x0BC #define SVC_I3C_MRDATAB 0x0C0 #define SVC_I3C_MRDATAH 0x0C8 +#define SVC_I3C_MWDATAB1 0x0CC #define SVC_I3C_MWMSG_SDR 0x0D0 #define SVC_I3C_MRMSG_SDR 0x0D4 #define SVC_I3C_MWMSG_DDR 0x0D8 @@ -940,7 +941,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, u8 *addrs, unsigned int *count) { u64 prov_id[SVC_I3C_MAX_DEVS] = {}, nacking_prov_id = 0; - unsigned int dev_nb = 0, last_addr = 0; + unsigned int dev_nb = 0, last_addr = 0, dyn_addr; u32 reg; int ret, i; @@ -983,6 +984,25 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master, if (SVC_I3C_MSTATUS_RXPEND(reg)) { u8 data[6]; + /* + * One slave sends its ID to request for address assignment, + * prefilling the dynamic address can reduce SCL clock stalls + * and also fix the SVC_I3C_QUIRK_FIFO_EMPTY quirk. + * + * Ideally, prefilling before the processDAA command is better. + * However, it requires an additional check to write the dyn_addr + * at the right time because the driver needs to write the processDAA + * command twice for one assignment. + * Prefilling here is safe and efficient because the FIFO starts + * filling within a few hundred nanoseconds, which is significantly + * faster compared to the 64 SCL clock cycles. + */ + dyn_addr = i3c_master_get_free_addr(&master->base, last_addr + 1); + if (dyn_addr < 0) + return -ENOSPC; + + 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. @@ -1061,21 +1081,16 @@ 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); + svc_i3c_master_flush_fifo(master); + return ret; } @@ -1272,6 +1287,24 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, SVC_I3C_MCTRL_RDTERM(*actual_len), master->regs + SVC_I3C_MCTRL); + /* + * The entire transaction can consist of multiple write transfers. + * Prefilling before EmitStartAddr causes the data to be emitted + * immediately, becoming part of the previous transfer. + * The only way to work around this hardware issue is to let the + * FIFO start filling as soon as possible after EmitStartAddr. + */ + if (svc_has_quirk(master, SVC_I3C_QUIRK_FIFO_EMPTY) && !rnw && xfer_len) { + u32 end = xfer_len > SVC_I3C_FIFO_SIZE ? 0 : BIT(8); + u32 len = min_t(u32, xfer_len, SVC_I3C_FIFO_SIZE); + + writesb(master->regs + SVC_I3C_MWDATAB1, out, len - 1); + /* Mark END bit if this is the last byte */ + writel(out[len - 1] | end, master->regs + SVC_I3C_MWDATAB); + xfer_len -= len; + out += len; + } + ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg, SVC_I3C_MSTATUS_MCTRLDONE(reg), 0, 1000); if (ret) @@ -1360,6 +1393,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; }