Re: [PATCH v2 06/10] ARM: EXYNOS: Add support for mapping PMU base address via DT
From: Pankaj Dubey
Date: Sun Apr 27 2014 - 03:12:00 EST
Hi Tomasz,
Thanks for review.
On 04/26/2014 06:43 AM, Tomasz Figa wrote:
Hi,
On 25.04.2014 14:32, Pankaj Dubey wrote:
From: Young-Gun Jang <yg1004.jang@xxxxxxxxxxx>
Add support for mapping Samsung Power Management Unit (PMU) base address
from device tree. Code will use existing samsung pmu binding information.
This patch also adds two helper functions as "get_exynos_pmuregmap" and
"get_exynos_pmuaddr".
"get_exynos_pmuregmap" returns a regmap based PMU register handle where as
"get_exynos_pmuaddr" returns ioremap virtual address.
Signed-off-by: Young-Gun Jang <yg1004.jang@xxxxxxxxxxx>
Signed-off-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
---
arch/arm/mach-exynos/Kconfig | 2 ++
arch/arm/mach-exynos/common.h | 3 ++
arch/arm/mach-exynos/exynos.c | 78 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index fc8bf18..2f60c90 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -26,6 +26,7 @@ config ARCH_EXYNOS4
select PINCTRL
select PM_GENERIC_DOMAINS if PM_RUNTIME
select S5P_DEV_MFC
+ select MFD_SYSCON
help
Samsung EXYNOS4 SoCs based systems
@@ -36,6 +37,7 @@ config ARCH_EXYNOS5
select HAVE_ARM_SCU if SMP
select HAVE_SMP
select PINCTRL
+ select MFD_SYSCON
help
Samsung EXYNOS5 (Cortex-A15) SoC based systems
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 31c5964..ecfd0fc 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -57,4 +57,7 @@ struct exynos_pmu_conf {
extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
+extern struct regmap *get_exynos_pmuregmap(void);
+extern void __iomem *get_exynos_pmuaddr(void);
Do you really need these functions? Couldn't all the drivers using PMU simply
call syscon_regmap_lookup_by_phandle() or of_iomap() directly?
Well, currently "get_exynos_pmuregmap" is used in three location under
mach-exynos as "exynos.c", "pm.c" and "hotplug.c" so just to avoid duplicate code
in all these files I added "get_exynos_pmuregmap" as helper function in "exynos.c".
So in my opinion it should be fine. What do you say?
Regarding "get_exynos_pmuaddr", only user of this function is "platsmp.c" so I can
move this function in platsmp.c and remove this helper. This will also solve the
other
issue of early mapping as you raised below.
+
#endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index d6f405f..68f60e1 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -19,6 +19,7 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
+#include <linux/mfd/syscon.h>
#include <asm/cacheflush.h>
#include <asm/hardware/cache-l2x0.h>
@@ -36,6 +37,9 @@
#define L2_AUX_VAL 0x7C470001
#define L2_AUX_MASK 0xC200ffff
+static struct regmap *exynos_pmu_regmap;
+static void __iomem *exynos_pmu_base;
+
static struct map_desc exynos4_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C_VA_SYS,
@@ -269,6 +273,46 @@ static int __init exynos_fdt_map_chipid(unsigned long
node, const char *uname,
return 1;
}
+static const struct of_device_id exynos_dt_pmu_match[] = {
+ { .compatible = "samsung,exynos4210-pmu" },
+ { .compatible = "samsung,exynos4212-pmu" },
+ { .compatible = "samsung,exynos4412-pmu" },
+ { .compatible = "samsung,exynos5250-pmu" },
+ {},
+};
+
+static int __init exynos_fdt_map_pmu(unsigned long node,
+ const char *uname, int depth, void *data)
+{
+ struct map_desc iodesc;
+ __be32 *reg;
+ unsigned long len;
+ phys_addr_t phys_addr;
+ const struct of_device_id *matches = exynos_dt_pmu_match;
+
+ for (; matches->compatible[0]; matches++) {
+ if (!of_flat_dt_is_compatible(node, matches->compatible))
+ continue;
+ reg = of_get_flat_dt_prop(node, "reg", &len);
+ if (reg == NULL || len != (sizeof(unsigned long) * 2))
+ return 0;
+
+ phys_addr = be32_to_cpu(reg[0]);
+ iodesc.pfn = __phys_to_pfn(phys_addr);
+ iodesc.length = be32_to_cpu(reg[1]) - 1;
+ iodesc.virtual = (unsigned long)S5P_VA_PMU;
+ iodesc.type = MT_DEVICE;
+ iotable_init(&iodesc, 1);
+
+ exynos_pmu_base = ioremap(phys_addr, be32_to_cpu(reg[1]));
+ if (WARN_ON(!exynos_pmu_base))
+ return -EFAULT;
+ return 1;
+ }
+
+ return 0;
+}
Is such early mapping really needed? Couldn't the code using PMU be deferred
to the stage that memory management is available and then of_iomap() used
directly?
OK, not really required, If I move mapping of PMU base address to platsmp.c.
As "exynos_pmu_base" is only required in platsmp.c during "exynos_smp_prepare_cpus"
function call, I can move mapping of PMU address in platsmp.c itself and there I
can use of_iomap.
+
/*
* exynos_map_io
*
@@ -302,6 +346,7 @@ static void __init exynos_init_io(void)
debug_ll_io_init();
of_scan_flat_dt(exynos_fdt_map_chipid, NULL);
+ of_scan_flat_dt(exynos_fdt_map_pmu, NULL);
/* detect cpu id and rev. */
s5p_init_cpu(S5P_VA_CHIPID);
@@ -336,6 +381,38 @@ static int __init exynos4_l2x0_cache_init(void)
}
early_initcall(exynos4_l2x0_cache_init);
+
+inline struct regmap *get_exynos_pmuregmap()
+{
+ return exynos_pmu_regmap;
+}
+
+inline void __iomem *get_exynos_pmuaddr()
+{
+ return exynos_pmu_base;
+}
Hmm, non-static inline inside a C file? Probably should be either static
inline or non-static non-inline. (Assuming that both are really needed, as I
pointed in the comments above.)
OK, will remove "get_exynos_pmuaddr" and make "get_exynos_pmuregmap" as
non-static non-inline.
+
+
+void __init exynos_map_pmu(void)
+{
+ struct device_node *np = NULL;
+
+ early_syscon_init();
+
+ np = of_find_matching_node(NULL, exynos_dt_pmu_match);
+
+ if (!np) {
+ pr_err("Failed to find PMU node\n");
+ return;
+ } else {
+ exynos_pmu_regmap = syscon_early_regmap_lookup_by_phandle(np,
+ "samsung,syscon-phandle");
Do you need this regmap really here? I believe it should be the code using PMU
registers which calls this function directly to retrieve a handle to the syscon.
Yes, as "exynos_restart" is accessing one PMU register. So even though all callers
(pm.c and hotplug.c) directly retrieve a regmap handle to PMU, we need regmap handle
in this file also. So as I mentioned above just to avoid duplicate code in all
files we can
retrieve regmap handle in "exynos.c" and using helper function other files can
use it.
Best regards,
Tomasz
--
Best Regards,
Pankaj Dubey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/