| Message ID | 1402500612-4778-2-git-send-email-t.figa@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h > index 060a75e..ddaebcd 100644 > --- a/arch/arm/include/asm/mach/arch.h > +++ b/arch/arm/include/asm/mach/arch.h > @@ -46,7 +46,8 @@ struct machine_desc { > enum reboot_mode reboot_mode; /* default restart mode */ > unsigned l2c_aux_val; /* L2 cache aux value */ > unsigned l2c_aux_mask; /* L2 cache aux mask */ > - void (*l2c_write_sec)(unsigned long, unsigned); > + void (*l2c_write_sec)(void __iomem *, > + unsigned long, unsigned); > struct smp_operations *smp; /* SMP operations */ > bool (*smp_init)(void); > void (*fixup)(struct tag *, char **); > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c > index efc5cab..1695eab 100644 > --- a/arch/arm/mm/cache-l2x0.c > +++ b/arch/arm/mm/cache-l2x0.c > @@ -72,7 +72,7 @@ static void l2c_write_sec(unsigned long val, void __iomem *base, unsigned reg) > if (val == readl_relaxed(base + reg)) > return; > if (outer_cache.write_sec) > - outer_cache.write_sec(val, reg); > + outer_cache.write_sec(base, val, reg); > else > writel_relaxed(val, base + reg); > } The parameter order (base, val, reg) seems very non-intuitive. Are you matching some existing prototype or adhering to some backwards compatibility issue? If not wouldn't, say, (base, reg, val) or (val, base, reg) be more intuitive? Thanks, jdl
On 11.06.2014 18:00, Jon Loeliger wrote: >> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h >> index 060a75e..ddaebcd 100644 >> --- a/arch/arm/include/asm/mach/arch.h >> +++ b/arch/arm/include/asm/mach/arch.h >> @@ -46,7 +46,8 @@ struct machine_desc { >> enum reboot_mode reboot_mode; /* default restart mode */ >> unsigned l2c_aux_val; /* L2 cache aux value */ >> unsigned l2c_aux_mask; /* L2 cache aux mask */ >> - void (*l2c_write_sec)(unsigned long, unsigned); >> + void (*l2c_write_sec)(void __iomem *, >> + unsigned long, unsigned); >> struct smp_operations *smp; /* SMP operations */ >> bool (*smp_init)(void); >> void (*fixup)(struct tag *, char **); > >> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c >> index efc5cab..1695eab 100644 >> --- a/arch/arm/mm/cache-l2x0.c >> +++ b/arch/arm/mm/cache-l2x0.c >> @@ -72,7 +72,7 @@ static void l2c_write_sec(unsigned long val, void __iomem *base, unsigned reg) >> if (val == readl_relaxed(base + reg)) >> return; >> if (outer_cache.write_sec) >> - outer_cache.write_sec(val, reg); >> + outer_cache.write_sec(base, val, reg); >> else >> writel_relaxed(val, base + reg); >> } > > The parameter order (base, val, reg) seems very non-intuitive. > Are you matching some existing prototype or adhering to some > backwards compatibility issue? If not wouldn't, say, (base, reg, val) > or (val, base, reg) be more intuitive? Hmm, I didn't think too much about this, so this order is just whatever first came to my mind, probably because I'm used to xxx_write(ctx, val, reg) accessors found in many drivers. Anyway, l2c_write_sec() in arm/mm/cache-l2x0.c, which calls outer_cache.write_sec(), already uses (val, base, reg) convention, so probably this one would be most suitable. I'll change in v2. Best regards, Tomasz
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h index 060a75e..ddaebcd 100644 --- a/arch/arm/include/asm/mach/arch.h +++ b/arch/arm/include/asm/mach/arch.h @@ -46,7 +46,8 @@ struct machine_desc { enum reboot_mode reboot_mode; /* default restart mode */ unsigned l2c_aux_val; /* L2 cache aux value */ unsigned l2c_aux_mask; /* L2 cache aux mask */ - void (*l2c_write_sec)(unsigned long, unsigned); + void (*l2c_write_sec)(void __iomem *, + unsigned long, unsigned); struct smp_operations *smp; /* SMP operations */ bool (*smp_init)(void); void (*fixup)(struct tag *, char **); diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/outercache.h index 891a56b..5cc703b 100644 --- a/arch/arm/include/asm/outercache.h +++ b/arch/arm/include/asm/outercache.h @@ -35,7 +35,7 @@ struct outer_cache_fns { void (*resume)(void); /* This is an ARM L2C thing */ - void (*write_sec)(unsigned long, unsigned); + void (*write_sec)(void __iomem *, unsigned long, unsigned); }; extern struct outer_cache_fns outer_cache; diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c index 8c35ae4..2bd3243 100644 --- a/arch/arm/mach-highbank/highbank.c +++ b/arch/arm/mach-highbank/highbank.c @@ -51,7 +51,8 @@ static void __init highbank_scu_map_io(void) } -static void highbank_l2c310_write_sec(unsigned long val, unsigned reg) +static void highbank_l2c310_write_sec(void __iomem *base, + unsigned long val, unsigned reg) { if (reg == L2X0_CTRL) highbank_smc1(0x102, val); diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c index 326cd98..bdbe658 100644 --- a/arch/arm/mach-omap2/omap4-common.c +++ b/arch/arm/mach-omap2/omap4-common.c @@ -167,7 +167,8 @@ void __iomem *omap4_get_l2cache_base(void) return l2cache_base; } -static void omap4_l2c310_write_sec(unsigned long val, unsigned reg) +static void omap4_l2c310_write_sec(void __iomem *base, + unsigned long val, unsigned reg) { unsigned smc_op; diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c index 842ebed..35c2623 100644 --- a/arch/arm/mach-ux500/cache-l2x0.c +++ b/arch/arm/mach-ux500/cache-l2x0.c @@ -35,7 +35,8 @@ static int __init ux500_l2x0_unlock(void) return 0; } -static void ux500_l2c310_write_sec(unsigned long val, unsigned reg) +static void ux500_l2c310_write_sec(void __iomem *base, + unsigned long val, unsigned reg) { /* * We can't write to secure registers as we are in non-secure diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index efc5cab..1695eab 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -72,7 +72,7 @@ static void l2c_write_sec(unsigned long val, void __iomem *base, unsigned reg) if (val == readl_relaxed(base + reg)) return; if (outer_cache.write_sec) - outer_cache.write_sec(val, reg); + outer_cache.write_sec(base, val, reg); else writel_relaxed(val, base + reg); }
For certain platforms (e.g. Exynos) it is necessary to read back some values from registers before they can be written (i.e. SMC calls that set multiple registers per call), so base address of L2C controller is needed for .write_sec operation. This patch adds base argument to .write_sec callback so that its implementation can also access registers directly. Signed-off-by: Tomasz Figa <t.figa@samsung.com> --- arch/arm/include/asm/mach/arch.h | 3 ++- arch/arm/include/asm/outercache.h | 2 +- arch/arm/mach-highbank/highbank.c | 3 ++- arch/arm/mach-omap2/omap4-common.c | 3 ++- arch/arm/mach-ux500/cache-l2x0.c | 3 ++- arch/arm/mm/cache-l2x0.c | 2 +- 6 files changed, 10 insertions(+), 6 deletions(-)