* [PATCH 0/4] i3c: master: svc: some bug fixes
@ 2021-07-15 8:24 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
Hi,
I am using SVC I3C module recently. I fix some problems and also have a
question.
My question is:
Can I3C bus support pure I2C mode in kernel?
Or in other words, in mixed mode, must there be at least one I3C device on
the I3C bus?
The pure I3C mode works fine. But when only have one I2C device on the
I3C bus, the probe in function i3c_master_bus_init() will go error. Because
there is no one on I3C bus can ACK the I3C message with I3C message speed. Then
it will return error at function i3c_master_rstdaa_locked() because of no ACK
for 0x7e start byte.
When I use the following dtb configuration, the above problem occurs.
&i3c2 {
#address-cells = <3>;
#size-cells = <0>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i3c2>;
i2c-scl-hz = <400000>;
status = "okay";
lsm6dso_i2c: imu@6a {
compatible = "st,lsm6dso";
reg = <0x6a 0x0 0x50>;
};
};
But I saw a similar configuration example in
/home/nxf47749/work/kernel/i3c/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt.
I wonder if that can work normally?
I know the definition in the specification is:
Mixed xxx Bus: I3C Bus topology with both I2C and I3C Devices present
on the I3C Bus...
But I think it is feasible to use pure I2C mode with I3C module.
I am not sure why the use of pure I2C mode is restricted in the software.
If there are errors in my ideas, please correct me in time. Thank you all.
Here are the fixes.
Clark Wang (4):
i3c: master: svc: move module reset behind clk enable
i3c: master: svc: fix atomic issue
i3c: master: svc: add support for slave to stop returning data
i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
drivers/i3c/master/svc-i3c-master.c | 45 +++++++++++++++++++----------
1 file changed, 30 insertions(+), 15 deletions(-)
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/4] i3c: master: svc: some bug fixes
@ 2021-07-15 8:24 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
Hi,
I am using SVC I3C module recently. I fix some problems and also have a
question.
My question is:
Can I3C bus support pure I2C mode in kernel?
Or in other words, in mixed mode, must there be at least one I3C device on
the I3C bus?
The pure I3C mode works fine. But when only have one I2C device on the
I3C bus, the probe in function i3c_master_bus_init() will go error. Because
there is no one on I3C bus can ACK the I3C message with I3C message speed. Then
it will return error at function i3c_master_rstdaa_locked() because of no ACK
for 0x7e start byte.
When I use the following dtb configuration, the above problem occurs.
&i3c2 {
#address-cells = <3>;
#size-cells = <0>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i3c2>;
i2c-scl-hz = <400000>;
status = "okay";
lsm6dso_i2c: imu@6a {
compatible = "st,lsm6dso";
reg = <0x6a 0x0 0x50>;
};
};
But I saw a similar configuration example in
/home/nxf47749/work/kernel/i3c/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt.
I wonder if that can work normally?
I know the definition in the specification is:
Mixed xxx Bus: I3C Bus topology with both I2C and I3C Devices present
on the I3C Bus...
But I think it is feasible to use pure I2C mode with I3C module.
I am not sure why the use of pure I2C mode is restricted in the software.
If there are errors in my ideas, please correct me in time. Thank you all.
Here are the fixes.
Clark Wang (4):
i3c: master: svc: move module reset behind clk enable
i3c: master: svc: fix atomic issue
i3c: master: svc: add support for slave to stop returning data
i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
drivers/i3c/master/svc-i3c-master.c | 45 +++++++++++++++++++----------
1 file changed, 30 insertions(+), 15 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/4] i3c: master: svc: move module reset behind clk enable
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 8:24 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
Reset I3C module will R/W its regs, so enable its clocks first.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
drivers/i3c/master/svc-i3c-master.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 879e5a64acaf..c25a372f6820 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1381,8 +1381,6 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
master->dev = dev;
- svc_i3c_master_reset(master);
-
ret = clk_prepare_enable(master->pclk);
if (ret)
return ret;
@@ -1419,6 +1417,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
+ svc_i3c_master_reset(master);
+
/* Register the master */
ret = i3c_master_register(&master->base, &pdev->dev,
&svc_i3c_master_ops, false);
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 1/4] i3c: master: svc: move module reset behind clk enable
@ 2021-07-15 8:24 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
Reset I3C module will R/W its regs, so enable its clocks first.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
drivers/i3c/master/svc-i3c-master.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 879e5a64acaf..c25a372f6820 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1381,8 +1381,6 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
master->dev = dev;
- svc_i3c_master_reset(master);
-
ret = clk_prepare_enable(master->pclk);
if (ret)
return ret;
@@ -1419,6 +1417,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
+ svc_i3c_master_reset(master);
+
/* Register the master */
ret = i3c_master_register(&master->base, &pdev->dev,
&svc_i3c_master_ops, false);
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/4] i3c: master: svc: fix atomic issue
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 8:24 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
do_daa_locked() function is in a spin lock environment, use
readl_poll_timeout_atomic() to replace the origin
readl_poll_timeout().
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
drivers/i3c/master/svc-i3c-master.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index c25a372f6820..9d80435638ea 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -656,7 +656,7 @@ static int svc_i3c_master_readb(struct svc_i3c_master *master, u8 *dst,
u32 reg;
for (i = 0; i < len; i++) {
- ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
+ ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_RXPEND(reg), 0, 1000);
if (ret)
return ret;
@@ -687,7 +687,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
* Either one slave will send its ID, or the assignment process
* is done.
*/
- ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
+ ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_RXPEND(reg) |
SVC_I3C_MSTATUS_MCTRLDONE(reg),
1, 1000);
@@ -744,7 +744,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
}
/* Wait for the slave to be ready to receive its address */
- ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
+ ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_MCTRLDONE(reg) &&
SVC_I3C_MSTATUS_STATE_DAA(reg) &&
SVC_I3C_MSTATUS_BETWEEN(reg),
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 2/4] i3c: master: svc: fix atomic issue
@ 2021-07-15 8:24 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
do_daa_locked() function is in a spin lock environment, use
readl_poll_timeout_atomic() to replace the origin
readl_poll_timeout().
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
drivers/i3c/master/svc-i3c-master.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index c25a372f6820..9d80435638ea 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -656,7 +656,7 @@ static int svc_i3c_master_readb(struct svc_i3c_master *master, u8 *dst,
u32 reg;
for (i = 0; i < len; i++) {
- ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
+ ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_RXPEND(reg), 0, 1000);
if (ret)
return ret;
@@ -687,7 +687,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
* Either one slave will send its ID, or the assignment process
* is done.
*/
- ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
+ ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_RXPEND(reg) |
SVC_I3C_MSTATUS_MCTRLDONE(reg),
1, 1000);
@@ -744,7 +744,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
}
/* Wait for the slave to be ready to receive its address */
- ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
+ ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
SVC_I3C_MSTATUS_MCTRLDONE(reg) &&
SVC_I3C_MSTATUS_STATE_DAA(reg) &&
SVC_I3C_MSTATUS_BETWEEN(reg),
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 8:24 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
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 <xiaoning.wang@nxp.com>
---
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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data
@ 2021-07-15 8:24 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
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 <xiaoning.wang@nxp.com>
---
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
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/4] i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 8:24 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
If using I2C/I3C mixed mode, need to set ODSTOP. Otherwise, the I2C
devices cannot see the stop signal. It may cause message sending errors.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
drivers/i3c/master/svc-i3c-master.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 892e57fec4b0..b05cc7f521e6 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -449,7 +449,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
struct i3c_device_info info = {};
unsigned long fclk_rate, fclk_period_ns;
unsigned int high_period_ns, od_low_period_ns;
- u32 ppbaud, pplow, odhpp, odbaud, i2cbaud, reg;
+ u32 ppbaud, pplow, odhpp, odbaud, odstop, i2cbaud, reg;
int ret;
/* Timings derivation */
@@ -479,6 +479,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
switch (bus->mode) {
case I3C_BUS_MODE_PURE:
i2cbaud = 0;
+ odstop = 0;
break;
case I3C_BUS_MODE_MIXED_FAST:
case I3C_BUS_MODE_MIXED_LIMITED:
@@ -487,6 +488,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
* between the high and low period does not really matter.
*/
i2cbaud = DIV_ROUND_UP(1000, od_low_period_ns) - 2;
+ odstop = 1;
break;
case I3C_BUS_MODE_MIXED_SLOW:
/*
@@ -494,6 +496,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
* constraints as the FM+ mode.
*/
i2cbaud = DIV_ROUND_UP(2500, od_low_period_ns) - 2;
+ odstop = 1;
break;
default:
return -EINVAL;
@@ -502,7 +505,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
reg = SVC_I3C_MCONFIG_MASTER_EN |
SVC_I3C_MCONFIG_DISTO(0) |
SVC_I3C_MCONFIG_HKEEP(0) |
- SVC_I3C_MCONFIG_ODSTOP(0) |
+ SVC_I3C_MCONFIG_ODSTOP(odstop) |
SVC_I3C_MCONFIG_PPBAUD(ppbaud) |
SVC_I3C_MCONFIG_PPLOW(pplow) |
SVC_I3C_MCONFIG_ODBAUD(odbaud) |
--
2.25.1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 4/4] i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
@ 2021-07-15 8:24 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-15 8:24 UTC (permalink / raw)
To: miquel.raynal, conor.culhane, alexandre.belloni, vitor.soares,
boris.brezillon
Cc: linux-i3c, linux-kernel, xiaoning.wang
If using I2C/I3C mixed mode, need to set ODSTOP. Otherwise, the I2C
devices cannot see the stop signal. It may cause message sending errors.
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
drivers/i3c/master/svc-i3c-master.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 892e57fec4b0..b05cc7f521e6 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -449,7 +449,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
struct i3c_device_info info = {};
unsigned long fclk_rate, fclk_period_ns;
unsigned int high_period_ns, od_low_period_ns;
- u32 ppbaud, pplow, odhpp, odbaud, i2cbaud, reg;
+ u32 ppbaud, pplow, odhpp, odbaud, odstop, i2cbaud, reg;
int ret;
/* Timings derivation */
@@ -479,6 +479,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
switch (bus->mode) {
case I3C_BUS_MODE_PURE:
i2cbaud = 0;
+ odstop = 0;
break;
case I3C_BUS_MODE_MIXED_FAST:
case I3C_BUS_MODE_MIXED_LIMITED:
@@ -487,6 +488,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
* between the high and low period does not really matter.
*/
i2cbaud = DIV_ROUND_UP(1000, od_low_period_ns) - 2;
+ odstop = 1;
break;
case I3C_BUS_MODE_MIXED_SLOW:
/*
@@ -494,6 +496,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
* constraints as the FM+ mode.
*/
i2cbaud = DIV_ROUND_UP(2500, od_low_period_ns) - 2;
+ odstop = 1;
break;
default:
return -EINVAL;
@@ -502,7 +505,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
reg = SVC_I3C_MCONFIG_MASTER_EN |
SVC_I3C_MCONFIG_DISTO(0) |
SVC_I3C_MCONFIG_HKEEP(0) |
- SVC_I3C_MCONFIG_ODSTOP(0) |
+ SVC_I3C_MCONFIG_ODSTOP(odstop) |
SVC_I3C_MCONFIG_PPBAUD(ppbaud) |
SVC_I3C_MCONFIG_PPLOW(pplow) |
SVC_I3C_MCONFIG_ODBAUD(odbaud) |
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] i3c: master: svc: move module reset behind clk enable
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 22:51 ` Miquel Raynal
-1 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:51 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:10
+0800:
> Reset I3C module will R/W its regs, so enable its clocks first.
>
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/4] i3c: master: svc: move module reset behind clk enable
@ 2021-07-15 22:51 ` Miquel Raynal
0 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:51 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:10
+0800:
> Reset I3C module will R/W its regs, so enable its clocks first.
>
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
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] 26+ messages in thread
* Re: [PATCH 2/4] i3c: master: svc: fix atomic issue
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 22:52 ` Miquel Raynal
-1 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:52 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:11
+0800:
> do_daa_locked() function is in a spin lock environment, use
> readl_poll_timeout_atomic() to replace the origin
> readl_poll_timeout().
>
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> ---
> drivers/i3c/master/svc-i3c-master.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index c25a372f6820..9d80435638ea 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -656,7 +656,7 @@ static int svc_i3c_master_readb(struct svc_i3c_master *master, u8 *dst,
> u32 reg;
>
> for (i = 0; i < len; i++) {
> - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_RXPEND(reg), 0, 1000);
You forgot to align the parameters of the function here and below.
Otherwise,
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> if (ret)
> return ret;
> @@ -687,7 +687,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> * Either one slave will send its ID, or the assignment process
> * is done.
> */
> - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_RXPEND(reg) |
> SVC_I3C_MSTATUS_MCTRLDONE(reg),
> 1, 1000);
> @@ -744,7 +744,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> }
>
> /* Wait for the slave to be ready to receive its address */
> - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_MCTRLDONE(reg) &&
> SVC_I3C_MSTATUS_STATE_DAA(reg) &&
> SVC_I3C_MSTATUS_BETWEEN(reg),
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/4] i3c: master: svc: fix atomic issue
@ 2021-07-15 22:52 ` Miquel Raynal
0 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:52 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:11
+0800:
> do_daa_locked() function is in a spin lock environment, use
> readl_poll_timeout_atomic() to replace the origin
> readl_poll_timeout().
>
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> ---
> drivers/i3c/master/svc-i3c-master.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index c25a372f6820..9d80435638ea 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -656,7 +656,7 @@ static int svc_i3c_master_readb(struct svc_i3c_master *master, u8 *dst,
> u32 reg;
>
> for (i = 0; i < len; i++) {
> - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_RXPEND(reg), 0, 1000);
You forgot to align the parameters of the function here and below.
Otherwise,
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> if (ret)
> return ret;
> @@ -687,7 +687,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> * Either one slave will send its ID, or the assignment process
> * is done.
> */
> - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_RXPEND(reg) |
> SVC_I3C_MSTATUS_MCTRLDONE(reg),
> 1, 1000);
> @@ -744,7 +744,7 @@ static int svc_i3c_master_do_daa_locked(struct svc_i3c_master *master,
> }
>
> /* Wait for the slave to be ready to receive its address */
> - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS, reg,
> + ret = readl_poll_timeout_atomic(master->regs + SVC_I3C_MSTATUS, reg,
> SVC_I3C_MSTATUS_MCTRLDONE(reg) &&
> SVC_I3C_MSTATUS_STATE_DAA(reg) &&
> SVC_I3C_MSTATUS_BETWEEN(reg),
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] 26+ messages in thread
* Re: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 22:55 ` Miquel Raynal
-1 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:55 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> 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 <xiaoning.wang@nxp.com>
> ---
> 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
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data
@ 2021-07-15 22:55 ` Miquel Raynal
0 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:55 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> 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 <xiaoning.wang@nxp.com>
> ---
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/4] i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
2021-07-15 8:24 ` Clark Wang
@ 2021-07-15 22:57 ` Miquel Raynal
-1 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:57 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:13
+0800:
> If using I2C/I3C mixed mode, need to set ODSTOP. Otherwise, the I2C
> devices cannot see the stop signal. It may cause message sending errors.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> ---
> drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 892e57fec4b0..b05cc7f521e6 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -449,7 +449,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> struct i3c_device_info info = {};
> unsigned long fclk_rate, fclk_period_ns;
> unsigned int high_period_ns, od_low_period_ns;
> - u32 ppbaud, pplow, odhpp, odbaud, i2cbaud, reg;
> + u32 ppbaud, pplow, odhpp, odbaud, odstop, i2cbaud, reg;
> int ret;
>
> /* Timings derivation */
> @@ -479,6 +479,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> switch (bus->mode) {
> case I3C_BUS_MODE_PURE:
> i2cbaud = 0;
> + odstop = 0;
> break;
> case I3C_BUS_MODE_MIXED_FAST:
> case I3C_BUS_MODE_MIXED_LIMITED:
> @@ -487,6 +488,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> * between the high and low period does not really matter.
> */
> i2cbaud = DIV_ROUND_UP(1000, od_low_period_ns) - 2;
> + odstop = 1;
> break;
> case I3C_BUS_MODE_MIXED_SLOW:
> /*
> @@ -494,6 +496,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> * constraints as the FM+ mode.
> */
> i2cbaud = DIV_ROUND_UP(2500, od_low_period_ns) - 2;
> + odstop = 1;
> break;
> default:
> return -EINVAL;
> @@ -502,7 +505,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> reg = SVC_I3C_MCONFIG_MASTER_EN |
> SVC_I3C_MCONFIG_DISTO(0) |
> SVC_I3C_MCONFIG_HKEEP(0) |
> - SVC_I3C_MCONFIG_ODSTOP(0) |
> + SVC_I3C_MCONFIG_ODSTOP(odstop) |
> SVC_I3C_MCONFIG_PPBAUD(ppbaud) |
> SVC_I3C_MCONFIG_PPLOW(pplow) |
> SVC_I3C_MCONFIG_ODBAUD(odbaud) |
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] 26+ messages in thread
* Re: [PATCH 4/4] i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
@ 2021-07-15 22:57 ` Miquel Raynal
0 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-15 22:57 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:13
+0800:
> If using I2C/I3C mixed mode, need to set ODSTOP. Otherwise, the I2C
> devices cannot see the stop signal. It may cause message sending errors.
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> ---
> drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 892e57fec4b0..b05cc7f521e6 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -449,7 +449,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> struct i3c_device_info info = {};
> unsigned long fclk_rate, fclk_period_ns;
> unsigned int high_period_ns, od_low_period_ns;
> - u32 ppbaud, pplow, odhpp, odbaud, i2cbaud, reg;
> + u32 ppbaud, pplow, odhpp, odbaud, odstop, i2cbaud, reg;
> int ret;
>
> /* Timings derivation */
> @@ -479,6 +479,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> switch (bus->mode) {
> case I3C_BUS_MODE_PURE:
> i2cbaud = 0;
> + odstop = 0;
> break;
> case I3C_BUS_MODE_MIXED_FAST:
> case I3C_BUS_MODE_MIXED_LIMITED:
> @@ -487,6 +488,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> * between the high and low period does not really matter.
> */
> i2cbaud = DIV_ROUND_UP(1000, od_low_period_ns) - 2;
> + odstop = 1;
> break;
> case I3C_BUS_MODE_MIXED_SLOW:
> /*
> @@ -494,6 +496,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> * constraints as the FM+ mode.
> */
> i2cbaud = DIV_ROUND_UP(2500, od_low_period_ns) - 2;
> + odstop = 1;
> break;
> default:
> return -EINVAL;
> @@ -502,7 +505,7 @@ static int svc_i3c_master_bus_init(struct i3c_master_controller *m)
> reg = SVC_I3C_MCONFIG_MASTER_EN |
> SVC_I3C_MCONFIG_DISTO(0) |
> SVC_I3C_MCONFIG_HKEEP(0) |
> - SVC_I3C_MCONFIG_ODSTOP(0) |
> + SVC_I3C_MCONFIG_ODSTOP(odstop) |
> SVC_I3C_MCONFIG_PPBAUD(ppbaud) |
> SVC_I3C_MCONFIG_PPLOW(pplow) |
> SVC_I3C_MCONFIG_ODBAUD(odbaud) |
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data
2021-07-15 22:55 ` Miquel Raynal
@ 2021-07-16 1:53 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-16 1:53 UTC (permalink / raw)
To: Miquel Raynal
Cc: conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
vitor.soares@synopsys.com, boris.brezillon@bootlin.com,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 5096 bytes --]
Hi Miquel,
Thanks for your review!
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 6:56
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com;
> vitor.soares@synopsys.com; boris.brezillon@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/4] i3c: master: svc: add support for slave to stop
> returning data
>
> Hi Clark,
>
> Clark Wang <xiaoning.wang@nxp.com> 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 <xiaoning.wang@nxp.com>
> > ---
> > 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
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9583 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data
@ 2021-07-16 1:53 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-16 1:53 UTC (permalink / raw)
To: Miquel Raynal
Cc: conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
vitor.soares@synopsys.com, boris.brezillon@bootlin.com,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 5096 bytes --]
Hi Miquel,
Thanks for your review!
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 6:56
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com;
> vitor.soares@synopsys.com; boris.brezillon@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/4] i3c: master: svc: add support for slave to stop
> returning data
>
> Hi Clark,
>
> Clark Wang <xiaoning.wang@nxp.com> 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 <xiaoning.wang@nxp.com>
> > ---
> > 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
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9583 bytes --]
[-- Attachment #2: Type: text/plain, Size: 111 bytes --]
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 2/4] i3c: master: svc: fix atomic issue
2021-07-15 22:52 ` Miquel Raynal
@ 2021-07-16 1:58 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-16 1:58 UTC (permalink / raw)
To: Miquel Raynal
Cc: conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
vitor.soares@synopsys.com, boris.brezillon@bootlin.com,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2822 bytes --]
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 6:52
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com;
> vitor.soares@synopsys.com; boris.brezillon@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/4] i3c: master: svc: fix atomic issue
>
> Hi Clark,
>
> Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:11
> +0800:
>
> > do_daa_locked() function is in a spin lock environment, use
> > readl_poll_timeout_atomic() to replace the origin
> > readl_poll_timeout().
> >
> > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> > ---
> > drivers/i3c/master/svc-i3c-master.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > b/drivers/i3c/master/svc-i3c-master.c
> > index c25a372f6820..9d80435638ea 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -656,7 +656,7 @@ static int svc_i3c_master_readb(struct
> svc_i3c_master *master, u8 *dst,
> > u32 reg;
> >
> > for (i = 0; i < len; i++) {
> > - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> reg,
> > + ret = readl_poll_timeout_atomic(master->regs +
> SVC_I3C_MSTATUS,
> > +reg,
> > SVC_I3C_MSTATUS_RXPEND(reg), 0,
> 1000);
>
> You forgot to align the parameters of the function here and below.
Oh, sorry about this. I will send V2 to correct this.
By the way, can you help to check the question I mentioned in the cover letter?
Thank you very much!
Best Regards,
Clark Wang
>
> Otherwise,
>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> > if (ret)
> > return ret;
> > @@ -687,7 +687,7 @@ static int svc_i3c_master_do_daa_locked(struct
> svc_i3c_master *master,
> > * Either one slave will send its ID, or the assignment process
> > * is done.
> > */
> > - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> reg,
> > + ret = readl_poll_timeout_atomic(master->regs +
> SVC_I3C_MSTATUS,
> > +reg,
> > SVC_I3C_MSTATUS_RXPEND(reg) |
> >
> SVC_I3C_MSTATUS_MCTRLDONE(reg),
> > 1, 1000);
> > @@ -744,7 +744,7 @@ static int svc_i3c_master_do_daa_locked(struct
> svc_i3c_master *master,
> > }
> >
> > /* Wait for the slave to be ready to receive its address */
> > - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> reg,
> > + ret = readl_poll_timeout_atomic(master->regs +
> SVC_I3C_MSTATUS,
> > +reg,
> >
> SVC_I3C_MSTATUS_MCTRLDONE(reg) &&
> > SVC_I3C_MSTATUS_STATE_DAA(reg)
> &&
> > SVC_I3C_MSTATUS_BETWEEN(reg),
>
>
>
>
> Thanks,
> Miquèl
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9583 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 2/4] i3c: master: svc: fix atomic issue
@ 2021-07-16 1:58 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-16 1:58 UTC (permalink / raw)
To: Miquel Raynal
Cc: conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
vitor.soares@synopsys.com, boris.brezillon@bootlin.com,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 2822 bytes --]
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 6:52
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com;
> vitor.soares@synopsys.com; boris.brezillon@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/4] i3c: master: svc: fix atomic issue
>
> Hi Clark,
>
> Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:11
> +0800:
>
> > do_daa_locked() function is in a spin lock environment, use
> > readl_poll_timeout_atomic() to replace the origin
> > readl_poll_timeout().
> >
> > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> > ---
> > drivers/i3c/master/svc-i3c-master.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > b/drivers/i3c/master/svc-i3c-master.c
> > index c25a372f6820..9d80435638ea 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -656,7 +656,7 @@ static int svc_i3c_master_readb(struct
> svc_i3c_master *master, u8 *dst,
> > u32 reg;
> >
> > for (i = 0; i < len; i++) {
> > - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> reg,
> > + ret = readl_poll_timeout_atomic(master->regs +
> SVC_I3C_MSTATUS,
> > +reg,
> > SVC_I3C_MSTATUS_RXPEND(reg), 0,
> 1000);
>
> You forgot to align the parameters of the function here and below.
Oh, sorry about this. I will send V2 to correct this.
By the way, can you help to check the question I mentioned in the cover letter?
Thank you very much!
Best Regards,
Clark Wang
>
> Otherwise,
>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> > if (ret)
> > return ret;
> > @@ -687,7 +687,7 @@ static int svc_i3c_master_do_daa_locked(struct
> svc_i3c_master *master,
> > * Either one slave will send its ID, or the assignment process
> > * is done.
> > */
> > - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> reg,
> > + ret = readl_poll_timeout_atomic(master->regs +
> SVC_I3C_MSTATUS,
> > +reg,
> > SVC_I3C_MSTATUS_RXPEND(reg) |
> >
> SVC_I3C_MSTATUS_MCTRLDONE(reg),
> > 1, 1000);
> > @@ -744,7 +744,7 @@ static int svc_i3c_master_do_daa_locked(struct
> svc_i3c_master *master,
> > }
> >
> > /* Wait for the slave to be ready to receive its address */
> > - ret = readl_poll_timeout(master->regs + SVC_I3C_MSTATUS,
> reg,
> > + ret = readl_poll_timeout_atomic(master->regs +
> SVC_I3C_MSTATUS,
> > +reg,
> >
> SVC_I3C_MSTATUS_MCTRLDONE(reg) &&
> > SVC_I3C_MSTATUS_STATE_DAA(reg)
> &&
> > SVC_I3C_MSTATUS_BETWEEN(reg),
>
>
>
>
> Thanks,
> Miquèl
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9583 bytes --]
[-- Attachment #2: Type: text/plain, Size: 111 bytes --]
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 0/4] i3c: master: svc: some bug fixes
2021-07-15 8:24 ` Clark Wang
@ 2021-07-16 7:51 ` Miquel Raynal
-1 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-16 7:51 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:09
+0800:
> Hi,
>
> I am using SVC I3C module recently. I fix some problems and also have a
> question.
>
> My question is:
> Can I3C bus support pure I2C mode in kernel?
> Or in other words, in mixed mode, must there be at least one I3C device on
> the I3C bus?
>
> The pure I3C mode works fine. But when only have one I2C device on the
> I3C bus, the probe in function i3c_master_bus_init() will go error. Because
> there is no one on I3C bus can ACK the I3C message with I3C message speed. Then
> it will return error at function i3c_master_rstdaa_locked() because of no ACK
> for 0x7e start byte.
> When I use the following dtb configuration, the above problem occurs.
> &i3c2 {
> #address-cells = <3>;
> #size-cells = <0>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_i3c2>;
> i2c-scl-hz = <400000>;
> status = "okay";
>
> lsm6dso_i2c: imu@6a {
> compatible = "st,lsm6dso";
> reg = <0x6a 0x0 0x50>;
> };
> };
>
> But I saw a similar configuration example in
> /home/nxf47749/work/kernel/i3c/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt.
> I wonder if that can work normally?
>
> I know the definition in the specification is:
> Mixed xxx Bus: I3C Bus topology with both I2C and I3C Devices present
> on the I3C Bus...
> But I think it is feasible to use pure I2C mode with I3C module.
> I am not sure why the use of pure I2C mode is restricted in the software.
>
> If there are errors in my ideas, please correct me in time. Thank you all.
As you pointed out, I am not aware of a specific I2C only bus setting
but if you find a way to workaround the issue raised above by software
in a rather clean way, then... why not?
> Here are the fixes.
>
> Clark Wang (4):
> i3c: master: svc: move module reset behind clk enable
> i3c: master: svc: fix atomic issue
> i3c: master: svc: add support for slave to stop returning data
> i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
>
> drivers/i3c/master/svc-i3c-master.c | 45 +++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
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] 26+ messages in thread
* Re: [PATCH 0/4] i3c: master: svc: some bug fixes
@ 2021-07-16 7:51 ` Miquel Raynal
0 siblings, 0 replies; 26+ messages in thread
From: Miquel Raynal @ 2021-07-16 7:51 UTC (permalink / raw)
To: Clark Wang
Cc: conor.culhane, alexandre.belloni, vitor.soares, boris.brezillon,
linux-i3c, linux-kernel
Hi Clark,
Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:09
+0800:
> Hi,
>
> I am using SVC I3C module recently. I fix some problems and also have a
> question.
>
> My question is:
> Can I3C bus support pure I2C mode in kernel?
> Or in other words, in mixed mode, must there be at least one I3C device on
> the I3C bus?
>
> The pure I3C mode works fine. But when only have one I2C device on the
> I3C bus, the probe in function i3c_master_bus_init() will go error. Because
> there is no one on I3C bus can ACK the I3C message with I3C message speed. Then
> it will return error at function i3c_master_rstdaa_locked() because of no ACK
> for 0x7e start byte.
> When I use the following dtb configuration, the above problem occurs.
> &i3c2 {
> #address-cells = <3>;
> #size-cells = <0>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_i3c2>;
> i2c-scl-hz = <400000>;
> status = "okay";
>
> lsm6dso_i2c: imu@6a {
> compatible = "st,lsm6dso";
> reg = <0x6a 0x0 0x50>;
> };
> };
>
> But I saw a similar configuration example in
> /home/nxf47749/work/kernel/i3c/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt.
> I wonder if that can work normally?
>
> I know the definition in the specification is:
> Mixed xxx Bus: I3C Bus topology with both I2C and I3C Devices present
> on the I3C Bus...
> But I think it is feasible to use pure I2C mode with I3C module.
> I am not sure why the use of pure I2C mode is restricted in the software.
>
> If there are errors in my ideas, please correct me in time. Thank you all.
As you pointed out, I am not aware of a specific I2C only bus setting
but if you find a way to workaround the issue raised above by software
in a rather clean way, then... why not?
> Here are the fixes.
>
> Clark Wang (4):
> i3c: master: svc: move module reset behind clk enable
> i3c: master: svc: fix atomic issue
> i3c: master: svc: add support for slave to stop returning data
> i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
>
> drivers/i3c/master/svc-i3c-master.c | 45 +++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 0/4] i3c: master: svc: some bug fixes
2021-07-16 7:51 ` Miquel Raynal
@ 2021-07-16 8:07 ` Clark Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-16 8:07 UTC (permalink / raw)
To: Miquel Raynal
Cc: conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 3026 bytes --]
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 15:52
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com;
> vitor.soares@synopsys.com; boris.brezillon@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 0/4] i3c: master: svc: some bug fixes
>
> Hi Clark,
>
> Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:09
> +0800:
>
> > Hi,
> >
> > I am using SVC I3C module recently. I fix some problems and also have a
> > question.
> >
> > My question is:
> > Can I3C bus support pure I2C mode in kernel?
> > Or in other words, in mixed mode, must there be at least one I3C device on
> > the I3C bus?
> >
> > The pure I3C mode works fine. But when only have one I2C device on the
> > I3C bus, the probe in function i3c_master_bus_init() will go error. Because
> > there is no one on I3C bus can ACK the I3C message with I3C message
> speed. Then
> > it will return error at function i3c_master_rstdaa_locked() because of no
> ACK
> > for 0x7e start byte.
> > When I use the following dtb configuration, the above problem occurs.
> > &i3c2 {
> > #address-cells = <3>;
> > #size-cells = <0>;
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_i3c2>;
> > i2c-scl-hz = <400000>;
> > status = "okay";
> >
> > lsm6dso_i2c: imu@6a {
> > compatible = "st,lsm6dso";
> > reg = <0x6a 0x0 0x50>;
> > };
> > };
> >
> > But I saw a similar configuration example in
> >
> /home/nxf47749/work/kernel/i3c/Documentation/devicetree/bindings/i3c/
> snps,dw-i3c-master.txt.
> > I wonder if that can work normally?
> >
> > I know the definition in the specification is:
> > Mixed xxx Bus: I3C Bus topology with both I2C and I3C Devices present
> > on the I3C Bus...
> > But I think it is feasible to use pure I2C mode with I3C module.
> > I am not sure why the use of pure I2C mode is restricted in the software.
> >
> > If there are errors in my ideas, please correct me in time. Thank you all.
>
> As you pointed out, I am not aware of a specific I2C only bus setting
> but if you find a way to workaround the issue raised above by software
> in a rather clean way, then... why not?
Hi Miquel,
Ok. At present, I just commented part of the code for registering i3c for pure I2C mode testing. I will try to make a clean revision afterwards.
Thank you very much for your suggestion!
Best Regards,
Clark Wang
>
> > Here are the fixes.
> >
> > Clark Wang (4):
> > i3c: master: svc: move module reset behind clk enable
> > i3c: master: svc: fix atomic issue
> > i3c: master: svc: add support for slave to stop returning data
> > i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
> >
> > drivers/i3c/master/svc-i3c-master.c | 45 +++++++++++++++++++----------
> > 1 file changed, 30 insertions(+), 15 deletions(-)
> >
>
> Thanks,
> Miquèl
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9583 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 0/4] i3c: master: svc: some bug fixes
@ 2021-07-16 8:07 ` Clark Wang
0 siblings, 0 replies; 26+ messages in thread
From: Clark Wang @ 2021-07-16 8:07 UTC (permalink / raw)
To: Miquel Raynal
Cc: conor.culhane@silvaco.com, alexandre.belloni@bootlin.com,
linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 3026 bytes --]
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Friday, July 16, 2021 15:52
> To: Clark Wang <xiaoning.wang@nxp.com>
> Cc: conor.culhane@silvaco.com; alexandre.belloni@bootlin.com;
> vitor.soares@synopsys.com; boris.brezillon@bootlin.com; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 0/4] i3c: master: svc: some bug fixes
>
> Hi Clark,
>
> Clark Wang <xiaoning.wang@nxp.com> wrote on Thu, 15 Jul 2021 16:24:09
> +0800:
>
> > Hi,
> >
> > I am using SVC I3C module recently. I fix some problems and also have a
> > question.
> >
> > My question is:
> > Can I3C bus support pure I2C mode in kernel?
> > Or in other words, in mixed mode, must there be at least one I3C device on
> > the I3C bus?
> >
> > The pure I3C mode works fine. But when only have one I2C device on the
> > I3C bus, the probe in function i3c_master_bus_init() will go error. Because
> > there is no one on I3C bus can ACK the I3C message with I3C message
> speed. Then
> > it will return error at function i3c_master_rstdaa_locked() because of no
> ACK
> > for 0x7e start byte.
> > When I use the following dtb configuration, the above problem occurs.
> > &i3c2 {
> > #address-cells = <3>;
> > #size-cells = <0>;
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_i3c2>;
> > i2c-scl-hz = <400000>;
> > status = "okay";
> >
> > lsm6dso_i2c: imu@6a {
> > compatible = "st,lsm6dso";
> > reg = <0x6a 0x0 0x50>;
> > };
> > };
> >
> > But I saw a similar configuration example in
> >
> /home/nxf47749/work/kernel/i3c/Documentation/devicetree/bindings/i3c/
> snps,dw-i3c-master.txt.
> > I wonder if that can work normally?
> >
> > I know the definition in the specification is:
> > Mixed xxx Bus: I3C Bus topology with both I2C and I3C Devices present
> > on the I3C Bus...
> > But I think it is feasible to use pure I2C mode with I3C module.
> > I am not sure why the use of pure I2C mode is restricted in the software.
> >
> > If there are errors in my ideas, please correct me in time. Thank you all.
>
> As you pointed out, I am not aware of a specific I2C only bus setting
> but if you find a way to workaround the issue raised above by software
> in a rather clean way, then... why not?
Hi Miquel,
Ok. At present, I just commented part of the code for registering i3c for pure I2C mode testing. I will try to make a clean revision afterwards.
Thank you very much for your suggestion!
Best Regards,
Clark Wang
>
> > Here are the fixes.
> >
> > Clark Wang (4):
> > i3c: master: svc: move module reset behind clk enable
> > i3c: master: svc: fix atomic issue
> > i3c: master: svc: add support for slave to stop returning data
> > i3c: master: svc: set ODSTOP to let I2C device see the STOP signal
> >
> > drivers/i3c/master/svc-i3c-master.c | 45 +++++++++++++++++++----------
> > 1 file changed, 30 insertions(+), 15 deletions(-)
> >
>
> Thanks,
> Miquèl
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9583 bytes --]
[-- Attachment #2: Type: text/plain, Size: 111 bytes --]
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-07-30 19:40 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 8:24 [PATCH 0/4] i3c: master: svc: some bug fixes Clark Wang
2021-07-15 8:24 ` Clark Wang
2021-07-15 8:24 ` [PATCH 1/4] i3c: master: svc: move module reset behind clk enable Clark Wang
2021-07-15 8:24 ` Clark Wang
2021-07-15 22:51 ` Miquel Raynal
2021-07-15 22:51 ` Miquel Raynal
2021-07-15 8:24 ` [PATCH 2/4] i3c: master: svc: fix atomic issue Clark Wang
2021-07-15 8:24 ` Clark Wang
2021-07-15 22:52 ` Miquel Raynal
2021-07-15 22:52 ` Miquel Raynal
2021-07-16 1:58 ` Clark Wang
2021-07-16 1:58 ` Clark Wang
2021-07-15 8:24 ` [PATCH 3/4] i3c: master: svc: add support for slave to stop returning data Clark Wang
2021-07-15 8:24 ` Clark Wang
2021-07-15 22:55 ` Miquel Raynal
2021-07-15 22:55 ` Miquel Raynal
2021-07-16 1:53 ` Clark Wang
2021-07-16 1:53 ` Clark Wang
2021-07-15 8:24 ` [PATCH 4/4] i3c: master: svc: set ODSTOP to let I2C device see the STOP signal Clark Wang
2021-07-15 8:24 ` Clark Wang
2021-07-15 22:57 ` Miquel Raynal
2021-07-15 22:57 ` Miquel Raynal
2021-07-16 7:51 ` [PATCH 0/4] i3c: master: svc: some bug fixes Miquel Raynal
2021-07-16 7:51 ` Miquel Raynal
2021-07-16 8:07 ` Clark Wang
2021-07-16 8:07 ` Clark Wang
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.