2023-06-02 08:58:39

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v3 6/7] soc: renesas: rmobile-sysc: Convert to readl_poll_timeout_atomic()

Use readl_poll_timeout_atomic() instead of open-coding the same
operation.

1. rmobile_pd_power_down(): as typically less than 20 retries are
needed, PSTR_RETRIES (100) µs is a suitable timeout value.

2. __rmobile_pd_power_up(): the old method of first polling some
cycles with a 1 µs delay, followed by more polling cycles without
any delay didn't make much sense, as the latter was insignificant
compared to the former. Furthermore, typically no retries are
needed. Hence just retain the polling with delay.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Polling measurements done on R-Mobile APE6 and A1, and SH-Mobile AG5.

v3:
- New.
---
drivers/soc/renesas/rmobile-sysc.c | 31 ++++++++++--------------------
1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/soc/renesas/rmobile-sysc.c b/drivers/soc/renesas/rmobile-sysc.c
index 728ebac98e14a5cc..5d621c35fba1116a 100644
--- a/drivers/soc/renesas/rmobile-sysc.c
+++ b/drivers/soc/renesas/rmobile-sysc.c
@@ -12,6 +12,8 @@
#include <linux/clk/renesas.h>
#include <linux/console.h>
#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/pm.h>
@@ -19,8 +21,6 @@
#include <linux/pm_domain.h>
#include <linux/slab.h>

-#include <asm/io.h>
-
/* SYSC */
#define SPDCR 0x08 /* SYS Power Down Control Register */
#define SWUCR 0x14 /* SYS Wakeup Control Register */
@@ -47,6 +47,7 @@ static int rmobile_pd_power_down(struct generic_pm_domain *genpd)
{
struct rmobile_pm_domain *rmobile_pd = to_rmobile_pd(genpd);
unsigned int mask = BIT(rmobile_pd->bit_shift);
+ u32 val;

if (rmobile_pd->suspend) {
int ret = rmobile_pd->suspend();
@@ -56,14 +57,10 @@ static int rmobile_pd_power_down(struct generic_pm_domain *genpd)
}

if (readl(rmobile_pd->base + PSTR) & mask) {
- unsigned int retry_count;
writel(mask, rmobile_pd->base + SPDCR);

- for (retry_count = PSTR_RETRIES; retry_count; retry_count--) {
- if (!(readl(rmobile_pd->base + SPDCR) & mask))
- break;
- cpu_relax();
- }
+ readl_poll_timeout_atomic(rmobile_pd->base + SPDCR, val,
+ !(val & mask), 0, PSTR_RETRIES);
}

pr_debug("%s: Power off, 0x%08x -> PSTR = 0x%08x\n", genpd->name, mask,
@@ -74,25 +71,17 @@ static int rmobile_pd_power_down(struct generic_pm_domain *genpd)

static int __rmobile_pd_power_up(struct rmobile_pm_domain *rmobile_pd)
{
- unsigned int mask = BIT(rmobile_pd->bit_shift);
- unsigned int retry_count;
- int ret = 0;
+ unsigned int val, mask = BIT(rmobile_pd->bit_shift);
+ int ret;

if (readl(rmobile_pd->base + PSTR) & mask)
return ret;

writel(mask, rmobile_pd->base + SWUCR);

- for (retry_count = 2 * PSTR_RETRIES; retry_count; retry_count--) {
- if (!(readl(rmobile_pd->base + SWUCR) & mask))
- break;
- if (retry_count > PSTR_RETRIES)
- udelay(PSTR_DELAY_US);
- else
- cpu_relax();
- }
- if (!retry_count)
- ret = -EIO;
+ ret = readl_poll_timeout_atomic(rmobile_pd->base + SWUCR, val,
+ (val & mask), PSTR_DELAY_US,
+ PSTR_RETRIES * PSTR_DELAY_US);

pr_debug("%s: Power on, 0x%08x -> PSTR = 0x%08x\n",
rmobile_pd->genpd.name, mask,
--
2.34.1



2023-06-05 13:56:44

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v3 6/7] soc: renesas: rmobile-sysc: Convert to readl_poll_timeout_atomic()

On Fri, Jun 2, 2023 at 10:51 AM Geert Uytterhoeven
<[email protected]> wrote:
> Use readl_poll_timeout_atomic() instead of open-coding the same
> operation.
>
> 1. rmobile_pd_power_down(): as typically less than 20 retries are
> needed, PSTR_RETRIES (100) µs is a suitable timeout value.
>
> 2. __rmobile_pd_power_up(): the old method of first polling some
> cycles with a 1 µs delay, followed by more polling cycles without
> any delay didn't make much sense, as the latter was insignificant
> compared to the former. Furthermore, typically no retries are
> needed. Hence just retain the polling with delay.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>

> diff --git a/drivers/soc/renesas/rmobile-sysc.c b/drivers/soc/renesas/rmobile-sysc.c
> index 728ebac98e14a5cc..5d621c35fba1116a 100644

> @@ -74,25 +71,17 @@ static int rmobile_pd_power_down(struct generic_pm_domain *genpd)
>
> static int __rmobile_pd_power_up(struct rmobile_pm_domain *rmobile_pd)
> {
> - unsigned int mask = BIT(rmobile_pd->bit_shift);
> - unsigned int retry_count;
> - int ret = 0;
> + unsigned int val, mask = BIT(rmobile_pd->bit_shift);
> + int ret;

Oops, "ret" should still be initialized to zero.

>
> if (readl(rmobile_pd->base + PSTR) & mask)
> return ret;
>
> writel(mask, rmobile_pd->base + SWUCR);
>
> - for (retry_count = 2 * PSTR_RETRIES; retry_count; retry_count--) {
> - if (!(readl(rmobile_pd->base + SWUCR) & mask))
> - break;
> - if (retry_count > PSTR_RETRIES)
> - udelay(PSTR_DELAY_US);
> - else
> - cpu_relax();
> - }
> - if (!retry_count)
> - ret = -EIO;
> + ret = readl_poll_timeout_atomic(rmobile_pd->base + SWUCR, val,
> + (val & mask), PSTR_DELAY_US,
> + PSTR_RETRIES * PSTR_DELAY_US);
>
> pr_debug("%s: Power on, 0x%08x -> PSTR = 0x%08x\n",
> rmobile_pd->genpd.name, mask,

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds