[PATCH] i3c: master: svc: fix i3c suspend/resume issue

[PATCH] i3c: master: svc: fix i3c suspend/resume issue

[Clark Wang]

When system suspends, if i3c module is powered down, register
value will lose.
Need to save the key registers before system suspend.
So save these registers value in runtime pm suspend, and restore them
if these register's value is different with the saved values
when runtime pm resume.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
 drivers/i3c/master/svc-i3c-master.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index e3f454123805..bca3a4352ad1 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -145,6 +145,11 @@ struct svc_i3c_xfer {
 	struct svc_i3c_cmd cmds[];
 };
 
+struct svc_i3c_regs_save {
+	u32 mconfig;
+	u32 mdynaddr;
+};
+
 /**
  * struct svc_i3c_master - Silvaco I3C Master structure
  * @base: I3C master controller
@@ -173,6 +178,7 @@ struct svc_i3c_master {
 	struct i3c_master_controller base;
 	struct device *dev;
 	void __iomem *regs;
+	struct svc_i3c_regs_save saved_regs;
 	u32 free_slots;
 	u8 addrs[SVC_I3C_MAX_DEVS];
 	struct i3c_dev_desc *descs[SVC_I3C_MAX_DEVS];
@@ -1579,10 +1585,28 @@ static void svc_i3c_master_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 }
 
+static void svc_i3c_save_regs(struct svc_i3c_master *master)
+{
+	master->saved_regs.mconfig = readl(master->regs + SVC_I3C_MCONFIG);
+	master->saved_regs.mdynaddr = readl(master->regs + SVC_I3C_MDYNADDR);
+}
+
+static void svc_i3c_restore_regs(struct svc_i3c_master *master)
+{
+	if (readl(master->regs + SVC_I3C_MDYNADDR) !=
+	    master->saved_regs.mdynaddr) {
+		writel(master->saved_regs.mconfig,
+		       master->regs + SVC_I3C_MCONFIG);
+		writel(master->saved_regs.mdynaddr,
+		       master->regs + SVC_I3C_MDYNADDR);
+	}
+}
+
 static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
 {
 	struct svc_i3c_master *master = dev_get_drvdata(dev);
 
+	svc_i3c_save_regs(master);
 	svc_i3c_master_unprepare_clks(master);
 	pinctrl_pm_select_sleep_state(dev);
 
@@ -1596,6 +1620,8 @@ static int __maybe_unused svc_i3c_runtime_resume(struct device *dev)
 	pinctrl_pm_select_default_state(dev);
 	svc_i3c_master_prepare_clks(master);
 
+	svc_i3c_restore_regs(master);
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH] i3c: master: svc: fix cpu schedule in spin lock

[Clark Wang]

pm_runtime_resume_and_get() may call sleep(). It cannot be used in
svc_i3c_master_start_xfer_locked(), because it is in a spin lock.

Move the pm runtime operations to svc_i3c_master_enqueue_xfer().

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Fixes: 05be23ef78f7 ("i3c: master: svc: add runtime pm support")
---
 drivers/i3c/master/svc-i3c-master.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index bca3a4352ad1..4edf33ed207d 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1096,12 +1096,6 @@ static void svc_i3c_master_start_xfer_locked(struct svc_i3c_master *master)
 	if (!xfer)
 		return;
 
-	ret = pm_runtime_resume_and_get(master->dev);
-	if (ret < 0) {
-		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
-		return;
-	}
-
 	svc_i3c_master_clear_merrwarn(master);
 	svc_i3c_master_flush_fifo(master);
 
@@ -1116,9 +1110,6 @@ static void svc_i3c_master_start_xfer_locked(struct svc_i3c_master *master)
 			break;
 	}
 
-	pm_runtime_mark_last_busy(master->dev);
-	pm_runtime_put_autosuspend(master->dev);
-
 	xfer->ret = ret;
 	complete(&xfer->comp);
 
@@ -1139,6 +1130,13 @@ static void svc_i3c_master_enqueue_xfer(struct svc_i3c_master *master,
 					struct svc_i3c_xfer *xfer)
 {
 	unsigned long flags;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(master->dev);
+	if (ret < 0) {
+		dev_err(master->dev, "<%s> Cannot get runtime PM.\n", __func__);
+		return;
+	}
 
 	init_completion(&xfer->comp);
 	spin_lock_irqsave(&master->xferqueue.lock, flags);
@@ -1149,6 +1147,9 @@ static void svc_i3c_master_enqueue_xfer(struct svc_i3c_master *master,
 		svc_i3c_master_start_xfer_locked(master);
 	}
 	spin_unlock_irqrestore(&master->xferqueue.lock, flags);
+
+	pm_runtime_mark_last_busy(master->dev);
+	pm_runtime_put_autosuspend(master->dev);
 }
 
 static bool
-- 
2.34.1

Re: [PATCH] i3c: master: svc: fix cpu schedule in spin lock

[Miquel Raynal]

Hi Clark,

xiaoning.wang@nxp.com wrote on Wed, 17 May 2023 11:30:29 +0800:

> pm_runtime_resume_and_get() may call sleep(). It cannot be used in
> svc_i3c_master_start_xfer_locked(), because it is in a spin lock.
> 
> Move the pm runtime operations to svc_i3c_master_enqueue_xfer().
> 
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> Fixes: 05be23ef78f7 ("i3c: master: svc: add runtime pm support")

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: (subset) [PATCH] i3c: master: svc: fix cpu schedule in spin lock

[Alexandre Belloni]

On Wed, 17 May 2023 11:30:29 +0800, Clark Wang wrote:
> pm_runtime_resume_and_get() may call sleep(). It cannot be used in
> svc_i3c_master_start_xfer_locked(), because it is in a spin lock.
> 
> Move the pm runtime operations to svc_i3c_master_enqueue_xfer().
> 
> 

Applied, thanks!

[1/1] i3c: master: svc: fix cpu schedule in spin lock
      commit: 33beadb3b1ab74e69db2c49d9663f3a93a273943

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH] i3c: master: svc: add NACK check after start byte sent

[Clark Wang]

Add NACK check after start byte is sent.
It is possible to detect early that a device is not on the bus
and avoid invalid transmissions thereafter.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
---
 drivers/i3c/master/svc-i3c-master.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index 4edf33ed207d..0d63b732ef0c 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -92,6 +92,7 @@
 #define SVC_I3C_MINTCLR      0x094
 #define SVC_I3C_MINTMASKED   0x098
 #define SVC_I3C_MERRWARN     0x09C
+#define   SVC_I3C_MERRWARN_NACK BIT(2)
 #define SVC_I3C_MDMACTRL     0x0A0
 #define SVC_I3C_MDATACTRL    0x0AC
 #define   SVC_I3C_MDATACTRL_FLUSHTB BIT(0)
@@ -1014,6 +1015,11 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
 	if (ret)
 		goto emit_stop;
 
+	if (readl(master->regs + SVC_I3C_MERRWARN) & SVC_I3C_MERRWARN_NACK) {
+		ret = -ENXIO;
+		goto emit_stop;
+	}
+
 	if (rnw)
 		ret = svc_i3c_master_read(master, in, xfer_len);
 	else
-- 
2.34.1

Re: [PATCH] i3c: master: svc: add NACK check after start byte sent

[Miquel Raynal]

Hi Clark,

xiaoning.wang@nxp.com wrote on Wed, 17 May 2023 11:30:30 +0800:

> Add NACK check after start byte is sent.
> It is possible to detect early that a device is not on the bus
> and avoid invalid transmissions thereafter.
> 
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>

Nice addition.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

> ---
>  drivers/i3c/master/svc-i3c-master.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index 4edf33ed207d..0d63b732ef0c 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -92,6 +92,7 @@
>  #define SVC_I3C_MINTCLR      0x094
>  #define SVC_I3C_MINTMASKED   0x098
>  #define SVC_I3C_MERRWARN     0x09C
> +#define   SVC_I3C_MERRWARN_NACK BIT(2)
>  #define SVC_I3C_MDMACTRL     0x0A0
>  #define SVC_I3C_MDATACTRL    0x0AC
>  #define   SVC_I3C_MDATACTRL_FLUSHTB BIT(0)
> @@ -1014,6 +1015,11 @@ static int svc_i3c_master_xfer(struct svc_i3c_master *master,
>  	if (ret)
>  		goto emit_stop;
>  
> +	if (readl(master->regs + SVC_I3C_MERRWARN) & SVC_I3C_MERRWARN_NACK) {
> +		ret = -ENXIO;
> +		goto emit_stop;
> +	}
> +
>  	if (rnw)
>  		ret = svc_i3c_master_read(master, in, xfer_len);
>  	else


Thanks,
Miquèl

Re: [PATCH] i3c: master: svc: fix i3c suspend/resume issue

[Miquel Raynal]

Hi Clark,

xiaoning.wang@nxp.com wrote on Wed, 17 May 2023 11:30:28 +0800:

> When system suspends, if i3c module is powered down, register
> value will lose.
> Need to save the key registers before system suspend.
> So save these registers value in runtime pm suspend, and restore them
> if these register's value is different with the saved values
> when runtime pm resume.
> 
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: (subset) [PATCH] i3c: master: svc: fix i3c suspend/resume issue

[Alexandre Belloni]

On Wed, 17 May 2023 11:30:28 +0800, Clark Wang wrote:
> When system suspends, if i3c module is powered down, register
> value will lose.
> Need to save the key registers before system suspend.
> So save these registers value in runtime pm suspend, and restore them
> if these register's value is different with the saved values
> when runtime pm resume.
> 
> [...]

Applied, thanks!

[1/1] i3c: master: svc: fix i3c suspend/resume issue
      commit: 1c5ee2a77b1bacd4c333bebea93610aaf17977be

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com