Message ID | 20231016153232.2851095-3-Frank.Li@nxp.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | i3c: master: svc: collection of bugs fixes | expand |
Hi Frank, Frank.Li@nxp.com wrote on Mon, 16 Oct 2023 11:32:28 -0400: > ┌─────┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┌───── > SCL: ┘ └─────┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┘ > ───┐ ┌─────┐ ┌─────┐ ┌───────────┐ > SDA: └───────────────────────┘ └─────┘ └─────┘ └───── > xxx╱ ╲╱ ╲╱ ╲╱ ╲╱ ╲ > : xxx╲IBI ╱╲ Addr(0x0a) ╱╲ RW ╱╲NACK╱╲ S ╱ > > In-Band Interrupt (IBI) occurred and IBI work thread may not to be If an In-Band... occurs and the IBI work thread is not immediately scheduled, when svc... initiates an I3C transfer and attempts... > scheduled. When svc_i3c_master_priv_xfers() initiates the I3C transfer > frame and attempts to send address 0x7e, the target interprets it as an > IBI handler and returns the target address 0x0a. > > However, svc_i3c_master_priv_xfers() does not handle this case and proceeds > with other transfers, resulting in incorrect data being returned. > > IBIWON check has been added in svc_i3c_master_xfer(). In case this Add IBIWON check in svc_... > situation occurs, a failure is now returned to the driver. return a failure... > > Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") > Cc: stable@vger.kernel.org > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/i3c/master/svc-i3c-master.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c > index ebdb3ea1af9d..0f57a5f75e39 100644 > --- a/drivers/i3c/master/svc-i3c-master.c > +++ b/drivers/i3c/master/svc-i3c-master.c > @@ -1009,6 +1009,9 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > u32 reg; > int ret; > > + /* clean SVC_I3C_MINT_IBIWON w1c bits */ > + writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); > + > writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | > xfer_type | > SVC_I3C_MCTRL_IBIRESP_NACK | > @@ -1027,6 +1030,23 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, > goto emit_stop; > } > > + /* > + * According to I3C spec ver 1.1.1, 5.1.2.2.3 Consequence of Controller Starting a Frame > + * with I3C Target Address. > + * > + * The I3C Controller normally should start a Frame, the Address may be arbitrated, and so > + * the Controller shall monitor to see whether an In-Band Interrupt request, a Controller > + * Role Request (i.e., Secondary Controller requests to become the Active Controller), or > + * a Hot-Join Request has been made. > + * > + * If missed IBIWON check, the wrong data will be return. When IBIWON happen, return falure > + * and yeild the above events handler. Typos: yeild and falure > + */ > + if (SVC_I3C_MSTATUS_IBIWON(reg)) { > + ret = -ENXIO; > + goto emit_stop; > + } > + > if (rnw) > ret = svc_i3c_master_read(master, in, xfer_len); > else With all the typos fixed: Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c index ebdb3ea1af9d..0f57a5f75e39 100644 --- a/drivers/i3c/master/svc-i3c-master.c +++ b/drivers/i3c/master/svc-i3c-master.c @@ -1009,6 +1009,9 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, u32 reg; int ret; + /* clean SVC_I3C_MINT_IBIWON w1c bits */ + writel(SVC_I3C_MINT_IBIWON, master->regs + SVC_I3C_MSTATUS); + writel(SVC_I3C_MCTRL_REQUEST_START_ADDR | xfer_type | SVC_I3C_MCTRL_IBIRESP_NACK | @@ -1027,6 +1030,23 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master, goto emit_stop; } + /* + * According to I3C spec ver 1.1.1, 5.1.2.2.3 Consequence of Controller Starting a Frame + * with I3C Target Address. + * + * The I3C Controller normally should start a Frame, the Address may be arbitrated, and so + * the Controller shall monitor to see whether an In-Band Interrupt request, a Controller + * Role Request (i.e., Secondary Controller requests to become the Active Controller), or + * a Hot-Join Request has been made. + * + * If missed IBIWON check, the wrong data will be return. When IBIWON happen, return falure + * and yeild the above events handler. + */ + if (SVC_I3C_MSTATUS_IBIWON(reg)) { + ret = -ENXIO; + goto emit_stop; + } + if (rnw) ret = svc_i3c_master_read(master, in, xfer_len); else
┌─────┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┏──┐ ┌───── SCL: ┘ └─────┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┛ └──┘ ───┐ ┌─────┐ ┌─────┐ ┌───────────┐ SDA: └───────────────────────┘ └─────┘ └─────┘ └───── xxx╱ ╲╱ ╲╱ ╲╱ ╲╱ ╲ : xxx╲IBI ╱╲ Addr(0x0a) ╱╲ RW ╱╲NACK╱╲ S ╱ In-Band Interrupt (IBI) occurred and IBI work thread may not to be scheduled. When svc_i3c_master_priv_xfers() initiates the I3C transfer frame and attempts to send address 0x7e, the target interprets it as an IBI handler and returns the target address 0x0a. However, svc_i3c_master_priv_xfers() does not handle this case and proceeds with other transfers, resulting in incorrect data being returned. IBIWON check has been added in svc_i3c_master_xfer(). In case this situation occurs, a failure is now returned to the driver. Fixes: dd3c52846d59 ("i3c: master: svc: Add Silvaco I3C master driver") Cc: stable@vger.kernel.org Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/i3c/master/svc-i3c-master.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)