2021-07-15 09:09:09

by Clark Wang

[permalink] [raw]
Subject: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data

When i3c controller reads data from slave device, slave device can stop
returning data with an ACK after any byte.
Add this support for svc i3c controller. Otherwise, it will go TIMEOUT
error path when the slave device ends the read operation early.

Signed-off-by: Clark Wang <[email protected]>
---
drivers/i3c/master/svc-i3c-master.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 9d80435638ea..892e57fec4b0 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -865,7 +865,7 @@ static int svc_i3c_master_read(struct svc_i3c_master *master,
u8 *in, unsigned int len)
{
int offset = 0, i, ret;
- u32 mdctrl;
+ u32 mdctrl, mstatus;

while (offset < len) {
unsigned int count;
@@ -874,8 +874,15 @@ static int svc_i3c_master_read(struct svc_i3c_master *master,
mdctrl,
!(mdctrl & SVC_I3C_MDATACTRL_RXEMPTY),
0, 1000);
- if (ret)
- return ret;
+ if (ret) {
+ ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
+ mstatus, SVC_I3C_MSTATUS_COMPLETE(mstatus),
+ 0, 1000);
+ if (ret)
+ return ret;
+ else
+ return offset;
+ }

count = SVC_I3C_MDATACTRL_RXCOUNT(mdctrl);
for (i = 0; i < count; i++)
@@ -884,7 +891,7 @@ static int svc_i3c_master_read(struct svc_i3c_master *master,
offset += count;
}

- return 0;
+ return offset;
}

static int svc_i3c_master_write(struct svc_i3c_master *master,
@@ -917,7 +924,7 @@ static int svc_i3c_master_write(struct svc_i3c_master *master,
static int svc_i3c_master_xfer(struct svc_i3c_master *master,
bool rnw, unsigned int xfer_type, u8 addr,
u8 *in, const u8 *out, unsigned int xfer_len,
- unsigned int read_len, bool continued)
+ unsigned int *read_len, bool continued)
{
u32 reg;
int ret;
@@ -927,7 +934,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
SVC_I3C_MCTRL_IBIRESP_NACK |
SVC_I3C_MCTRL_DIR(rnw) |
SVC_I3C_MCTRL_ADDR(addr) |
- SVC_I3C_MCTRL_RDTERM(read_len),
+ SVC_I3C_MCTRL_RDTERM(*read_len),
master->regs + SVC_I3C_MCTRL);

ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
@@ -939,8 +946,10 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
ret = svc_i3c_master_read(master, in, xfer_len);
else
ret = svc_i3c_master_write(master, out, xfer_len);
- if (ret)
+ if (ret < 0)
goto emit_stop;
+ if (rnw)
+ *read_len = ret;

ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_COMPLETE(reg), 0, 1000);
@@ -1012,7 +1021,7 @@ static void svc_i3c_master_start_xfer_locked(struct svc_i3c_master *master)

ret = svc_i3c_master_xfer(master, cmd->rnw, xfer->type,
cmd->addr, cmd->in, cmd->out,
- cmd->len, cmd->read_len,
+ cmd->len, &cmd->read_len,
cmd->continued);
if (ret)
break;
@@ -1141,6 +1150,9 @@ static int svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master,
if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000)))
svc_i3c_master_dequeue_xfer(master, xfer);

+ if (cmd->read_len != xfer_len)
+ ccc->dests[0].payload.len = cmd->read_len;
+
ret = xfer->ret;
svc_i3c_master_free_xfer(xfer);

--
2.25.1


2021-07-15 22:57:25

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data

Hi Clark,

Clark Wang <[email protected]> wrote on Thu, 15 Jul 2021 16:24:12
+0800:

> When i3c controller reads data from slave device, slave device can stop
> returning data with an ACK after any byte.
> Add this support for svc i3c controller. Otherwise, it will go TIMEOUT
> error path when the slave device ends the read operation early.

Is this part of the I3C specification? I am not aware about it.

> Signed-off-by: Clark Wang <[email protected]>
> ---
> drivers/i3c/master/svc-i3c-master.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 9d80435638ea..892e57fec4b0 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -865,7 +865,7 @@ static int svc_i3c_master_read(struct svc_i3c_master *master,
> u8 *in, unsigned int len)
> {
> int offset = 0, i, ret;
> - u32 mdctrl;
> + u32 mdctrl, mstatus;
>
> while (offset < len) {
> unsigned int count;
> @@ -874,8 +874,15 @@ static int svc_i3c_master_read(struct svc_i3c_master *master,
> mdctrl,
> !(mdctrl & SVC_I3C_MDATACTRL_RXEMPTY),
> 0, 1000);
> - if (ret)
> - return ret;
> + if (ret) {
> + ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> + mstatus, SVC_I3C_MSTATUS_COMPLETE(mstatus),
> + 0, 1000);
> + if (ret)
> + return ret;
> + else
> + return offset;
> + }
>
> count = SVC_I3C_MDATACTRL_RXCOUNT(mdctrl);
> for (i = 0; i < count; i++)
> @@ -884,7 +891,7 @@ static int svc_i3c_master_read(struct svc_i3c_master *master,
> offset += count;
> }
>
> - return 0;
> + return offset;
> }
>
> static int svc_i3c_master_write(struct svc_i3c_master *master,
> @@ -917,7 +924,7 @@ static int svc_i3c_master_write(struct svc_i3c_master *master,
> static int svc_i3c_master_xfer(struct svc_i3c_master *master,
> bool rnw, unsigned int xfer_type, u8 addr,
> u8 *in, const u8 *out, unsigned int xfer_len,
> - unsigned int read_len, bool continued)
> + unsigned int *read_len, bool continued)
> {
> u32 reg;
> int ret;
> @@ -927,7 +934,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
> SVC_I3C_MCTRL_IBIRESP_NACK |
> SVC_I3C_MCTRL_DIR(rnw) |
> SVC_I3C_MCTRL_ADDR(addr) |
> - SVC_I3C_MCTRL_RDTERM(read_len),
> + SVC_I3C_MCTRL_RDTERM(*read_len),
> master->regs + SVC_I3C_MCTRL);
>
> ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> @@ -939,8 +946,10 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
> ret = svc_i3c_master_read(master, in, xfer_len);
> else
> ret = svc_i3c_master_write(master, out, xfer_len);
> - if (ret)
> + if (ret < 0)
> goto emit_stop;
> + if (rnw)
> + *read_len = ret;
>
> ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_COMPLETE(reg), 0, 1000);
> @@ -1012,7 +1021,7 @@ static void svc_i3c_master_start_xfer_locked(struct svc_i3c_master *master)
>
> ret = svc_i3c_master_xfer(master, cmd->rnw, xfer->type,
> cmd->addr, cmd->in, cmd->out,
> - cmd->len, cmd->read_len,
> + cmd->len, &cmd->read_len,
> cmd->continued);
> if (ret)
> break;
> @@ -1141,6 +1150,9 @@ static int svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master,
> if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000)))
> svc_i3c_master_dequeue_xfer(master, xfer);
>
> + if (cmd->read_len != xfer_len)
> + ccc->dests[0].payload.len = cmd->read_len;
> +
> ret = xfer->ret;
> svc_i3c_master_free_xfer(xfer);
>

Thanks,
Miquèl

2021-07-16 01:54:44

by Clark Wang

[permalink] [raw]
Subject: RE: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data

Hi Miquel,

Thanks for your review!

> -----Original Message-----
> From: Miquel Raynal <[email protected]>
> Sent: Friday, July 16, 2021 6:56
> To: Clark Wang <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 3/4] i3c: master: svc: add support for slave to stop
> returning data
>
> Hi Clark,
>
> Clark Wang <[email protected]> wrote on Thu, 15 Jul 2021 16:24:12
> +0800:
>
> > When i3c controller reads data from slave device, slave device can
> > stop returning data with an ACK after any byte.
> > Add this support for svc i3c controller. Otherwise, it will go TIMEOUT
> > error path when the slave device ends the read operation early.
>
> Is this part of the I3C specification? I am not aware about it.

Yes. I think the spec mentions about this. You can check section "5.1.2.3 I3C SDR Data Words" of <mipi_i3c_basic_specification>.
[3. Ninth Bit of SDR Slave Returned (Read) Data as End-of-Data: In I2C, the ninth Data bit from
Slave to Master is an ACK by the Master. By contrast, in I3C this bit allows the Slave to end a
Read, and allows the Master to Abort a Read. In SDR terms, the ninth bit of Read data is referred
to as the T-Bit (for ‘Transition’) (see Section 5.1.2.3.4).]

Best Regards,
Clark Wang

>
> > Signed-off-by: Clark Wang <[email protected]>
> > ---
> > drivers/i3c/master/svc-i3c-master.c | 28 ++++++++++++++++++++--------
> > 1 file changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > b/drivers/i3c/master/svc-i3c-master.c
> > index 9d80435638ea..892e57fec4b0 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -865,7 +865,7 @@ static int svc_i3c_master_read(struct
> svc_i3c_master *master,
> > u8 *in, unsigned int len)
> > {
> > int offset = 0, i, ret;
> > - u32 mdctrl;
> > + u32 mdctrl, mstatus;
> >
> > while (offset < len) {
> > unsigned int count;
> > @@ -874,8 +874,15 @@ static int svc_i3c_master_read(struct
> svc_i3c_master *master,
> > mdctrl,
> > !(mdctrl &
> SVC_I3C_MDATACTRL_RXEMPTY),
> > 0, 1000);
> > - if (ret)
> > - return ret;
> > + if (ret) {
> > + ret = readl_poll_timeout(master->regs +
> SVC_I3C_MSTATUS,
> > + mstatus,
> SVC_I3C_MSTATUS_COMPLETE(mstatus),
> > + 0, 1000);
> > + if (ret)
> > + return ret;
> > + else
> > + return offset;
> > + }
> >
> > count = SVC_I3C_MDATACTRL_RXCOUNT(mdctrl);
> > for (i = 0; i < count; i++)
> > @@ -884,7 +891,7 @@ static int svc_i3c_master_read(struct
> svc_i3c_master *master,
> > offset += count;
> > }
> >
> > - return 0;
> > + return offset;
> > }
> >
> > static int svc_i3c_master_write(struct svc_i3c_master *master, @@
> > -917,7 +924,7 @@ static int svc_i3c_master_write(struct svc_i3c_master
> > *master, static int svc_i3c_master_xfer(struct svc_i3c_master *master,
> > bool rnw, unsigned int xfer_type, u8 addr,
> > u8 *in, const u8 *out, unsigned int xfer_len,
> > - unsigned int read_len, bool continued)
> > + unsigned int *read_len, bool continued)
> > {
> > u32 reg;
> > int ret;
> > @@ -927,7 +934,7 @@ static int svc_i3c_master_xfer(struct svc_i3c_master
> *master,
> > SVC_I3C_MCTRL_IBIRESP_NACK |
> > SVC_I3C_MCTRL_DIR(rnw) |
> > SVC_I3C_MCTRL_ADDR(addr) |
> > - SVC_I3C_MCTRL_RDTERM(read_len),
> > + SVC_I3C_MCTRL_RDTERM(*read_len),
> > master->regs + SVC_I3C_MCTRL);
> >
> > ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> @@
> > -939,8 +946,10 @@ static int svc_i3c_master_xfer(struct svc_i3c_master
> *master,
> > ret = svc_i3c_master_read(master, in, xfer_len);
> > else
> > ret = svc_i3c_master_write(master, out, xfer_len);
> > - if (ret)
> > + if (ret < 0)
> > goto emit_stop;
> > + if (rnw)
> > + *read_len = ret;
> >
> > ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> > SVC_I3C_MSTATUS_COMPLETE(reg), 0,
> 1000); @@ -1012,7 +1021,7 @@
> > static void svc_i3c_master_start_xfer_locked(struct svc_i3c_master
> > *master)
> >
> > ret = svc_i3c_master_xfer(master, cmd->rnw, xfer->type,
> > cmd->addr, cmd->in, cmd->out,
> > - cmd->len, cmd->read_len,
> > + cmd->len, &cmd->read_len,
> > cmd->continued);
> > if (ret)
> > break;
> > @@ -1141,6 +1150,9 @@ static int
> svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master,
> > if (!wait_for_completion_timeout(&xfer->comp,
> msecs_to_jiffies(1000)))
> > svc_i3c_master_dequeue_xfer(master, xfer);
> >
> > + if (cmd->read_len != xfer_len)
> > + ccc->dests[0].payload.len = cmd->read_len;
> > +
> > ret = xfer->ret;
> > svc_i3c_master_free_xfer(xfer);
> >
>
> Thanks,
> Miquèl


Attachments:
smime.p7s (9.36 kB)