Hi Pankaj,

[Dropped yg1004.j...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to