All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] bus: sunxi-rsb: Fix poweroff issues
@ 2022-11-05 19:19 ` Samuel Holland
  0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-11-05 19:19 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jernej Skrabec
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

This series fixes a couple of issues that occur when powering off a
board using a PMIC attached to the RSB bus.

These issues only affected 32-bit platforms, because 64-bit platforms
use PSCI for pm_power_off(), and the PSCI firmware reinitializes the
RSB controller.


Samuel Holland (2):
  bus: sunxi-rsb: Remove shutdown callback
  bus: sunxi-rsb: Support atomic transfers

 drivers/bus/sunxi-rsb.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 0/2] bus: sunxi-rsb: Fix poweroff issues
@ 2022-11-05 19:19 ` Samuel Holland
  0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-11-05 19:19 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jernej Skrabec
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

This series fixes a couple of issues that occur when powering off a
board using a PMIC attached to the RSB bus.

These issues only affected 32-bit platforms, because 64-bit platforms
use PSCI for pm_power_off(), and the PSCI firmware reinitializes the
RSB controller.


Samuel Holland (2):
  bus: sunxi-rsb: Remove shutdown callback
  bus: sunxi-rsb: Support atomic transfers

 drivers/bus/sunxi-rsb.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback
  2022-11-05 19:19 ` Samuel Holland
@ 2022-11-05 19:19   ` Samuel Holland
  -1 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-11-05 19:19 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jernej Skrabec
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Shutting down the RSB controller prevents communicating with a PMIC
inside pm_power_off(), so it breaks system poweroff on some boards.

Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown callbacks")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/bus/sunxi-rsb.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 4cd2e127946e..17343cd75338 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -812,14 +812,6 @@ static int sunxi_rsb_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static void sunxi_rsb_shutdown(struct platform_device *pdev)
-{
-	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
-
-	pm_runtime_disable(&pdev->dev);
-	sunxi_rsb_hw_exit(rsb);
-}
-
 static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
 	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
 			   sunxi_rsb_runtime_resume, NULL)
@@ -835,7 +827,6 @@ MODULE_DEVICE_TABLE(of, sunxi_rsb_of_match_table);
 static struct platform_driver sunxi_rsb_driver = {
 	.probe = sunxi_rsb_probe,
 	.remove	= sunxi_rsb_remove,
-	.shutdown = sunxi_rsb_shutdown,
 	.driver	= {
 		.name = RSB_CTRL_NAME,
 		.of_match_table = sunxi_rsb_of_match_table,
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback
@ 2022-11-05 19:19   ` Samuel Holland
  0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-11-05 19:19 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jernej Skrabec
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Shutting down the RSB controller prevents communicating with a PMIC
inside pm_power_off(), so it breaks system poweroff on some boards.

Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown callbacks")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/bus/sunxi-rsb.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 4cd2e127946e..17343cd75338 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -812,14 +812,6 @@ static int sunxi_rsb_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static void sunxi_rsb_shutdown(struct platform_device *pdev)
-{
-	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
-
-	pm_runtime_disable(&pdev->dev);
-	sunxi_rsb_hw_exit(rsb);
-}
-
 static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
 	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
 			   sunxi_rsb_runtime_resume, NULL)
@@ -835,7 +827,6 @@ MODULE_DEVICE_TABLE(of, sunxi_rsb_of_match_table);
 static struct platform_driver sunxi_rsb_driver = {
 	.probe = sunxi_rsb_probe,
 	.remove	= sunxi_rsb_remove,
-	.shutdown = sunxi_rsb_shutdown,
 	.driver	= {
 		.name = RSB_CTRL_NAME,
 		.of_match_table = sunxi_rsb_of_match_table,
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers
  2022-11-05 19:19 ` Samuel Holland
@ 2022-11-05 19:19   ` Samuel Holland
  -1 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-11-05 19:19 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jernej Skrabec
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

When communicating with a PMIC during system poweroff (pm_power_off()),
IRQs are disabled and we are in a RCU read-side critical section, so we
cannot use wait_for_completion_io_timeout(). Instead, poll the status
register for transfer completion.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/bus/sunxi-rsb.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 17343cd75338..0f0e498d4379 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -267,6 +267,9 @@ EXPORT_SYMBOL_GPL(sunxi_rsb_driver_register);
 /* common code that starts a transfer */
 static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
 {
+	bool timeout;
+	u32 status;
+
 	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
 		dev_dbg(rsb->dev, "RSB transfer still in progress\n");
 		return -EBUSY;
@@ -279,8 +282,16 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
 	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
 	       rsb->regs + RSB_CTRL);
 
-	if (!wait_for_completion_io_timeout(&rsb->complete,
-					    msecs_to_jiffies(100))) {
+	if (irqs_disabled()) {
+		timeout = readl_poll_timeout_atomic(rsb->regs + RSB_INTS,
+						    status, status, 10, 100000);
+	} else {
+		timeout = !wait_for_completion_io_timeout(&rsb->complete,
+							  msecs_to_jiffies(100));
+		status = rsb->status;
+	}
+
+	if (timeout) {
 		dev_dbg(rsb->dev, "RSB timeout\n");
 
 		/* abort the transfer */
@@ -292,18 +303,18 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
 		return -ETIMEDOUT;
 	}
 
-	if (rsb->status & RSB_INTS_LOAD_BSY) {
+	if (status & RSB_INTS_LOAD_BSY) {
 		dev_dbg(rsb->dev, "RSB busy\n");
 		return -EBUSY;
 	}
 
-	if (rsb->status & RSB_INTS_TRANS_ERR) {
-		if (rsb->status & RSB_INTS_TRANS_ERR_ACK) {
+	if (status & RSB_INTS_TRANS_ERR) {
+		if (status & RSB_INTS_TRANS_ERR_ACK) {
 			dev_dbg(rsb->dev, "RSB slave nack\n");
 			return -EINVAL;
 		}
 
-		if (rsb->status & RSB_INTS_TRANS_ERR_DATA) {
+		if (status & RSB_INTS_TRANS_ERR_DATA) {
 			dev_dbg(rsb->dev, "RSB transfer data error\n");
 			return -EIO;
 		}
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers
@ 2022-11-05 19:19   ` Samuel Holland
  0 siblings, 0 replies; 14+ messages in thread
From: Samuel Holland @ 2022-11-05 19:19 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jernej Skrabec
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

When communicating with a PMIC during system poweroff (pm_power_off()),
IRQs are disabled and we are in a RCU read-side critical section, so we
cannot use wait_for_completion_io_timeout(). Instead, poll the status
register for transfer completion.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/bus/sunxi-rsb.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 17343cd75338..0f0e498d4379 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -267,6 +267,9 @@ EXPORT_SYMBOL_GPL(sunxi_rsb_driver_register);
 /* common code that starts a transfer */
 static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
 {
+	bool timeout;
+	u32 status;
+
 	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
 		dev_dbg(rsb->dev, "RSB transfer still in progress\n");
 		return -EBUSY;
@@ -279,8 +282,16 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
 	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
 	       rsb->regs + RSB_CTRL);
 
-	if (!wait_for_completion_io_timeout(&rsb->complete,
-					    msecs_to_jiffies(100))) {
+	if (irqs_disabled()) {
+		timeout = readl_poll_timeout_atomic(rsb->regs + RSB_INTS,
+						    status, status, 10, 100000);
+	} else {
+		timeout = !wait_for_completion_io_timeout(&rsb->complete,
+							  msecs_to_jiffies(100));
+		status = rsb->status;
+	}
+
+	if (timeout) {
 		dev_dbg(rsb->dev, "RSB timeout\n");
 
 		/* abort the transfer */
@@ -292,18 +303,18 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
 		return -ETIMEDOUT;
 	}
 
-	if (rsb->status & RSB_INTS_LOAD_BSY) {
+	if (status & RSB_INTS_LOAD_BSY) {
 		dev_dbg(rsb->dev, "RSB busy\n");
 		return -EBUSY;
 	}
 
-	if (rsb->status & RSB_INTS_TRANS_ERR) {
-		if (rsb->status & RSB_INTS_TRANS_ERR_ACK) {
+	if (status & RSB_INTS_TRANS_ERR) {
+		if (status & RSB_INTS_TRANS_ERR_ACK) {
 			dev_dbg(rsb->dev, "RSB slave nack\n");
 			return -EINVAL;
 		}
 
-		if (rsb->status & RSB_INTS_TRANS_ERR_DATA) {
+		if (status & RSB_INTS_TRANS_ERR_DATA) {
 			dev_dbg(rsb->dev, "RSB transfer data error\n");
 			return -EIO;
 		}
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback
  2022-11-05 19:19   ` Samuel Holland
@ 2022-11-05 19:29     ` Jernej Škrabec
  -1 siblings, 0 replies; 14+ messages in thread
From: Jernej Škrabec @ 2022-11-05 19:29 UTC (permalink / raw)
  To: Chen-Yu Tsai, Samuel Holland
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Dne sobota, 05. november 2022 ob 20:19:52 CET je Samuel Holland napisal(a):
> Shutting down the RSB controller prevents communicating with a PMIC
> inside pm_power_off(), so it breaks system poweroff on some boards.
> 
> Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown
> callbacks") Signed-off-by: Samuel Holland <samuel@sholland.org>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
> 
>  drivers/bus/sunxi-rsb.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index 4cd2e127946e..17343cd75338 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -812,14 +812,6 @@ static int sunxi_rsb_remove(struct platform_device
> *pdev) return 0;
>  }
> 
> -static void sunxi_rsb_shutdown(struct platform_device *pdev)
> -{
> -	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
> -
> -	pm_runtime_disable(&pdev->dev);
> -	sunxi_rsb_hw_exit(rsb);
> -}
> -
>  static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
>  	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
>  			   sunxi_rsb_runtime_resume, NULL)
> @@ -835,7 +827,6 @@ MODULE_DEVICE_TABLE(of, sunxi_rsb_of_match_table);
>  static struct platform_driver sunxi_rsb_driver = {
>  	.probe = sunxi_rsb_probe,
>  	.remove	= sunxi_rsb_remove,
> -	.shutdown = sunxi_rsb_shutdown,
>  	.driver	= {
>  		.name = RSB_CTRL_NAME,
>  		.of_match_table = sunxi_rsb_of_match_table,





^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback
@ 2022-11-05 19:29     ` Jernej Škrabec
  0 siblings, 0 replies; 14+ messages in thread
From: Jernej Škrabec @ 2022-11-05 19:29 UTC (permalink / raw)
  To: Chen-Yu Tsai, Samuel Holland
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Dne sobota, 05. november 2022 ob 20:19:52 CET je Samuel Holland napisal(a):
> Shutting down the RSB controller prevents communicating with a PMIC
> inside pm_power_off(), so it breaks system poweroff on some boards.
> 
> Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown
> callbacks") Signed-off-by: Samuel Holland <samuel@sholland.org>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
> 
>  drivers/bus/sunxi-rsb.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index 4cd2e127946e..17343cd75338 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -812,14 +812,6 @@ static int sunxi_rsb_remove(struct platform_device
> *pdev) return 0;
>  }
> 
> -static void sunxi_rsb_shutdown(struct platform_device *pdev)
> -{
> -	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
> -
> -	pm_runtime_disable(&pdev->dev);
> -	sunxi_rsb_hw_exit(rsb);
> -}
> -
>  static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
>  	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
>  			   sunxi_rsb_runtime_resume, NULL)
> @@ -835,7 +827,6 @@ MODULE_DEVICE_TABLE(of, sunxi_rsb_of_match_table);
>  static struct platform_driver sunxi_rsb_driver = {
>  	.probe = sunxi_rsb_probe,
>  	.remove	= sunxi_rsb_remove,
> -	.shutdown = sunxi_rsb_shutdown,
>  	.driver	= {
>  		.name = RSB_CTRL_NAME,
>  		.of_match_table = sunxi_rsb_of_match_table,





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers
  2022-11-05 19:19   ` Samuel Holland
@ 2022-11-05 19:46     ` Jernej Škrabec
  -1 siblings, 0 replies; 14+ messages in thread
From: Jernej Škrabec @ 2022-11-05 19:46 UTC (permalink / raw)
  To: Chen-Yu Tsai, Samuel Holland
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Dne sobota, 05. november 2022 ob 20:19:53 CET je Samuel Holland napisal(a):
> When communicating with a PMIC during system poweroff (pm_power_off()),
> IRQs are disabled and we are in a RCU read-side critical section, so we
> cannot use wait_for_completion_io_timeout(). Instead, poll the status
> register for transfer completion.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  drivers/bus/sunxi-rsb.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index 17343cd75338..0f0e498d4379 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -267,6 +267,9 @@ EXPORT_SYMBOL_GPL(sunxi_rsb_driver_register);
>  /* common code that starts a transfer */
>  static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>  {
> +	bool timeout;
> +	u32 status;
> +
>  	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
>  		dev_dbg(rsb->dev, "RSB transfer still in progress\n");
>  		return -EBUSY;
> @@ -279,8 +282,16 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>  	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
>  	       rsb->regs + RSB_CTRL);
> 
> -	if (!wait_for_completion_io_timeout(&rsb->complete,
> -					    
msecs_to_jiffies(100))) {
> +	if (irqs_disabled()) {
> +		timeout = readl_poll_timeout_atomic(rsb->regs + 
RSB_INTS,
> +						    status, 
status, 10, 100000);

It would be good to check only for RSB_INTS_LOAD_BSY, RSB_INTS_TRANS_ERR and 
RSB_INTS_TRANS_OVER flags and clear them afterwards. That way we avoid problems 
if this path is used outside power off case.

Best regards,
Jernej

> +	} else {
> +		timeout = !wait_for_completion_io_timeout(&rsb-
>complete,
> +							  
msecs_to_jiffies(100));
> +		status = rsb->status;
> +	}
> +
> +	if (timeout) {
>  		dev_dbg(rsb->dev, "RSB timeout\n");
> 
>  		/* abort the transfer */
> @@ -292,18 +303,18 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>  		return -ETIMEDOUT;
>  	}
> 
> -	if (rsb->status & RSB_INTS_LOAD_BSY) {
> +	if (status & RSB_INTS_LOAD_BSY) {
>  		dev_dbg(rsb->dev, "RSB busy\n");
>  		return -EBUSY;
>  	}
> 
> -	if (rsb->status & RSB_INTS_TRANS_ERR) {
> -		if (rsb->status & RSB_INTS_TRANS_ERR_ACK) {
> +	if (status & RSB_INTS_TRANS_ERR) {
> +		if (status & RSB_INTS_TRANS_ERR_ACK) {
>  			dev_dbg(rsb->dev, "RSB slave nack\n");
>  			return -EINVAL;
>  		}
> 
> -		if (rsb->status & RSB_INTS_TRANS_ERR_DATA) {
> +		if (status & RSB_INTS_TRANS_ERR_DATA) {
>  			dev_dbg(rsb->dev, "RSB transfer data 
error\n");
>  			return -EIO;
>  		}





^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers
@ 2022-11-05 19:46     ` Jernej Škrabec
  0 siblings, 0 replies; 14+ messages in thread
From: Jernej Škrabec @ 2022-11-05 19:46 UTC (permalink / raw)
  To: Chen-Yu Tsai, Samuel Holland
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Dne sobota, 05. november 2022 ob 20:19:53 CET je Samuel Holland napisal(a):
> When communicating with a PMIC during system poweroff (pm_power_off()),
> IRQs are disabled and we are in a RCU read-side critical section, so we
> cannot use wait_for_completion_io_timeout(). Instead, poll the status
> register for transfer completion.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  drivers/bus/sunxi-rsb.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index 17343cd75338..0f0e498d4379 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -267,6 +267,9 @@ EXPORT_SYMBOL_GPL(sunxi_rsb_driver_register);
>  /* common code that starts a transfer */
>  static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>  {
> +	bool timeout;
> +	u32 status;
> +
>  	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
>  		dev_dbg(rsb->dev, "RSB transfer still in progress\n");
>  		return -EBUSY;
> @@ -279,8 +282,16 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>  	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
>  	       rsb->regs + RSB_CTRL);
> 
> -	if (!wait_for_completion_io_timeout(&rsb->complete,
> -					    
msecs_to_jiffies(100))) {
> +	if (irqs_disabled()) {
> +		timeout = readl_poll_timeout_atomic(rsb->regs + 
RSB_INTS,
> +						    status, 
status, 10, 100000);

It would be good to check only for RSB_INTS_LOAD_BSY, RSB_INTS_TRANS_ERR and 
RSB_INTS_TRANS_OVER flags and clear them afterwards. That way we avoid problems 
if this path is used outside power off case.

Best regards,
Jernej

> +	} else {
> +		timeout = !wait_for_completion_io_timeout(&rsb-
>complete,
> +							  
msecs_to_jiffies(100));
> +		status = rsb->status;
> +	}
> +
> +	if (timeout) {
>  		dev_dbg(rsb->dev, "RSB timeout\n");
> 
>  		/* abort the transfer */
> @@ -292,18 +303,18 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>  		return -ETIMEDOUT;
>  	}
> 
> -	if (rsb->status & RSB_INTS_LOAD_BSY) {
> +	if (status & RSB_INTS_LOAD_BSY) {
>  		dev_dbg(rsb->dev, "RSB busy\n");
>  		return -EBUSY;
>  	}
> 
> -	if (rsb->status & RSB_INTS_TRANS_ERR) {
> -		if (rsb->status & RSB_INTS_TRANS_ERR_ACK) {
> +	if (status & RSB_INTS_TRANS_ERR) {
> +		if (status & RSB_INTS_TRANS_ERR_ACK) {
>  			dev_dbg(rsb->dev, "RSB slave nack\n");
>  			return -EINVAL;
>  		}
> 
> -		if (rsb->status & RSB_INTS_TRANS_ERR_DATA) {
> +		if (status & RSB_INTS_TRANS_ERR_DATA) {
>  			dev_dbg(rsb->dev, "RSB transfer data 
error\n");
>  			return -EIO;
>  		}





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers
  2022-11-05 19:46     ` Jernej Škrabec
@ 2022-11-05 19:48       ` Jernej Škrabec
  -1 siblings, 0 replies; 14+ messages in thread
From: Jernej Škrabec @ 2022-11-05 19:48 UTC (permalink / raw)
  To: Chen-Yu Tsai, Samuel Holland
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Dne sobota, 05. november 2022 ob 20:46:47 CET je Jernej Škrabec napisal(a):
> Dne sobota, 05. november 2022 ob 20:19:53 CET je Samuel Holland napisal(a):
> > When communicating with a PMIC during system poweroff (pm_power_off()),
> > IRQs are disabled and we are in a RCU read-side critical section, so we
> > cannot use wait_for_completion_io_timeout(). Instead, poll the status
> > register for transfer completion.
> > 
> > Signed-off-by: Samuel Holland <samuel@sholland.org>

Also a fixes tag would be in order here.

Best regards,
Jernej

> > ---
> > 
> >  drivers/bus/sunxi-rsb.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> > index 17343cd75338..0f0e498d4379 100644
> > --- a/drivers/bus/sunxi-rsb.c
> > +++ b/drivers/bus/sunxi-rsb.c
> > @@ -267,6 +267,9 @@ EXPORT_SYMBOL_GPL(sunxi_rsb_driver_register);
> > 
> >  /* common code that starts a transfer */
> >  static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
> >  {
> > 
> > +	bool timeout;
> > +	u32 status;
> > +
> > 
> >  	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
> >  	
> >  		dev_dbg(rsb->dev, "RSB transfer still in progress\n");
> >  		return -EBUSY;
> > 
> > @@ -279,8 +282,16 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
> > 
> >  	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
> >  	
> >  	       rsb->regs + RSB_CTRL);
> > 
> > -	if (!wait_for_completion_io_timeout(&rsb->complete,
> > -
> 
> msecs_to_jiffies(100))) {
> 
> > +	if (irqs_disabled()) {
> > +		timeout = readl_poll_timeout_atomic(rsb->regs +
> 
> RSB_INTS,
> 
> > +						    status,
> 
> status, 10, 100000);
> 
> It would be good to check only for RSB_INTS_LOAD_BSY, RSB_INTS_TRANS_ERR and
> RSB_INTS_TRANS_OVER flags and clear them afterwards. That way we avoid
> problems if this path is used outside power off case.
> 
> Best regards,
> Jernej
> 
> > +	} else {
> > +		timeout = !wait_for_completion_io_timeout(&rsb-
> >
> >complete,
> >
> > +
> 
> msecs_to_jiffies(100));
> 
> > +		status = rsb->status;
> > +	}
> > +
> > +	if (timeout) {
> > 
> >  		dev_dbg(rsb->dev, "RSB timeout\n");
> >  		
> >  		/* abort the transfer */
> > 
> > @@ -292,18 +303,18 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb
> > *rsb)
> > 
> >  		return -ETIMEDOUT;
> >  	
> >  	}
> > 
> > -	if (rsb->status & RSB_INTS_LOAD_BSY) {
> > +	if (status & RSB_INTS_LOAD_BSY) {
> > 
> >  		dev_dbg(rsb->dev, "RSB busy\n");
> >  		return -EBUSY;
> >  	
> >  	}
> > 
> > -	if (rsb->status & RSB_INTS_TRANS_ERR) {
> > -		if (rsb->status & RSB_INTS_TRANS_ERR_ACK) {
> > +	if (status & RSB_INTS_TRANS_ERR) {
> > +		if (status & RSB_INTS_TRANS_ERR_ACK) {
> > 
> >  			dev_dbg(rsb->dev, "RSB slave nack\n");
> >  			return -EINVAL;
> >  		
> >  		}
> > 
> > -		if (rsb->status & RSB_INTS_TRANS_ERR_DATA) {
> > +		if (status & RSB_INTS_TRANS_ERR_DATA) {
> > 
> >  			dev_dbg(rsb->dev, "RSB transfer data
> 
> error\n");
> 
> >  			return -EIO;
> >  		
> >  		}





^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers
@ 2022-11-05 19:48       ` Jernej Škrabec
  0 siblings, 0 replies; 14+ messages in thread
From: Jernej Škrabec @ 2022-11-05 19:48 UTC (permalink / raw)
  To: Chen-Yu Tsai, Samuel Holland
  Cc: Ivaylo Dimitrov, linux-kernel, linux-arm-kernel, linux-sunxi,
	Samuel Holland

Dne sobota, 05. november 2022 ob 20:46:47 CET je Jernej Škrabec napisal(a):
> Dne sobota, 05. november 2022 ob 20:19:53 CET je Samuel Holland napisal(a):
> > When communicating with a PMIC during system poweroff (pm_power_off()),
> > IRQs are disabled and we are in a RCU read-side critical section, so we
> > cannot use wait_for_completion_io_timeout(). Instead, poll the status
> > register for transfer completion.
> > 
> > Signed-off-by: Samuel Holland <samuel@sholland.org>

Also a fixes tag would be in order here.

Best regards,
Jernej

> > ---
> > 
> >  drivers/bus/sunxi-rsb.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> > index 17343cd75338..0f0e498d4379 100644
> > --- a/drivers/bus/sunxi-rsb.c
> > +++ b/drivers/bus/sunxi-rsb.c
> > @@ -267,6 +267,9 @@ EXPORT_SYMBOL_GPL(sunxi_rsb_driver_register);
> > 
> >  /* common code that starts a transfer */
> >  static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
> >  {
> > 
> > +	bool timeout;
> > +	u32 status;
> > +
> > 
> >  	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
> >  	
> >  		dev_dbg(rsb->dev, "RSB transfer still in progress\n");
> >  		return -EBUSY;
> > 
> > @@ -279,8 +282,16 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
> > 
> >  	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
> >  	
> >  	       rsb->regs + RSB_CTRL);
> > 
> > -	if (!wait_for_completion_io_timeout(&rsb->complete,
> > -
> 
> msecs_to_jiffies(100))) {
> 
> > +	if (irqs_disabled()) {
> > +		timeout = readl_poll_timeout_atomic(rsb->regs +
> 
> RSB_INTS,
> 
> > +						    status,
> 
> status, 10, 100000);
> 
> It would be good to check only for RSB_INTS_LOAD_BSY, RSB_INTS_TRANS_ERR and
> RSB_INTS_TRANS_OVER flags and clear them afterwards. That way we avoid
> problems if this path is used outside power off case.
> 
> Best regards,
> Jernej
> 
> > +	} else {
> > +		timeout = !wait_for_completion_io_timeout(&rsb-
> >
> >complete,
> >
> > +
> 
> msecs_to_jiffies(100));
> 
> > +		status = rsb->status;
> > +	}
> > +
> > +	if (timeout) {
> > 
> >  		dev_dbg(rsb->dev, "RSB timeout\n");
> >  		
> >  		/* abort the transfer */
> > 
> > @@ -292,18 +303,18 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb
> > *rsb)
> > 
> >  		return -ETIMEDOUT;
> >  	
> >  	}
> > 
> > -	if (rsb->status & RSB_INTS_LOAD_BSY) {
> > +	if (status & RSB_INTS_LOAD_BSY) {
> > 
> >  		dev_dbg(rsb->dev, "RSB busy\n");
> >  		return -EBUSY;
> >  	
> >  	}
> > 
> > -	if (rsb->status & RSB_INTS_TRANS_ERR) {
> > -		if (rsb->status & RSB_INTS_TRANS_ERR_ACK) {
> > +	if (status & RSB_INTS_TRANS_ERR) {
> > +		if (status & RSB_INTS_TRANS_ERR_ACK) {
> > 
> >  			dev_dbg(rsb->dev, "RSB slave nack\n");
> >  			return -EINVAL;
> >  		
> >  		}
> > 
> > -		if (rsb->status & RSB_INTS_TRANS_ERR_DATA) {
> > +		if (status & RSB_INTS_TRANS_ERR_DATA) {
> > 
> >  			dev_dbg(rsb->dev, "RSB transfer data
> 
> error\n");
> 
> >  			return -EIO;
> >  		
> >  		}





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback
  2022-11-05 19:29     ` Jernej Škrabec
@ 2022-11-06  7:59       ` Ivaylo Dimitrov
  -1 siblings, 0 replies; 14+ messages in thread
From: Ivaylo Dimitrov @ 2022-11-06  7:59 UTC (permalink / raw)
  To: Jernej Škrabec, Chen-Yu Tsai, Samuel Holland
  Cc: linux-kernel, linux-arm-kernel, linux-sunxi

On 5.11.22 г. 21:29 ч., Jernej Škrabec wrote:
> Dne sobota, 05. november 2022 ob 20:19:52 CET je Samuel Holland napisal(a):
>> Shutting down the RSB controller prevents communicating with a PMIC
>> inside pm_power_off(), so it breaks system poweroff on some boards.
>>
>> Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>> Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown
>> callbacks") Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> 

Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

Thanks,
Ivo

> Best regards,
> Jernej
> 
>> ---
>>
>>   drivers/bus/sunxi-rsb.c | 9 ---------
>>   1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
>> index 4cd2e127946e..17343cd75338 100644
>> --- a/drivers/bus/sunxi-rsb.c
>> +++ b/drivers/bus/sunxi-rsb.c
>> @@ -812,14 +812,6 @@ static int sunxi_rsb_remove(struct platform_device
>> *pdev) return 0;
>>   }
>>
>> -static void sunxi_rsb_shutdown(struct platform_device *pdev)
>> -{
>> -	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
>> -
>> -	pm_runtime_disable(&pdev->dev);
>> -	sunxi_rsb_hw_exit(rsb);
>> -}
>> -
>>   static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
>>   	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
>>   			   sunxi_rsb_runtime_resume, NULL)
>> @@ -835,7 +827,6 @@ MODULE_DEVICE_TABLE(of, sunxi_rsb_of_match_table);
>>   static struct platform_driver sunxi_rsb_driver = {
>>   	.probe = sunxi_rsb_probe,
>>   	.remove	= sunxi_rsb_remove,
>> -	.shutdown = sunxi_rsb_shutdown,
>>   	.driver	= {
>>   		.name = RSB_CTRL_NAME,
>>   		.of_match_table = sunxi_rsb_of_match_table,
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback
@ 2022-11-06  7:59       ` Ivaylo Dimitrov
  0 siblings, 0 replies; 14+ messages in thread
From: Ivaylo Dimitrov @ 2022-11-06  7:59 UTC (permalink / raw)
  To: Jernej Škrabec, Chen-Yu Tsai, Samuel Holland
  Cc: linux-kernel, linux-arm-kernel, linux-sunxi

On 5.11.22 г. 21:29 ч., Jernej Škrabec wrote:
> Dne sobota, 05. november 2022 ob 20:19:52 CET je Samuel Holland napisal(a):
>> Shutting down the RSB controller prevents communicating with a PMIC
>> inside pm_power_off(), so it breaks system poweroff on some boards.
>>
>> Reported-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>> Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown
>> callbacks") Signed-off-by: Samuel Holland <samuel@sholland.org>
> 
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> 

Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

Thanks,
Ivo

> Best regards,
> Jernej
> 
>> ---
>>
>>   drivers/bus/sunxi-rsb.c | 9 ---------
>>   1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
>> index 4cd2e127946e..17343cd75338 100644
>> --- a/drivers/bus/sunxi-rsb.c
>> +++ b/drivers/bus/sunxi-rsb.c
>> @@ -812,14 +812,6 @@ static int sunxi_rsb_remove(struct platform_device
>> *pdev) return 0;
>>   }
>>
>> -static void sunxi_rsb_shutdown(struct platform_device *pdev)
>> -{
>> -	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
>> -
>> -	pm_runtime_disable(&pdev->dev);
>> -	sunxi_rsb_hw_exit(rsb);
>> -}
>> -
>>   static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
>>   	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
>>   			   sunxi_rsb_runtime_resume, NULL)
>> @@ -835,7 +827,6 @@ MODULE_DEVICE_TABLE(of, sunxi_rsb_of_match_table);
>>   static struct platform_driver sunxi_rsb_driver = {
>>   	.probe = sunxi_rsb_probe,
>>   	.remove	= sunxi_rsb_remove,
>> -	.shutdown = sunxi_rsb_shutdown,
>>   	.driver	= {
>>   		.name = RSB_CTRL_NAME,
>>   		.of_match_table = sunxi_rsb_of_match_table,
> 
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-11-06  8:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-05 19:19 [PATCH 0/2] bus: sunxi-rsb: Fix poweroff issues Samuel Holland
2022-11-05 19:19 ` Samuel Holland
2022-11-05 19:19 ` [PATCH 1/2] bus: sunxi-rsb: Remove shutdown callback Samuel Holland
2022-11-05 19:19   ` Samuel Holland
2022-11-05 19:29   ` Jernej Škrabec
2022-11-05 19:29     ` Jernej Škrabec
2022-11-06  7:59     ` Ivaylo Dimitrov
2022-11-06  7:59       ` Ivaylo Dimitrov
2022-11-05 19:19 ` [PATCH 2/2] bus: sunxi-rsb: Support atomic transfers Samuel Holland
2022-11-05 19:19   ` Samuel Holland
2022-11-05 19:46   ` Jernej Škrabec
2022-11-05 19:46     ` Jernej Škrabec
2022-11-05 19:48     ` Jernej Škrabec
2022-11-05 19:48       ` Jernej Škrabec

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.