| Message ID | 20220515202032.3046-10-stefan.wahren@i2se.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: bcm2835-power: Prepare BCM2711 V3D support | expand |
On Sun, May 15, 2022 at 9:21 PM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > The macros in order to access the ASB registers have a hard coded base > address. So extending them for other platforms would make them harder > to read. As a solution resolve these macros. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> A minor query below: Reviewed-by: Peter Robinson <pbrobinson@gmail.com> > --- > drivers/soc/bcm/bcm2835-power.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c > index 77dc9e62b207..fa0a13035794 100644 > --- a/drivers/soc/bcm/bcm2835-power.c > +++ b/drivers/soc/bcm/bcm2835-power.c > @@ -126,9 +126,6 @@ > > #define ASB_AXI_BRDG_ID 0x20 > > -#define ASB_READ(reg) readl(power->asb + (reg)) > -#define ASB_WRITE(reg, val) writel(PM_PASSWORD | (val), power->asb + (reg)) > - > struct bcm2835_power_domain { > struct generic_pm_domain base; > struct bcm2835_power *power; > @@ -150,7 +147,10 @@ struct bcm2835_power { > > static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable) > { > + void __iomem *base = power->asb; I wonder the perf of defining base here vs the readability of power->asb; throughout. > u64 start; > + u32 val; > + > > if (!reg) > return 0; > @@ -159,12 +159,13 @@ static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable > > /* Enable the module's async AXI bridges. */ > if (enable) { > - ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP); > + val = readl(base + reg) & ~ASB_REQ_STOP; > } else { > - ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP); > + val = readl(base + reg) | ASB_REQ_STOP; > } > + writel(PM_PASSWORD | val, base + reg); > > - while (ASB_READ(reg) & ASB_ACK) { > + while (readl(base + reg) & ASB_ACK) { > cpu_relax(); > if (ktime_get_ns() - start >= 1000) > return -ETIMEDOUT; > @@ -622,7 +623,7 @@ static int bcm2835_power_probe(struct platform_device *pdev) > power->base = pm->base; > power->asb = pm->asb; > > - id = ASB_READ(ASB_AXI_BRDG_ID); > + id = readl(power->asb + ASB_AXI_BRDG_ID); > if (id != 0x62726467 /* "BRDG" */) { > dev_err(dev, "ASB register ID returned 0x%08x\n", id); > return -ENODEV; > -- > 2.25.1 >
Hi Peter, Am 31.05.22 um 17:47 schrieb Peter Robinson: > On Sun, May 15, 2022 at 9:21 PM Stefan Wahren <stefan.wahren@i2se.com> wrote: >> The macros in order to access the ASB registers have a hard coded base >> address. So extending them for other platforms would make them harder >> to read. As a solution resolve these macros. >> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > A minor query below: > Reviewed-by: Peter Robinson <pbrobinson@gmail.com> >> --- >> drivers/soc/bcm/bcm2835-power.c | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c >> index 77dc9e62b207..fa0a13035794 100644 >> --- a/drivers/soc/bcm/bcm2835-power.c >> +++ b/drivers/soc/bcm/bcm2835-power.c >> @@ -126,9 +126,6 @@ >> >> #define ASB_AXI_BRDG_ID 0x20 >> >> -#define ASB_READ(reg) readl(power->asb + (reg)) >> -#define ASB_WRITE(reg, val) writel(PM_PASSWORD | (val), power->asb + (reg)) >> - >> struct bcm2835_power_domain { >> struct generic_pm_domain base; >> struct bcm2835_power *power; >> @@ -150,7 +147,10 @@ struct bcm2835_power { >> >> static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable) >> { >> + void __iomem *base = power->asb; > I wonder the perf of defining base here vs the readability of > power->asb; throughout. this is the whole "trick" in order to change the base to rpivid_asb in patch #10. > >> u64 start; >> + u32 val; >> + >> >> if (!reg) >> return 0; >> @@ -159,12 +159,13 @@ static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable >> >> /* Enable the module's async AXI bridges. */ >> if (enable) { >> - ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP); >> + val = readl(base + reg) & ~ASB_REQ_STOP; >> } else { >> - ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP); >> + val = readl(base + reg) | ASB_REQ_STOP; >> } >> + writel(PM_PASSWORD | val, base + reg); >> >> - while (ASB_READ(reg) & ASB_ACK) { >> + while (readl(base + reg) & ASB_ACK) { >> cpu_relax(); >> if (ktime_get_ns() - start >= 1000) >> return -ETIMEDOUT; >> @@ -622,7 +623,7 @@ static int bcm2835_power_probe(struct platform_device *pdev) >> power->base = pm->base; >> power->asb = pm->asb; >> >> - id = ASB_READ(ASB_AXI_BRDG_ID); >> + id = readl(power->asb + ASB_AXI_BRDG_ID); >> if (id != 0x62726467 /* "BRDG" */) { >> dev_err(dev, "ASB register ID returned 0x%08x\n", id); >> return -ENODEV; >> -- >> 2.25.1 >>
diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c index 77dc9e62b207..fa0a13035794 100644 --- a/drivers/soc/bcm/bcm2835-power.c +++ b/drivers/soc/bcm/bcm2835-power.c @@ -126,9 +126,6 @@ #define ASB_AXI_BRDG_ID 0x20 -#define ASB_READ(reg) readl(power->asb + (reg)) -#define ASB_WRITE(reg, val) writel(PM_PASSWORD | (val), power->asb + (reg)) - struct bcm2835_power_domain { struct generic_pm_domain base; struct bcm2835_power *power; @@ -150,7 +147,10 @@ struct bcm2835_power { static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable) { + void __iomem *base = power->asb; u64 start; + u32 val; + if (!reg) return 0; @@ -159,12 +159,13 @@ static int bcm2835_asb_control(struct bcm2835_power *power, u32 reg, bool enable /* Enable the module's async AXI bridges. */ if (enable) { - ASB_WRITE(reg, ASB_READ(reg) & ~ASB_REQ_STOP); + val = readl(base + reg) & ~ASB_REQ_STOP; } else { - ASB_WRITE(reg, ASB_READ(reg) | ASB_REQ_STOP); + val = readl(base + reg) | ASB_REQ_STOP; } + writel(PM_PASSWORD | val, base + reg); - while (ASB_READ(reg) & ASB_ACK) { + while (readl(base + reg) & ASB_ACK) { cpu_relax(); if (ktime_get_ns() - start >= 1000) return -ETIMEDOUT; @@ -622,7 +623,7 @@ static int bcm2835_power_probe(struct platform_device *pdev) power->base = pm->base; power->asb = pm->asb; - id = ASB_READ(ASB_AXI_BRDG_ID); + id = readl(power->asb + ASB_AXI_BRDG_ID); if (id != 0x62726467 /* "BRDG" */) { dev_err(dev, "ASB register ID returned 0x%08x\n", id); return -ENODEV;
The macros in order to access the ASB registers have a hard coded base address. So extending them for other platforms would make them harder to read. As a solution resolve these macros. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- drivers/soc/bcm/bcm2835-power.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)