* [PATCH v2 0/2] bus: sunxi-rsb: Fix poweroff issues @ 2022-11-07 5:21 Samuel Holland 2022-11-07 5:21 ` [PATCH v2 1/2] bus: sunxi-rsb: Remove shutdown callback Samuel Holland 2022-11-07 5:22 ` [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers Samuel Holland 0 siblings, 2 replies; 5+ messages in thread From: Samuel Holland @ 2022-11-07 5:21 UTC (permalink / raw) To: Chen-Yu Tsai, Jernej Skrabec Cc: linux-arm-kernel, Ivaylo Dimitrov, linux-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. Changes in v2: - Add Fixes tag to patch 2 - Only check for specific status bits when polling Samuel Holland (2): bus: sunxi-rsb: Remove shutdown callback bus: sunxi-rsb: Support atomic transfers drivers/bus/sunxi-rsb.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) -- 2.37.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] bus: sunxi-rsb: Remove shutdown callback 2022-11-07 5:21 [PATCH v2 0/2] bus: sunxi-rsb: Fix poweroff issues Samuel Holland @ 2022-11-07 5:21 ` Samuel Holland 2022-11-07 5:22 ` [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers Samuel Holland 1 sibling, 0 replies; 5+ messages in thread From: Samuel Holland @ 2022-11-07 5:21 UTC (permalink / raw) To: Chen-Yu Tsai, Jernej Skrabec Cc: linux-arm-kernel, Ivaylo Dimitrov, linux-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> Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> Fixes: 843107498f91 ("bus: sunxi-rsb: Implement suspend/resume/shutdown callbacks") Signed-off-by: Samuel Holland <samuel@sholland.org> --- (no changes since v1) 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] 5+ messages in thread
* [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers 2022-11-07 5:21 [PATCH v2 0/2] bus: sunxi-rsb: Fix poweroff issues Samuel Holland 2022-11-07 5:21 ` [PATCH v2 1/2] bus: sunxi-rsb: Remove shutdown callback Samuel Holland @ 2022-11-07 5:22 ` Samuel Holland 2022-11-07 11:30 ` Andre Przywara 1 sibling, 1 reply; 5+ messages in thread From: Samuel Holland @ 2022-11-07 5:22 UTC (permalink / raw) To: Chen-Yu Tsai, Jernej Skrabec Cc: linux-arm-kernel, Ivaylo Dimitrov, linux-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. Fixes: d787dcdb9c8f ("bus: sunxi-rsb: Add driver for Allwinner Reduced Serial Bus") Signed-off-by: Samuel Holland <samuel@sholland.org> --- Changes in v2: - Add Fixes tag to patch 2 - Only check for specific status bits when polling drivers/bus/sunxi-rsb.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c index 17343cd75338..012e82f9b7b0 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) { + u32 int_mask, status; + bool timeout; + if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) { dev_dbg(rsb->dev, "RSB transfer still in progress\n"); return -EBUSY; @@ -274,13 +277,23 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb) reinit_completion(&rsb->complete); - writel(RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER, + int_mask = RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER; + writel(int_mask, rsb->regs + RSB_INTE); 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 & int_mask), + 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 +305,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] 5+ messages in thread
* Re: [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers 2022-11-07 5:22 ` [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers Samuel Holland @ 2022-11-07 11:30 ` Andre Przywara 2022-11-07 18:05 ` Jernej Škrabec 0 siblings, 1 reply; 5+ messages in thread From: Andre Przywara @ 2022-11-07 11:30 UTC (permalink / raw) To: Samuel Holland Cc: Chen-Yu Tsai, Jernej Skrabec, linux-arm-kernel, Ivaylo Dimitrov, linux-kernel, linux-sunxi On Sun, 6 Nov 2022 23:22:00 -0600 Samuel Holland <samuel@sholland.org> wrote: Hi, > 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. > > Fixes: d787dcdb9c8f ("bus: sunxi-rsb: Add driver for Allwinner Reduced Serial Bus") > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > Changes in v2: > - Add Fixes tag to patch 2 > - Only check for specific status bits when polling > > drivers/bus/sunxi-rsb.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c > index 17343cd75338..012e82f9b7b0 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) > { > + u32 int_mask, status; > + bool timeout; > + > if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) { > dev_dbg(rsb->dev, "RSB transfer still in progress\n"); > return -EBUSY; > @@ -274,13 +277,23 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb) > > reinit_completion(&rsb->complete); > > - writel(RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER, > + int_mask = RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER; > + writel(int_mask, > rsb->regs + RSB_INTE); > 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 & int_mask), > + 10, 100000); So if I understand correctly, this mimics the operation of sunxi_rsb_irq(), just replacing rsb->status with status. But wouldn't that also mean that we need to clear the interrupt bits in INTS, since we are about to handle them below? It probably doesn't matter in practise, since we call this during power down only, but looks like more robust to do, from a driver's perspective. Cheers, Andre > + } 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 +305,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] 5+ messages in thread
* Re: Re: [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers 2022-11-07 11:30 ` Andre Przywara @ 2022-11-07 18:05 ` Jernej Škrabec 0 siblings, 0 replies; 5+ messages in thread From: Jernej Škrabec @ 2022-11-07 18:05 UTC (permalink / raw) To: Samuel Holland, Andre Przywara Cc: Chen-Yu Tsai, linux-arm-kernel, Ivaylo Dimitrov, linux-kernel, linux-sunxi Dne ponedeljek, 07. november 2022 ob 12:30:29 CET je Andre Przywara napisal(a): > On Sun, 6 Nov 2022 23:22:00 -0600 > Samuel Holland <samuel@sholland.org> wrote: > > Hi, > > > 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. > > > > Fixes: d787dcdb9c8f ("bus: sunxi-rsb: Add driver for Allwinner Reduced > > Serial Bus") Signed-off-by: Samuel Holland <samuel@sholland.org> > > --- > > > > Changes in v2: > > - Add Fixes tag to patch 2 > > - Only check for specific status bits when polling > > > > drivers/bus/sunxi-rsb.c | 27 ++++++++++++++++++++------- > > 1 file changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c > > index 17343cd75338..012e82f9b7b0 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) > > { > > > > + u32 int_mask, status; > > + bool timeout; > > + > > > > if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) { > > > > dev_dbg(rsb->dev, "RSB transfer still in progress\n"); > > return -EBUSY; > > > > @@ -274,13 +277,23 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb > > *rsb) > > > > reinit_completion(&rsb->complete); > > > > - writel(RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER, > > + int_mask = RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER; > > + writel(int_mask, > > > > rsb->regs + RSB_INTE); > > > > 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 & int_mask), > > + 10, 100000); > > So if I understand correctly, this mimics the operation of > sunxi_rsb_irq(), just replacing rsb->status with status. > But wouldn't that also mean that we need to clear the interrupt bits in > INTS, since we are about to handle them below? Yes, I pointed out that in review of v1. Best regards, Jernej > > It probably doesn't matter in practise, since we call this during power > down only, but looks like more robust to do, from a driver's perspective. > > Cheers, > Andre > > > + } 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 +305,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] 5+ messages in thread
end of thread, other threads:[~2022-11-07 18:09 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-07 5:21 [PATCH v2 0/2] bus: sunxi-rsb: Fix poweroff issues Samuel Holland 2022-11-07 5:21 ` [PATCH v2 1/2] bus: sunxi-rsb: Remove shutdown callback Samuel Holland 2022-11-07 5:22 ` [PATCH v2 2/2] bus: sunxi-rsb: Support atomic transfers Samuel Holland 2022-11-07 11:30 ` Andre Przywara 2022-11-07 18:05 ` Jernej Škrabec
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox