On Sun, May 15, 2022 at 9:21 PM Stefan Wahren <stefan.wahren@xxxxxxxx> 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@xxxxxxxx>
A minor query below:
Reviewed-by: Peter Robinson <pbrobinson@xxxxxxxxx>
> ---
> 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
>
