Messages in this thread | | | Date | Tue, 17 Jun 2014 18:02:06 +0200 | From | Tomasz Figa <> | Subject | Re: [PATCH v4 08/11] ARM: EXYNOS: Refactored code for using PMU address via DT |
| |
Hi Pankaj,
[Dropped yg1004.jang@samsung.com from CC, as apparently his mailbox is "temporarily locked", at least from the point of view of my mail server.]
Please see my comments inline.
On 10.05.2014 08:56, Pankaj Dubey wrote: > Under "arm/mach-exynos" many files are using PMU register offsets. > Since we have added support for accessing PMU base address via DT, > now we can remove PMU mapping from exynosX_iodesc. Let's convert > all these access using either of iomapped address or regmap handle. > This will help us in removing static mapping of PMU base address > as well as help in reducing dependency over machine header files. > Thus helping for migration of PMU implementation from machine to > driver folder which can be reused for ARM64 bsed SoC.
[snip]
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c > index 3b1d245..59eb1f1 100644 > --- a/arch/arm/mach-exynos/exynos.c > +++ b/arch/arm/mach-exynos/exynos.c
[snip]
> @@ -156,7 +147,7 @@ static void exynos_restart(enum reboot_mode mode, const char *cmd) > { > struct device_node *np; > u32 val = 0x1; > - void __iomem *addr = EXYNOS_SWRESET; > + void __iomem *addr = NULL;
This will probably change into addr = exynos_pmu_base + EXYNOS_SWRESET.
> > if (of_machine_is_compatible("samsung,exynos5440")) { > u32 status; > @@ -169,9 +160,9 @@ static void exynos_restart(enum reboot_mode mode, const char *cmd) > val = __raw_readl(addr); > > val = (val & 0xffff0000) | (status & 0xffff); > - } > - > - __raw_writel(val, addr); > + __raw_writel(val, addr); > + } else > + regmap_write(exynos_pmu_regmap, EXYNOS_SWRESET, val); > } > > static struct platform_device exynos_cpuidle = { > diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c > index 73b0b5c..0243ef3 100644 > --- a/arch/arm/mach-exynos/hotplug.c > +++ b/arch/arm/mach-exynos/hotplug.c > @@ -13,6 +13,7 @@ > #include <linux/errno.h> > #include <linux/smp.h> > #include <linux/io.h> > +#include <linux/regmap.h> > > #include <asm/cacheflush.h> > #include <asm/cp15.h> > @@ -91,11 +92,12 @@ static inline void cpu_leave_lowpower(void) > > static inline void platform_do_lowpower(unsigned int cpu, int *spurious) > { > + struct regmap *pmu_regmap = get_exynos_pmuregmap(); > for (;;) { > > /* make cpu1 to be turned off at next WFI command */ > if (cpu == 1) > - __raw_writel(0, S5P_ARM_CORE1_CONFIGURATION); > + regmap_write(pmu_regmap, S5P_ARM_CORE1_CONFIGURATION, 0);
This change should disappear.
[snip]
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c > index 1f63596..33eb364 100644 > --- a/arch/arm/mach-exynos/platsmp.c > +++ b/arch/arm/mach-exynos/platsmp.c
[snip]
> +static void __iomem *pmu_base_addr(void) > +{ > + struct device_node *np = NULL; > + if (!pmu_base) { > + np = of_find_matching_node(NULL, exynos_dt_pmu_match); > + > + if (!np) > + panic("%s, failed to find PMU node\n", __func__); > + > + pmu_base = of_iomap(np, 0); > + > + if (!pmu_base) > + panic("%s: failed to map registers\n", __func__); > + } > + return pmu_base; > +}
As we discussed before, PMU mapping for arch code should be handled in the same way as SYSRAM.
> static DEFINE_SPINLOCK(boot_lock); > > static void exynos_secondary_init(unsigned int cpu) > @@ -132,14 +158,14 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle) > */ > write_pen_release(phys_cpu); > > - if (!(__raw_readl(S5P_ARM_CORE1_STATUS) & S5P_CORE_LOCAL_PWR_EN)) { > + if (!(__raw_readl(pmu_base + S5P_ARM_CORE1_STATUS) > + & S5P_CORE_LOCAL_PWR_EN)) {
Creating a variable to save register value to would probably make the code more readable.
> __raw_writel(S5P_CORE_LOCAL_PWR_EN, > - S5P_ARM_CORE1_CONFIGURATION); > - > + pmu_base + S5P_ARM_CORE1_CONFIGURATION); > timeout = 10; > > /* wait max 10 ms until cpu1 is on */ > - while ((__raw_readl(S5P_ARM_CORE1_STATUS) > + while ((__raw_readl(pmu_base + S5P_ARM_CORE1_STATUS) > & S5P_CORE_LOCAL_PWR_EN) != S5P_CORE_LOCAL_PWR_EN) {
Ditto.
> if (timeout-- == 0) > break; > @@ -238,6 +264,8 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus) > { > int i; > > + pmu_base = pmu_base_addr(); > + > if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) > scu_enable(scu_base_addr()); > > diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c > index f445c49..ee427d7 100644 > --- a/arch/arm/mach-exynos/pm.c > +++ b/arch/arm/mach-exynos/pm.c > @@ -21,6 +21,7 @@ > #include <linux/irqchip/arm-gic.h> > #include <linux/err.h> > #include <linux/clk.h> > +#include <linux/regmap.h> > > #include <asm/cacheflush.h> > #include <asm/hardware/cache-l2x0.h> > @@ -38,6 +39,8 @@ > #include "regs-pmu.h" > #include "regs-sys.h" > > +static struct regmap *pmu_regmap; > +
This and other regmap related changes in this file won't be needed anymore.
> /** > * struct exynos_wkup_irq - Exynos GIC to PMU IRQ mapping > * @hwirq: Hardware IRQ signal of the GIC > @@ -104,10 +107,10 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
[snip]
> diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h > index fd8a19d..a72b1bc 100644 > --- a/arch/arm/mach-exynos/regs-pmu.h > +++ b/arch/arm/mach-exynos/regs-pmu.h
[snip]
> > -#define S5P_WAKEUP_STAT S5P_PMUREG(0x0600) > -#define S5P_EINT_WAKEUP_MASK S5P_PMUREG(0x0604) > -#define S5P_WAKEUP_MASK S5P_PMUREG(0x0608) > +#define S5P_WAKEUP_STAT 0x0600 > +#define S5P_EINT_WAKEUP_MASK 0x0604 > +#define S5P_WAKEUP_MASK 0x0608
Is it just my mail client or indentation is inconsistent here?
Best regards, Tomasz
| |