All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-03  6:11 ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-arm-kernel
  Cc: kgene.kim, linux, t.figa, Pankaj Dubey

This patch series attempts to get rid of soc_is_exynosXXXX macros
and eventually with the help of this series we can probably get
rid of CONFIG_SOC_EXYNOSXXXX in near future.
Each Exynos SoC has ChipID block which can give information about
SoC's product Id and revision number. Currently we have single
DT binding information for this as "samsung,exynos4210-chipid".
But Exynos4 and Exynos5 SoC series have one small difference in
chip Id, with resepect to product id bit-masks. So it means we
should have separate compatible string for these different series
of SoCs. So I have created new binding information for handling
this difference. Also currently I can think of putting this driver
code under "drivers/misc/" but suggestions are welcome.
Also current form of driver is missing platfrom driver and needs
init function to be called from machine file (either exynos.c or
platsmp.c). I hope lot of suggestions and comments to improve this
further.

This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
and prepared on top of following patch series and it's dependent
patch series.

[1]: Map SYSRAM through generic SRAM bindings.
	http://www.spinics.net/lists/arm-kernel/msg327677.html
[2]: Exynos PMU cleanup and refactoring.
	https://lkml.org/lkml/2014/4/30/44

Pankaj Dubey (4):
  ARM: EXYNOS: remove soc_is_exynos4/5 from exynos.c
  ARM:  EXYNOS: remove unused header inclusion from hotplug.c
  misc: exynos-chipid: Add Exynos Chipid driver support
  ARM: EXYNOS: Refactoring to remove soc_is_exynosXXXX macros from
    exynos

 .../bindings/arm/samsung/exynos-chipid.txt         |   15 ++++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/exynos4.dtsi                     |    2 +-
 arch/arm/boot/dts/exynos5.dtsi                     |    2 +-
 arch/arm/mach-exynos/exynos.c                      |   66 ++++++++--------
 arch/arm/mach-exynos/hotplug.c                     |    2 -
 arch/arm/mach-exynos/platsmp.c                     |   10 ++-
 arch/arm/mach-exynos/pm.c                          |   28 +++----
 arch/arm/plat-samsung/include/plat/cpu.h           |   60 --------------
 drivers/clk/samsung/clk-exynos4.c                  |    2 +-
 drivers/cpufreq/exynos-cpufreq.c                   |    9 +--
 drivers/cpufreq/exynos-cpufreq.h                   |    1 -
 drivers/cpufreq/exynos4x12-cpufreq.c               |    5 +-
 drivers/misc/Kconfig                               |    7 ++
 drivers/misc/Makefile                              |    1 +
 drivers/misc/exynos-chipid.c                       |   83 ++++++++++++++++++++
 include/linux/exynos-soc.h                         |   46 +++++++++++
 17 files changed, 215 insertions(+), 125 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt
 create mode 100644 drivers/misc/exynos-chipid.c
 create mode 100644 include/linux/exynos-soc.h

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-03  6:11 ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-arm-kernel

This patch series attempts to get rid of soc_is_exynosXXXX macros
and eventually with the help of this series we can probably get
rid of CONFIG_SOC_EXYNOSXXXX in near future.
Each Exynos SoC has ChipID block which can give information about
SoC's product Id and revision number. Currently we have single
DT binding information for this as "samsung,exynos4210-chipid".
But Exynos4 and Exynos5 SoC series have one small difference in
chip Id, with resepect to product id bit-masks. So it means we
should have separate compatible string for these different series
of SoCs. So I have created new binding information for handling
this difference. Also currently I can think of putting this driver
code under "drivers/misc/" but suggestions are welcome.
Also current form of driver is missing platfrom driver and needs
init function to be called from machine file (either exynos.c or
platsmp.c). I hope lot of suggestions and comments to improve this
further.

This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
and prepared on top of following patch series and it's dependent
patch series.

[1]: Map SYSRAM through generic SRAM bindings.
	http://www.spinics.net/lists/arm-kernel/msg327677.html
[2]: Exynos PMU cleanup and refactoring.
	https://lkml.org/lkml/2014/4/30/44

Pankaj Dubey (4):
  ARM: EXYNOS: remove soc_is_exynos4/5 from exynos.c
  ARM:  EXYNOS: remove unused header inclusion from hotplug.c
  misc: exynos-chipid: Add Exynos Chipid driver support
  ARM: EXYNOS: Refactoring to remove soc_is_exynosXXXX macros from
    exynos

 .../bindings/arm/samsung/exynos-chipid.txt         |   15 ++++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/exynos4.dtsi                     |    2 +-
 arch/arm/boot/dts/exynos5.dtsi                     |    2 +-
 arch/arm/mach-exynos/exynos.c                      |   66 ++++++++--------
 arch/arm/mach-exynos/hotplug.c                     |    2 -
 arch/arm/mach-exynos/platsmp.c                     |   10 ++-
 arch/arm/mach-exynos/pm.c                          |   28 +++----
 arch/arm/plat-samsung/include/plat/cpu.h           |   60 --------------
 drivers/clk/samsung/clk-exynos4.c                  |    2 +-
 drivers/cpufreq/exynos-cpufreq.c                   |    9 +--
 drivers/cpufreq/exynos-cpufreq.h                   |    1 -
 drivers/cpufreq/exynos4x12-cpufreq.c               |    5 +-
 drivers/misc/Kconfig                               |    7 ++
 drivers/misc/Makefile                              |    1 +
 drivers/misc/exynos-chipid.c                       |   83 ++++++++++++++++++++
 include/linux/exynos-soc.h                         |   46 +++++++++++
 17 files changed, 215 insertions(+), 125 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt
 create mode 100644 drivers/misc/exynos-chipid.c
 create mode 100644 include/linux/exynos-soc.h

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/4] ARM: EXYNOS: remove soc_is_exynos4/5 from exynos.c
  2014-05-03  6:11 ` Pankaj Dubey
@ 2014-05-03  6:11   ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-arm-kernel
  Cc: kgene.kim, linux, t.figa, Pankaj Dubey, Heiko Stuebner, Thomas Abraham

This patch adds support for checking soc compatibility based on
compatibility match. It will help us in removing soc_is_exynos4
and soc_is_exynos5 function usage and definition.

CC: Russell King <linux@arm.linux.org.uk>
CC: Heiko Stuebner <heiko@sntech.de>
CC: Thomas Abraham <thomas.abraham@linaro.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/exynos.c            |   30 +++++++++++++++++++++++++++---
 arch/arm/plat-samsung/include/plat/cpu.h |    3 ---
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 59eb1f1..93ae076 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -212,6 +212,30 @@ static const struct of_device_id exynos_dt_pmu_match[] = {
 	{},
 };
 
+static const struct of_device_id exynos5_device_ids[] = {
+	{ .compatible = "samsung,exynos5250", },
+	{ .compatible = "samsung,exynos5420", },
+	{},
+};
+
+static const struct of_device_id exynos4_device_ids[] = {
+	{ .compatible = "samsung,exynos4210", },
+	{ .compatible = "samsung,exynos4212", },
+	{ .compatible = "samsung,exynos4412", },
+	{},
+};
+
+static inline bool soc_is_compatible(const struct of_device_id *device_ids)
+{
+	unsigned long root = of_get_flat_dt_root();
+	const struct of_device_id *matches = device_ids;
+	for (; matches->compatible[0]; matches++) {
+		if (of_flat_dt_is_compatible(root, matches->compatible))
+			return true;
+	}
+	return false;
+}
+
 /*
  * exynos_map_io
  *
@@ -219,10 +243,10 @@ static const struct of_device_id exynos_dt_pmu_match[] = {
  */
 static void __init exynos_map_io(void)
 {
-	if (soc_is_exynos4())
+	if (soc_is_compatible(exynos4_device_ids))
 		iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
 
-	if (soc_is_exynos5())
+	if (soc_is_compatible(exynos5_device_ids))
 		iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos5_iodesc));
 }
 
@@ -306,7 +330,7 @@ static void __init exynos_dt_machine_init(void)
 	 * are available then re-configure the interrupts via the
 	 * system register.
 	 */
-	if (soc_is_exynos5()) {
+	if (soc_is_compatible(exynos5_device_ids)) {
 		for_each_compatible_node(i2c_np, NULL, i2c_compat) {
 			if (of_device_is_available(i2c_np)) {
 				id = of_alias_get_id(i2c_np, "i2c");
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
index 5992b8d..18a9a00 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -166,9 +166,6 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 # define soc_is_exynos5440()	0
 #endif
 
-#define soc_is_exynos4() (soc_is_exynos4210() || soc_is_exynos4212() || \
-			  soc_is_exynos4412())
-#define soc_is_exynos5() (soc_is_exynos5250() || soc_is_exynos5420())
 
 #define IODESC_ENT(x) { (unsigned long)S3C24XX_VA_##x, __phys_to_pfn(S3C24XX_PA_##x), S3C24XX_SZ_##x, MT_DEVICE }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 1/4] ARM: EXYNOS: remove soc_is_exynos4/5 from exynos.c
@ 2014-05-03  6:11   ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds support for checking soc compatibility based on
compatibility match. It will help us in removing soc_is_exynos4
and soc_is_exynos5 function usage and definition.

CC: Russell King <linux@arm.linux.org.uk>
CC: Heiko Stuebner <heiko@sntech.de>
CC: Thomas Abraham <thomas.abraham@linaro.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/exynos.c            |   30 +++++++++++++++++++++++++++---
 arch/arm/plat-samsung/include/plat/cpu.h |    3 ---
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 59eb1f1..93ae076 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -212,6 +212,30 @@ static const struct of_device_id exynos_dt_pmu_match[] = {
 	{},
 };
 
+static const struct of_device_id exynos5_device_ids[] = {
+	{ .compatible = "samsung,exynos5250", },
+	{ .compatible = "samsung,exynos5420", },
+	{},
+};
+
+static const struct of_device_id exynos4_device_ids[] = {
+	{ .compatible = "samsung,exynos4210", },
+	{ .compatible = "samsung,exynos4212", },
+	{ .compatible = "samsung,exynos4412", },
+	{},
+};
+
+static inline bool soc_is_compatible(const struct of_device_id *device_ids)
+{
+	unsigned long root = of_get_flat_dt_root();
+	const struct of_device_id *matches = device_ids;
+	for (; matches->compatible[0]; matches++) {
+		if (of_flat_dt_is_compatible(root, matches->compatible))
+			return true;
+	}
+	return false;
+}
+
 /*
  * exynos_map_io
  *
@@ -219,10 +243,10 @@ static const struct of_device_id exynos_dt_pmu_match[] = {
  */
 static void __init exynos_map_io(void)
 {
-	if (soc_is_exynos4())
+	if (soc_is_compatible(exynos4_device_ids))
 		iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
 
-	if (soc_is_exynos5())
+	if (soc_is_compatible(exynos5_device_ids))
 		iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos5_iodesc));
 }
 
@@ -306,7 +330,7 @@ static void __init exynos_dt_machine_init(void)
 	 * are available then re-configure the interrupts via the
 	 * system register.
 	 */
-	if (soc_is_exynos5()) {
+	if (soc_is_compatible(exynos5_device_ids)) {
 		for_each_compatible_node(i2c_np, NULL, i2c_compat) {
 			if (of_device_is_available(i2c_np)) {
 				id = of_alias_get_id(i2c_np, "i2c");
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
index 5992b8d..18a9a00 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -166,9 +166,6 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 # define soc_is_exynos5440()	0
 #endif
 
-#define soc_is_exynos4() (soc_is_exynos4210() || soc_is_exynos4212() || \
-			  soc_is_exynos4412())
-#define soc_is_exynos5() (soc_is_exynos5250() || soc_is_exynos5420())
 
 #define IODESC_ENT(x) { (unsigned long)S3C24XX_VA_##x, __phys_to_pfn(S3C24XX_PA_##x), S3C24XX_SZ_##x, MT_DEVICE }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/4] ARM:  EXYNOS: remove unused header inclusion from hotplug.c
  2014-05-03  6:11 ` Pankaj Dubey
@ 2014-05-03  6:11   ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-arm-kernel
  Cc: kgene.kim, linux, t.figa, Pankaj Dubey

This patch removed "plat/cpu.h" inclusion from hotplug.c as it
is not required.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/hotplug.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 0243ef3..5e19601 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -19,8 +19,6 @@
 #include <asm/cp15.h>
 #include <asm/smp_plat.h>
 
-#include <plat/cpu.h>
-
 #include "common.h"
 #include "regs-pmu.h"
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/4] ARM: EXYNOS: remove unused header inclusion from hotplug.c
@ 2014-05-03  6:11   ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-arm-kernel

This patch removed "plat/cpu.h" inclusion from hotplug.c as it
is not required.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/mach-exynos/hotplug.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 0243ef3..5e19601 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -19,8 +19,6 @@
 #include <asm/cp15.h>
 #include <asm/smp_plat.h>
 
-#include <plat/cpu.h>
-
 #include "common.h"
 #include "regs-pmu.h"
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support
  2014-05-03  6:11 ` Pankaj Dubey
@ 2014-05-03  6:11   ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-arm-kernel
  Cc: kgene.kim, linux, t.figa, Pankaj Dubey, Arnd Bergmann,
	Greg Kroah-Hartman

Exynos SoCs have Chipid IP, for identification of product IDs
and SoC revistions. Till now we are using static macros
such as soc_is_exynosxxxx and #ifdefs for run time identification
of SoCs and their revisions. This is leading to add new Kconfig,
soc_is_exynosXXXX definitions each time new SoC support is getting
added. So this driver intends to provide initialization code
all these functionalites and thus helping in removing macros.

CC: Arnd Bergmann <arnd@arndb.de>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 drivers/misc/Kconfig         |    7 ++++
 drivers/misc/Makefile        |    1 +
 drivers/misc/exynos-chipid.c |   83 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/exynos-soc.h   |   46 +++++++++++++++++++++++
 4 files changed, 137 insertions(+)
 create mode 100644 drivers/misc/exynos-chipid.c
 create mode 100644 include/linux/exynos-soc.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1cb7408..f313bd3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -515,6 +515,13 @@ config SRAM
 	  the genalloc API. It is supposed to be used for small on-chip SRAM
 	  areas found on many SoCs.
 
+config EXYNOS_CHIPID
+	tristate "Support Exynos CHIPID"
+	default y
+	depends on ARCH_EXYNOS || ARM64
+	help
+	  If you say Y here you get support for the Exynos CHIP id.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7eb4b69..48c8fb5 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)		+= sram.o
 obj-y				+= mic/
 obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
+obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
diff --git a/drivers/misc/exynos-chipid.c b/drivers/misc/exynos-chipid.c
new file mode 100644
index 0000000..eb23339
--- /dev/null
+++ b/drivers/misc/exynos-chipid.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2014-2015 Samsung Electronics Co., Ltd.
+ *	      http://www.samsung.com/
+ *
+ * EXYNOS - CHIP ID support
+ * Author: Pankaj Dubey <pankaj.dubey@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/exynos-soc.h>
+
+#define EXYNOS4_SOC_MASK	0xFFFE0
+#define EXYNOS5_SOC_MASK	0xFFFFF
+
+#define PROD_ID_SHIFT	(12)
+
+static void __iomem	*exynos_chipid_base;
+unsigned int exynos_soc_id = EXYNOS_SOC_UNKNOWN;
+unsigned int exynos_soc_rev;
+
+struct exynos_chipid_data {
+	unsigned int product_id_mask;
+	unsigned int product_id_shift;
+};
+
+static struct exynos_chipid_data exynos4_chipid_data = {
+	.product_id_mask	= EXYNOS4_SOC_MASK,
+	.product_id_shift	= PROD_ID_SHIFT,
+};
+
+static struct exynos_chipid_data exynos5_chipid_data = {
+	.product_id_mask	= EXYNOS5_SOC_MASK,
+	.product_id_shift	= PROD_ID_SHIFT,
+};
+
+static struct of_device_id of_exynos_chipid_ids[] = {
+	{
+		.compatible	= "samsung,exynos4-chipid",
+		.data		= (void *)&exynos4_chipid_data,
+	},
+	{
+		.compatible	= "samsung,exynos5-chipid",
+		.data		= (void *)&exynos5_chipid_data,
+	},
+	{},
+};
+
+/**
+ * early_exynos_chipid_init - Early chipid initialization
+ */
+void __init early_exynos_chipid_init(void)
+{
+	struct device_node *np = NULL;
+	const struct of_device_id *match;
+	struct exynos_chipid_data *chipid_data;
+	int pro_id;
+
+	if (!exynos_chipid_base) {
+		np = of_find_matching_node_and_match(NULL,
+				of_exynos_chipid_ids, &match);
+		if (!np)
+			panic("%s, failed to find chipid node\n", __func__);
+
+		chipid_data = (struct exynos_chipid_data *) match->data;
+		exynos_chipid_base = of_iomap(np, 0);
+
+		if (!exynos_chipid_base)
+			panic("%s: failed to map registers\n", __func__);
+
+		pro_id  = __raw_readl(exynos_chipid_base);
+		exynos_soc_id = (pro_id >> chipid_data->product_id_shift)
+			& chipid_data->product_id_mask;
+		exynos_soc_rev = pro_id & 0xFF;
+		pr_info("Exynos: CPUID[0x%x] CPU_REV[0x%x] Detected\n",
+				exynos_soc_id, exynos_soc_rev);
+	}
+}
diff --git a/include/linux/exynos-soc.h b/include/linux/exynos-soc.h
new file mode 100644
index 0000000..cb3ae06
--- /dev/null
+++ b/include/linux/exynos-soc.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Header for EXYNOS SoC Chipid support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __EXYNOS_SOC_H
+#define __EXYNOS_SOC_H
+
+/* enum holding product id of Exynos SoC
+ * Any new Exynos SoC product id should be added here
+ */
+enum exynos_soc_id {
+	EXYNOS4210 = 0xE4210,
+	EXYNOS4212 = 0xE4412,
+	EXYNOS4412 = 0xE4412,
+	EXYNOS5250 = 0x43520,
+	EXYNOS5420 = 0xE5420,
+	EXYNOS5440 = 0xE5440,
+	EXYNOS_SOC_UNKNOWN = -1,
+};
+
+/* enum holding revision id of Exynos SoC
+ * Any new Exynos SoC revision id should be added here
+ */
+enum exynos_soc_rev {
+	EXYNOS4210_REV_0	= 0x0,
+	EXYNOS4210_REV_1_0	= 0x10,
+	EXYNOS4210_REV_1_1	= 0x11,
+};
+
+/* Since we need chipid to be initialized as early as possible
+ * during secondary core bootup adding early initialization function
+ * Note: This should be called only after device tree gets unflatten
+ */
+extern void early_exynos_chipid_init(void);
+
+extern unsigned int exynos_soc_id;
+extern unsigned int exynos_soc_rev;
+
+#endif /* __EXYNOS_SOC_H */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support
@ 2014-05-03  6:11   ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-arm-kernel

Exynos SoCs have Chipid IP, for identification of product IDs
and SoC revistions. Till now we are using static macros
such as soc_is_exynosxxxx and #ifdefs for run time identification
of SoCs and their revisions. This is leading to add new Kconfig,
soc_is_exynosXXXX definitions each time new SoC support is getting
added. So this driver intends to provide initialization code
all these functionalites and thus helping in removing macros.

CC: Arnd Bergmann <arnd@arndb.de>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 drivers/misc/Kconfig         |    7 ++++
 drivers/misc/Makefile        |    1 +
 drivers/misc/exynos-chipid.c |   83 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/exynos-soc.h   |   46 +++++++++++++++++++++++
 4 files changed, 137 insertions(+)
 create mode 100644 drivers/misc/exynos-chipid.c
 create mode 100644 include/linux/exynos-soc.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1cb7408..f313bd3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -515,6 +515,13 @@ config SRAM
 	  the genalloc API. It is supposed to be used for small on-chip SRAM
 	  areas found on many SoCs.
 
+config EXYNOS_CHIPID
+	tristate "Support Exynos CHIPID"
+	default y
+	depends on ARCH_EXYNOS || ARM64
+	help
+	  If you say Y here you get support for the Exynos CHIP id.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7eb4b69..48c8fb5 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)		+= sram.o
 obj-y				+= mic/
 obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
+obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
diff --git a/drivers/misc/exynos-chipid.c b/drivers/misc/exynos-chipid.c
new file mode 100644
index 0000000..eb23339
--- /dev/null
+++ b/drivers/misc/exynos-chipid.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2014-2015 Samsung Electronics Co., Ltd.
+ *	      http://www.samsung.com/
+ *
+ * EXYNOS - CHIP ID support
+ * Author: Pankaj Dubey <pankaj.dubey@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/exynos-soc.h>
+
+#define EXYNOS4_SOC_MASK	0xFFFE0
+#define EXYNOS5_SOC_MASK	0xFFFFF
+
+#define PROD_ID_SHIFT	(12)
+
+static void __iomem	*exynos_chipid_base;
+unsigned int exynos_soc_id = EXYNOS_SOC_UNKNOWN;
+unsigned int exynos_soc_rev;
+
+struct exynos_chipid_data {
+	unsigned int product_id_mask;
+	unsigned int product_id_shift;
+};
+
+static struct exynos_chipid_data exynos4_chipid_data = {
+	.product_id_mask	= EXYNOS4_SOC_MASK,
+	.product_id_shift	= PROD_ID_SHIFT,
+};
+
+static struct exynos_chipid_data exynos5_chipid_data = {
+	.product_id_mask	= EXYNOS5_SOC_MASK,
+	.product_id_shift	= PROD_ID_SHIFT,
+};
+
+static struct of_device_id of_exynos_chipid_ids[] = {
+	{
+		.compatible	= "samsung,exynos4-chipid",
+		.data		= (void *)&exynos4_chipid_data,
+	},
+	{
+		.compatible	= "samsung,exynos5-chipid",
+		.data		= (void *)&exynos5_chipid_data,
+	},
+	{},
+};
+
+/**
+ * early_exynos_chipid_init - Early chipid initialization
+ */
+void __init early_exynos_chipid_init(void)
+{
+	struct device_node *np = NULL;
+	const struct of_device_id *match;
+	struct exynos_chipid_data *chipid_data;
+	int pro_id;
+
+	if (!exynos_chipid_base) {
+		np = of_find_matching_node_and_match(NULL,
+				of_exynos_chipid_ids, &match);
+		if (!np)
+			panic("%s, failed to find chipid node\n", __func__);
+
+		chipid_data = (struct exynos_chipid_data *) match->data;
+		exynos_chipid_base = of_iomap(np, 0);
+
+		if (!exynos_chipid_base)
+			panic("%s: failed to map registers\n", __func__);
+
+		pro_id  = __raw_readl(exynos_chipid_base);
+		exynos_soc_id = (pro_id >> chipid_data->product_id_shift)
+			& chipid_data->product_id_mask;
+		exynos_soc_rev = pro_id & 0xFF;
+		pr_info("Exynos: CPUID[0x%x] CPU_REV[0x%x] Detected\n",
+				exynos_soc_id, exynos_soc_rev);
+	}
+}
diff --git a/include/linux/exynos-soc.h b/include/linux/exynos-soc.h
new file mode 100644
index 0000000..cb3ae06
--- /dev/null
+++ b/include/linux/exynos-soc.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Header for EXYNOS SoC Chipid support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __EXYNOS_SOC_H
+#define __EXYNOS_SOC_H
+
+/* enum holding product id of Exynos SoC
+ * Any new Exynos SoC product id should be added here
+ */
+enum exynos_soc_id {
+	EXYNOS4210 = 0xE4210,
+	EXYNOS4212 = 0xE4412,
+	EXYNOS4412 = 0xE4412,
+	EXYNOS5250 = 0x43520,
+	EXYNOS5420 = 0xE5420,
+	EXYNOS5440 = 0xE5440,
+	EXYNOS_SOC_UNKNOWN = -1,
+};
+
+/* enum holding revision id of Exynos SoC
+ * Any new Exynos SoC revision id should be added here
+ */
+enum exynos_soc_rev {
+	EXYNOS4210_REV_0	= 0x0,
+	EXYNOS4210_REV_1_0	= 0x10,
+	EXYNOS4210_REV_1_1	= 0x11,
+};
+
+/* Since we need chipid to be initialized as early as possible
+ * during secondary core bootup adding early initialization function
+ * Note: This should be called only after device tree gets unflatten
+ */
+extern void early_exynos_chipid_init(void);
+
+extern unsigned int exynos_soc_id;
+extern unsigned int exynos_soc_rev;
+
+#endif /* __EXYNOS_SOC_H */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 4/4] ARM: EXYNOS: Refactoring to remove soc_is_exynosXXXX macros from exynos
  2014-05-03  6:11 ` Pankaj Dubey
@ 2014-05-03  6:11   ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-samsung-soc, linux-kernel, linux-arm-kernel
  Cc: devicetree, kgene.kim, linux, Pawel Moll, Pankaj Dubey, t.figa,
	Randy Dunlap, Rafael J. Wysocki, Rob Herring, Thomas Abraham,
	Viresh Kumar, Mike Turquette

This patch enables chipid driver for ARCH_EXYNOS and refactors
machine code as well as exynos cpufreq driver code for using
chipid driver for identification of SoC ID and SoC rev.

This patch also updates DT binding information in exynos4 and
exynos5 dtsi file. As to differentiate product id bit-mask we need
separate compatible string for exynos4 and exynos5. Hoping this will
be helpful in future as bit-mask and bit-shift bit may differ.
Added binding information as well.

CC: Mike Turquette <mturquette@linaro.org>
CC: Pawel Moll <pawel.moll@arm.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Randy Dunlap <rdunlap@infradead.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: "Rafael J. Wysocki" <rjw@rjwysocki.net>
CC: Thomas Abraham <thomas.abraham@linaro.org>
CC: Viresh Kumar <viresh.kumar@linaro.org>
CC: devicetree@vger.kernel.org
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 .../bindings/arm/samsung/exynos-chipid.txt         |   15 ++++++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/exynos4.dtsi                     |    2 +-
 arch/arm/boot/dts/exynos5.dtsi                     |    2 +-
 arch/arm/mach-exynos/exynos.c                      |   36 ++-----------
 arch/arm/mach-exynos/platsmp.c                     |   10 ++--
 arch/arm/mach-exynos/pm.c                          |   28 +++++-----
 arch/arm/plat-samsung/include/plat/cpu.h           |   57 --------------------
 drivers/clk/samsung/clk-exynos4.c                  |    2 +-
 drivers/cpufreq/exynos-cpufreq.c                   |    9 ++--
 drivers/cpufreq/exynos-cpufreq.h                   |    1 -
 drivers/cpufreq/exynos4x12-cpufreq.c               |    5 +-
 12 files changed, 51 insertions(+), 117 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt
new file mode 100644
index 0000000..b283c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt
@@ -0,0 +1,15 @@
+SAMSUNG Exynos4/Exynos5 SoCs Chipid driver.
+
+Required properties:
+- compatible : should be:
+	"samsung,exynos4-chipid"
+	"samsung,exynos5-chipid"
+  Details:
+  samsung,exynos4-chipid: Exynos4 SoCs has Chip ID block that can provide
+	product id, revision number and package information.
+	It has different product-id bit-mask than Exynos5 series SoC.
+  samsung,exynos5-chipid: Exynos5 SoCs has Chip ID block that can provide
+	product id, revision number and package information.
+	It has different product-id bit-mask then Exynos4 series SoC.
+- reg: offset and length of the register set
+
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a4eac2f..06f14a4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -856,6 +856,7 @@ config ARCH_EXYNOS
 	select SRAM
 	select USE_OF
 	select MFD_SYSCON
+	select EXYNOS_CHIPID
 	help
 	  Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
 
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 2f8bcd0..6db9029 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -46,7 +46,7 @@
 	};
 
 	chipid@10000000 {
-		compatible = "samsung,exynos4210-chipid";
+		compatible = "samsung,exynos4-chipid";
 		reg = <0x10000000 0x100>;
 	};
 
diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index 79d0608..18e6047 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -19,7 +19,7 @@
 	interrupt-parent = <&gic>;
 
 	chipid@10000000 {
-		compatible = "samsung,exynos4210-chipid";
+		compatible = "samsung,exynos5-chipid";
 		reg = <0x10000000 0x100>;
 	};
 
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 93ae076..64d804e 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -21,6 +21,7 @@
 #include <linux/pm_domain.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/exynos-soc.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -28,8 +29,6 @@
 #include <asm/mach/map.h>
 #include <asm/memory.h>
 
-#include <plat/cpu.h>
-
 #include "common.h"
 #include "mfc.h"
 #include "regs-pmu.h"
@@ -181,29 +180,6 @@ static void __init exynos_init_late(void)
 	exynos_pm_init();
 }
 
-static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
-					int depth, void *data)
-{
-	struct map_desc iodesc;
-	__be32 *reg;
-	unsigned long len;
-
-	if (!of_flat_dt_is_compatible(node, "samsung,exynos4210-chipid") &&
-		!of_flat_dt_is_compatible(node, "samsung,exynos5440-clock"))
-		return 0;
-
-	reg = of_get_flat_dt_prop(node, "reg", &len);
-	if (reg == NULL || len != (sizeof(unsigned long) * 2))
-		return 0;
-
-	iodesc.pfn = __phys_to_pfn(be32_to_cpu(reg[0]));
-	iodesc.length = be32_to_cpu(reg[1]) - 1;
-	iodesc.virtual = (unsigned long)S5P_VA_CHIPID;
-	iodesc.type = MT_DEVICE;
-	iotable_init(&iodesc, 1);
-	return 1;
-}
-
 static const struct of_device_id exynos_dt_pmu_match[] = {
 	{ .compatible = "samsung,exynos4210-pmu" },
 	{ .compatible = "samsung,exynos4212-pmu" },
@@ -254,11 +230,6 @@ static void __init exynos_init_io(void)
 {
 	debug_ll_io_init();
 
-	of_scan_flat_dt(exynos_fdt_map_chipid, NULL);
-
-	/* detect cpu id and rev. */
-	s5p_init_cpu(S5P_VA_CHIPID);
-
 	exynos_map_io();
 }
 
@@ -345,7 +316,10 @@ static void __init exynos_dt_machine_init(void)
 
 	exynos_map_pmu();
 
-	if (!soc_is_exynos5440())
+	if (exynos_soc_id == EXYNOS_SOC_UNKNOWN)
+		early_exynos_chipid_init();
+
+	if (exynos_soc_id != EXYNOS5440)
 		platform_device_register(&exynos_cpuidle);
 
 	platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 12ea1fa..f63fd36 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -20,13 +20,13 @@
 #include <linux/smp.h>
 #include <linux/io.h>
 #include <linux/of_address.h>
+#include <linux/exynos-soc.h>
 
 #include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
 #include <asm/smp_scu.h>
 #include <asm/firmware.h>
 
-#include <plat/cpu.h>
 #include <mach/map.h>
 
 #include "common.h"
@@ -67,7 +67,8 @@ static int __init exynos_smp_prepare_sram(void)
 
 static inline void __iomem *cpu_boot_reg_base(void)
 {
-	if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
+	if (exynos_soc_id == EXYNOS4210 &&
+			exynos_soc_rev == EXYNOS4210_REV_1_1)
 		return pmu_base + S5P_INFORM5;
 
 	return sram_base_addr;
@@ -78,9 +79,9 @@ static inline void __iomem *cpu_boot_reg(int cpu)
 	void __iomem *boot_reg;
 
 	boot_reg = cpu_boot_reg_base();
-	if (soc_is_exynos4412())
+	if (exynos_soc_id == EXYNOS4412)
 		boot_reg += 4*cpu;
-	else if (soc_is_exynos5420())
+	else if (exynos_soc_id == EXYNOS5420)
 		boot_reg += 4;
 	return boot_reg;
 }
@@ -261,6 +262,7 @@ static void __init exynos_smp_init_cpus(void)
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 {
 	int i;
+	early_exynos_chipid_init();
 
 	pmu_base = pmu_base_addr();
 
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index a7a1b7f..d7dab1d 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -22,13 +22,13 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/regmap.h>
+#include <linux/exynos-soc.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
 
-#include <plat/cpu.h>
 #include <plat/pm-common.h>
 #include <plat/pll.h>
 #include <plat/regs-srom.h>
@@ -87,7 +87,7 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
 {
 	const struct exynos_wkup_irq *wkup_irq;
 
-	if (soc_is_exynos5250())
+	if (exynos_soc_id == EXYNOS5250)
 		wkup_irq = exynos5250_wkup_irq;
 	else
 		wkup_irq = exynos4_wkup_irq;
@@ -106,11 +106,11 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
 	return -ENOENT;
 }
 
-#define EXYNOS_BOOT_VECTOR_ADDR	(samsung_rev() == EXYNOS4210_REV_1_1 ? \
-			S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
+#define EXYNOS_BOOT_VECTOR_ADDR	(exynos_soc_rev == EXYNOS4210_REV_1_1 ? \
+			S5P_INFORM7 : (exynos_soc_rev == EXYNOS4210_REV_1_0 ? \
 			0x24 : S5P_INFORM0))
-#define EXYNOS_BOOT_VECTOR_FLAG	(samsung_rev() == EXYNOS4210_REV_1_1 ? \
-			S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
+#define EXYNOS_BOOT_VECTOR_FLAG	(exynos_soc_rev == EXYNOS4210_REV_1_1 ? \
+			S5P_INFORM6 : (exynos_soc_rev == EXYNOS4210_REV_1_0 ? \
 			0x20 : S5P_INFORM1))
 
 #define S5P_CHECK_AFTR  0xFCBA0D10
@@ -124,7 +124,7 @@ static void exynos_set_wakeupmask(long mask)
 
 static void exynos_cpu_set_boot_vector(long flags)
 {
-	if (samsung_rev() == EXYNOS4210_REV_1_0) {
+	if (exynos_soc_rev == EXYNOS4210_REV_1_0) {
 		__raw_writel(virt_to_phys(exynos_cpu_resume),
 				S5P_VA_SYSRAM + EXYNOS_BOOT_VECTOR_ADDR);
 		__raw_writel(flags, S5P_VA_SYSRAM + EXYNOS_BOOT_VECTOR_FLAG);
@@ -188,7 +188,7 @@ static int exynos_cpu_suspend(unsigned long arg)
 	outer_flush_all();
 #endif
 
-	if (soc_is_exynos5250())
+	if (exynos_soc_id == EXYNOS5250)
 		flush_cache_all();
 
 	/* issue the standby signal into the pm unit. */
@@ -210,7 +210,7 @@ static void exynos_pm_prepare(void)
 
 	s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-	if (soc_is_exynos5250()) {
+	if (exynos_soc_id == EXYNOS5250) {
 		s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
 		/* Disable USE_RETENTION of JPEG_MEM_OPTION */
 		regmap_read(pmu_regmap, EXYNOS5_JPEG_MEM_OPTION, &tmp);
@@ -249,7 +249,7 @@ static int exynos_pm_suspend(void)
 	tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
 	regmap_write(pmu_regmap, S5P_CENTRAL_SEQ_OPTION, tmp);
 
-	if (!soc_is_exynos5250())
+	if (exynos_soc_id != EXYNOS5250)
 		exynos_cpu_save_register();
 
 	return 0;
@@ -283,7 +283,7 @@ static void exynos_pm_resume(void)
 	if (exynos_pm_central_resume())
 		goto early_wakeup;
 
-	if (!soc_is_exynos5250())
+	if (exynos_soc_id != EXYNOS5250)
 		exynos_cpu_restore_register();
 
 	/* For release retention */
@@ -296,13 +296,13 @@ static void exynos_pm_resume(void)
 	regmap_write(pmu_regmap, S5P_PAD_RET_EBIA_OPTION, (1 << 28));
 	regmap_write(pmu_regmap, S5P_PAD_RET_EBIB_OPTION, (1 << 28));
 
-	if (soc_is_exynos5250())
+	if (exynos_soc_id == EXYNOS5250)
 		s3c_pm_do_restore(exynos5_sys_save,
 			ARRAY_SIZE(exynos5_sys_save));
 
 	s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-	if (!soc_is_exynos5250())
+	if (exynos_soc_id != EXYNOS5250)
 		scu_enable(S5P_VA_SCU);
 
 early_wakeup:
@@ -395,7 +395,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
 
 	case CPU_PM_EXIT:
 		if (cpu == 0) {
-			if (!soc_is_exynos5250())
+			if (exynos_soc_id != EXYNOS5250)
 				scu_enable(S5P_VA_SCU);
 			exynos_cpu_restore_register();
 			exynos_pm_central_resume();
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
index 18a9a00..135471d 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -43,16 +43,6 @@ extern unsigned long samsung_cpu_id;
 #define S5PV210_CPU_ID		0x43110000
 #define S5PV210_CPU_MASK	0xFFFFF000
 
-#define EXYNOS4210_CPU_ID	0x43210000
-#define EXYNOS4212_CPU_ID	0x43220000
-#define EXYNOS4412_CPU_ID	0xE4412200
-#define EXYNOS4_CPU_MASK	0xFFFE0000
-
-#define EXYNOS5250_SOC_ID	0x43520000
-#define EXYNOS5420_SOC_ID	0xE5420000
-#define EXYNOS5440_SOC_ID	0xE5440000
-#define EXYNOS5_SOC_MASK	0xFFFFF000
-
 #define IS_SAMSUNG_CPU(name, id, mask)		\
 static inline int is_samsung_##name(void)	\
 {						\
@@ -68,12 +58,6 @@ IS_SAMSUNG_CPU(s5p6440, S5P6440_CPU_ID, S5P64XX_CPU_MASK)
 IS_SAMSUNG_CPU(s5p6450, S5P6450_CPU_ID, S5P64XX_CPU_MASK)
 IS_SAMSUNG_CPU(s5pc100, S5PC100_CPU_ID, S5PC100_CPU_MASK)
 IS_SAMSUNG_CPU(s5pv210, S5PV210_CPU_ID, S5PV210_CPU_MASK)
-IS_SAMSUNG_CPU(exynos4210, EXYNOS4210_CPU_ID, EXYNOS4_CPU_MASK)
-IS_SAMSUNG_CPU(exynos4212, EXYNOS4212_CPU_ID, EXYNOS4_CPU_MASK)
-IS_SAMSUNG_CPU(exynos4412, EXYNOS4412_CPU_ID, EXYNOS4_CPU_MASK)
-IS_SAMSUNG_CPU(exynos5250, EXYNOS5250_SOC_ID, EXYNOS5_SOC_MASK)
-IS_SAMSUNG_CPU(exynos5420, EXYNOS5420_SOC_ID, EXYNOS5_SOC_MASK)
-IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 
 #if defined(CONFIG_CPU_S3C2410) || defined(CONFIG_CPU_S3C2412) || \
     defined(CONFIG_CPU_S3C2416) || defined(CONFIG_CPU_S3C2440) || \
@@ -126,47 +110,6 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 # define soc_is_s5pv210()	0
 #endif
 
-#if defined(CONFIG_CPU_EXYNOS4210)
-# define soc_is_exynos4210()	is_samsung_exynos4210()
-#else
-# define soc_is_exynos4210()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS4212)
-# define soc_is_exynos4212()	is_samsung_exynos4212()
-#else
-# define soc_is_exynos4212()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS4412)
-# define soc_is_exynos4412()	is_samsung_exynos4412()
-#else
-# define soc_is_exynos4412()	0
-#endif
-
-#define EXYNOS4210_REV_0	(0x0)
-#define EXYNOS4210_REV_1_0	(0x10)
-#define EXYNOS4210_REV_1_1	(0x11)
-
-#if defined(CONFIG_SOC_EXYNOS5250)
-# define soc_is_exynos5250()	is_samsung_exynos5250()
-#else
-# define soc_is_exynos5250()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS5420)
-# define soc_is_exynos5420()	is_samsung_exynos5420()
-#else
-# define soc_is_exynos5420()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS5440)
-# define soc_is_exynos5440()	is_samsung_exynos5440()
-#else
-# define soc_is_exynos5440()	0
-#endif
-
-
 #define IODESC_ENT(x) { (unsigned long)S3C24XX_VA_##x, __phys_to_pfn(S3C24XX_PA_##x), S3C24XX_SZ_##x, MT_DEVICE }
 
 #ifndef KHZ
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index b4f9672..7b0ea1f 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1030,7 +1030,7 @@ static unsigned long exynos4_get_xom(void)
 	void __iomem *chipid_base;
 	struct device_node *np;
 
-	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
+	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4-chipid");
 	if (np) {
 		chipid_base = of_iomap(np, 0);
 
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index f99cfe2..458ddcc 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -17,8 +17,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/cpufreq.h>
 #include <linux/platform_device.h>
-
-#include <plat/cpu.h>
+#include <linux/exynos-soc.h>
 
 #include "exynos-cpufreq.h"
 
@@ -163,11 +162,11 @@ static int exynos_cpufreq_probe(struct platform_device *pdev)
 	if (!exynos_info)
 		return -ENOMEM;
 
-	if (soc_is_exynos4210())
+	if (exynos_soc_id == EXYNOS4210)
 		ret = exynos4210_cpufreq_init(exynos_info);
-	else if (soc_is_exynos4212() || soc_is_exynos4412())
+	else if ((exynos_soc_id == EXYNOS4212) || (exynos_soc_id == EXYNOS4412))
 		ret = exynos4x12_cpufreq_init(exynos_info);
-	else if (soc_is_exynos5250())
+	else if (exynos_soc_id == EXYNOS5250)
 		ret = exynos5250_cpufreq_init(exynos_info);
 	else
 		return 0;
diff --git a/drivers/cpufreq/exynos-cpufreq.h b/drivers/cpufreq/exynos-cpufreq.h
index 3ddade8..4201b6d 100644
--- a/drivers/cpufreq/exynos-cpufreq.h
+++ b/drivers/cpufreq/exynos-cpufreq.h
@@ -68,7 +68,6 @@ static inline int exynos5250_cpufreq_init(struct exynos_dvfs_info *info)
 }
 #endif
 
-#include <plat/cpu.h>
 #include <mach/map.h>
 
 #define EXYNOS4_CLKSRC_CPU			(S5P_VA_CMU + 0x14200)
diff --git a/drivers/cpufreq/exynos4x12-cpufreq.c b/drivers/cpufreq/exynos4x12-cpufreq.c
index 466c76a..e328c8f 100644
--- a/drivers/cpufreq/exynos4x12-cpufreq.c
+++ b/drivers/cpufreq/exynos4x12-cpufreq.c
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/cpufreq.h>
+#include <linux/exynos-soc.h>
 
 #include "exynos-cpufreq.h"
 
@@ -115,7 +116,7 @@ static void exynos4x12_set_clkdiv(unsigned int div_index)
 	tmp = apll_freq_4x12[div_index].clk_div_cpu1;
 
 	__raw_writel(tmp, EXYNOS4_CLKDIV_CPU1);
-	if (soc_is_exynos4212())
+	if (exynos_soc_id == EXYNOS4212)
 		stat_cpu1 = 0x11;
 	else
 		stat_cpu1 = 0x111;
@@ -184,7 +185,7 @@ int exynos4x12_cpufreq_init(struct exynos_dvfs_info *info)
 	if (IS_ERR(mout_apll))
 		goto err_mout_apll;
 
-	if (soc_is_exynos4212())
+	if (exynos_soc_id == EXYNOS4212)
 		apll_freq_4x12 = apll_freq_4212;
 	else
 		apll_freq_4x12 = apll_freq_4412;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 4/4] ARM: EXYNOS: Refactoring to remove soc_is_exynosXXXX macros from exynos
@ 2014-05-03  6:11   ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-03  6:11 UTC (permalink / raw)
  To: linux-arm-kernel

This patch enables chipid driver for ARCH_EXYNOS and refactors
machine code as well as exynos cpufreq driver code for using
chipid driver for identification of SoC ID and SoC rev.

This patch also updates DT binding information in exynos4 and
exynos5 dtsi file. As to differentiate product id bit-mask we need
separate compatible string for exynos4 and exynos5. Hoping this will
be helpful in future as bit-mask and bit-shift bit may differ.
Added binding information as well.

CC: Mike Turquette <mturquette@linaro.org>
CC: Pawel Moll <pawel.moll@arm.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Randy Dunlap <rdunlap@infradead.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: "Rafael J. Wysocki" <rjw@rjwysocki.net>
CC: Thomas Abraham <thomas.abraham@linaro.org>
CC: Viresh Kumar <viresh.kumar@linaro.org>
CC: devicetree at vger.kernel.org
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 .../bindings/arm/samsung/exynos-chipid.txt         |   15 ++++++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/exynos4.dtsi                     |    2 +-
 arch/arm/boot/dts/exynos5.dtsi                     |    2 +-
 arch/arm/mach-exynos/exynos.c                      |   36 ++-----------
 arch/arm/mach-exynos/platsmp.c                     |   10 ++--
 arch/arm/mach-exynos/pm.c                          |   28 +++++-----
 arch/arm/plat-samsung/include/plat/cpu.h           |   57 --------------------
 drivers/clk/samsung/clk-exynos4.c                  |    2 +-
 drivers/cpufreq/exynos-cpufreq.c                   |    9 ++--
 drivers/cpufreq/exynos-cpufreq.h                   |    1 -
 drivers/cpufreq/exynos4x12-cpufreq.c               |    5 +-
 12 files changed, 51 insertions(+), 117 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt
new file mode 100644
index 0000000..b283c6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-chipid.txt
@@ -0,0 +1,15 @@
+SAMSUNG Exynos4/Exynos5 SoCs Chipid driver.
+
+Required properties:
+- compatible : should be:
+	"samsung,exynos4-chipid"
+	"samsung,exynos5-chipid"
+  Details:
+  samsung,exynos4-chipid: Exynos4 SoCs has Chip ID block that can provide
+	product id, revision number and package information.
+	It has different product-id bit-mask than Exynos5 series SoC.
+  samsung,exynos5-chipid: Exynos5 SoCs has Chip ID block that can provide
+	product id, revision number and package information.
+	It has different product-id bit-mask then Exynos4 series SoC.
+- reg: offset and length of the register set
+
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a4eac2f..06f14a4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -856,6 +856,7 @@ config ARCH_EXYNOS
 	select SRAM
 	select USE_OF
 	select MFD_SYSCON
+	select EXYNOS_CHIPID
 	help
 	  Support for SAMSUNG's EXYNOS SoCs (EXYNOS4/5)
 
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 2f8bcd0..6db9029 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -46,7 +46,7 @@
 	};
 
 	chipid at 10000000 {
-		compatible = "samsung,exynos4210-chipid";
+		compatible = "samsung,exynos4-chipid";
 		reg = <0x10000000 0x100>;
 	};
 
diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index 79d0608..18e6047 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -19,7 +19,7 @@
 	interrupt-parent = <&gic>;
 
 	chipid at 10000000 {
-		compatible = "samsung,exynos4210-chipid";
+		compatible = "samsung,exynos5-chipid";
 		reg = <0x10000000 0x100>;
 	};
 
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 93ae076..64d804e 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -21,6 +21,7 @@
 #include <linux/pm_domain.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/exynos-soc.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -28,8 +29,6 @@
 #include <asm/mach/map.h>
 #include <asm/memory.h>
 
-#include <plat/cpu.h>
-
 #include "common.h"
 #include "mfc.h"
 #include "regs-pmu.h"
@@ -181,29 +180,6 @@ static void __init exynos_init_late(void)
 	exynos_pm_init();
 }
 
-static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
-					int depth, void *data)
-{
-	struct map_desc iodesc;
-	__be32 *reg;
-	unsigned long len;
-
-	if (!of_flat_dt_is_compatible(node, "samsung,exynos4210-chipid") &&
-		!of_flat_dt_is_compatible(node, "samsung,exynos5440-clock"))
-		return 0;
-
-	reg = of_get_flat_dt_prop(node, "reg", &len);
-	if (reg == NULL || len != (sizeof(unsigned long) * 2))
-		return 0;
-
-	iodesc.pfn = __phys_to_pfn(be32_to_cpu(reg[0]));
-	iodesc.length = be32_to_cpu(reg[1]) - 1;
-	iodesc.virtual = (unsigned long)S5P_VA_CHIPID;
-	iodesc.type = MT_DEVICE;
-	iotable_init(&iodesc, 1);
-	return 1;
-}
-
 static const struct of_device_id exynos_dt_pmu_match[] = {
 	{ .compatible = "samsung,exynos4210-pmu" },
 	{ .compatible = "samsung,exynos4212-pmu" },
@@ -254,11 +230,6 @@ static void __init exynos_init_io(void)
 {
 	debug_ll_io_init();
 
-	of_scan_flat_dt(exynos_fdt_map_chipid, NULL);
-
-	/* detect cpu id and rev. */
-	s5p_init_cpu(S5P_VA_CHIPID);
-
 	exynos_map_io();
 }
 
@@ -345,7 +316,10 @@ static void __init exynos_dt_machine_init(void)
 
 	exynos_map_pmu();
 
-	if (!soc_is_exynos5440())
+	if (exynos_soc_id == EXYNOS_SOC_UNKNOWN)
+		early_exynos_chipid_init();
+
+	if (exynos_soc_id != EXYNOS5440)
 		platform_device_register(&exynos_cpuidle);
 
 	platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 12ea1fa..f63fd36 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -20,13 +20,13 @@
 #include <linux/smp.h>
 #include <linux/io.h>
 #include <linux/of_address.h>
+#include <linux/exynos-soc.h>
 
 #include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
 #include <asm/smp_scu.h>
 #include <asm/firmware.h>
 
-#include <plat/cpu.h>
 #include <mach/map.h>
 
 #include "common.h"
@@ -67,7 +67,8 @@ static int __init exynos_smp_prepare_sram(void)
 
 static inline void __iomem *cpu_boot_reg_base(void)
 {
-	if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
+	if (exynos_soc_id == EXYNOS4210 &&
+			exynos_soc_rev == EXYNOS4210_REV_1_1)
 		return pmu_base + S5P_INFORM5;
 
 	return sram_base_addr;
@@ -78,9 +79,9 @@ static inline void __iomem *cpu_boot_reg(int cpu)
 	void __iomem *boot_reg;
 
 	boot_reg = cpu_boot_reg_base();
-	if (soc_is_exynos4412())
+	if (exynos_soc_id == EXYNOS4412)
 		boot_reg += 4*cpu;
-	else if (soc_is_exynos5420())
+	else if (exynos_soc_id == EXYNOS5420)
 		boot_reg += 4;
 	return boot_reg;
 }
@@ -261,6 +262,7 @@ static void __init exynos_smp_init_cpus(void)
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 {
 	int i;
+	early_exynos_chipid_init();
 
 	pmu_base = pmu_base_addr();
 
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index a7a1b7f..d7dab1d 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -22,13 +22,13 @@
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/regmap.h>
+#include <linux/exynos-soc.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
 
-#include <plat/cpu.h>
 #include <plat/pm-common.h>
 #include <plat/pll.h>
 #include <plat/regs-srom.h>
@@ -87,7 +87,7 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
 {
 	const struct exynos_wkup_irq *wkup_irq;
 
-	if (soc_is_exynos5250())
+	if (exynos_soc_id == EXYNOS5250)
 		wkup_irq = exynos5250_wkup_irq;
 	else
 		wkup_irq = exynos4_wkup_irq;
@@ -106,11 +106,11 @@ static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
 	return -ENOENT;
 }
 
-#define EXYNOS_BOOT_VECTOR_ADDR	(samsung_rev() == EXYNOS4210_REV_1_1 ? \
-			S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
+#define EXYNOS_BOOT_VECTOR_ADDR	(exynos_soc_rev == EXYNOS4210_REV_1_1 ? \
+			S5P_INFORM7 : (exynos_soc_rev == EXYNOS4210_REV_1_0 ? \
 			0x24 : S5P_INFORM0))
-#define EXYNOS_BOOT_VECTOR_FLAG	(samsung_rev() == EXYNOS4210_REV_1_1 ? \
-			S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
+#define EXYNOS_BOOT_VECTOR_FLAG	(exynos_soc_rev == EXYNOS4210_REV_1_1 ? \
+			S5P_INFORM6 : (exynos_soc_rev == EXYNOS4210_REV_1_0 ? \
 			0x20 : S5P_INFORM1))
 
 #define S5P_CHECK_AFTR  0xFCBA0D10
@@ -124,7 +124,7 @@ static void exynos_set_wakeupmask(long mask)
 
 static void exynos_cpu_set_boot_vector(long flags)
 {
-	if (samsung_rev() == EXYNOS4210_REV_1_0) {
+	if (exynos_soc_rev == EXYNOS4210_REV_1_0) {
 		__raw_writel(virt_to_phys(exynos_cpu_resume),
 				S5P_VA_SYSRAM + EXYNOS_BOOT_VECTOR_ADDR);
 		__raw_writel(flags, S5P_VA_SYSRAM + EXYNOS_BOOT_VECTOR_FLAG);
@@ -188,7 +188,7 @@ static int exynos_cpu_suspend(unsigned long arg)
 	outer_flush_all();
 #endif
 
-	if (soc_is_exynos5250())
+	if (exynos_soc_id == EXYNOS5250)
 		flush_cache_all();
 
 	/* issue the standby signal into the pm unit. */
@@ -210,7 +210,7 @@ static void exynos_pm_prepare(void)
 
 	s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-	if (soc_is_exynos5250()) {
+	if (exynos_soc_id == EXYNOS5250) {
 		s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
 		/* Disable USE_RETENTION of JPEG_MEM_OPTION */
 		regmap_read(pmu_regmap, EXYNOS5_JPEG_MEM_OPTION, &tmp);
@@ -249,7 +249,7 @@ static int exynos_pm_suspend(void)
 	tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
 	regmap_write(pmu_regmap, S5P_CENTRAL_SEQ_OPTION, tmp);
 
-	if (!soc_is_exynos5250())
+	if (exynos_soc_id != EXYNOS5250)
 		exynos_cpu_save_register();
 
 	return 0;
@@ -283,7 +283,7 @@ static void exynos_pm_resume(void)
 	if (exynos_pm_central_resume())
 		goto early_wakeup;
 
-	if (!soc_is_exynos5250())
+	if (exynos_soc_id != EXYNOS5250)
 		exynos_cpu_restore_register();
 
 	/* For release retention */
@@ -296,13 +296,13 @@ static void exynos_pm_resume(void)
 	regmap_write(pmu_regmap, S5P_PAD_RET_EBIA_OPTION, (1 << 28));
 	regmap_write(pmu_regmap, S5P_PAD_RET_EBIB_OPTION, (1 << 28));
 
-	if (soc_is_exynos5250())
+	if (exynos_soc_id == EXYNOS5250)
 		s3c_pm_do_restore(exynos5_sys_save,
 			ARRAY_SIZE(exynos5_sys_save));
 
 	s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
 
-	if (!soc_is_exynos5250())
+	if (exynos_soc_id != EXYNOS5250)
 		scu_enable(S5P_VA_SCU);
 
 early_wakeup:
@@ -395,7 +395,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
 
 	case CPU_PM_EXIT:
 		if (cpu == 0) {
-			if (!soc_is_exynos5250())
+			if (exynos_soc_id != EXYNOS5250)
 				scu_enable(S5P_VA_SCU);
 			exynos_cpu_restore_register();
 			exynos_pm_central_resume();
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
index 18a9a00..135471d 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -43,16 +43,6 @@ extern unsigned long samsung_cpu_id;
 #define S5PV210_CPU_ID		0x43110000
 #define S5PV210_CPU_MASK	0xFFFFF000
 
-#define EXYNOS4210_CPU_ID	0x43210000
-#define EXYNOS4212_CPU_ID	0x43220000
-#define EXYNOS4412_CPU_ID	0xE4412200
-#define EXYNOS4_CPU_MASK	0xFFFE0000
-
-#define EXYNOS5250_SOC_ID	0x43520000
-#define EXYNOS5420_SOC_ID	0xE5420000
-#define EXYNOS5440_SOC_ID	0xE5440000
-#define EXYNOS5_SOC_MASK	0xFFFFF000
-
 #define IS_SAMSUNG_CPU(name, id, mask)		\
 static inline int is_samsung_##name(void)	\
 {						\
@@ -68,12 +58,6 @@ IS_SAMSUNG_CPU(s5p6440, S5P6440_CPU_ID, S5P64XX_CPU_MASK)
 IS_SAMSUNG_CPU(s5p6450, S5P6450_CPU_ID, S5P64XX_CPU_MASK)
 IS_SAMSUNG_CPU(s5pc100, S5PC100_CPU_ID, S5PC100_CPU_MASK)
 IS_SAMSUNG_CPU(s5pv210, S5PV210_CPU_ID, S5PV210_CPU_MASK)
-IS_SAMSUNG_CPU(exynos4210, EXYNOS4210_CPU_ID, EXYNOS4_CPU_MASK)
-IS_SAMSUNG_CPU(exynos4212, EXYNOS4212_CPU_ID, EXYNOS4_CPU_MASK)
-IS_SAMSUNG_CPU(exynos4412, EXYNOS4412_CPU_ID, EXYNOS4_CPU_MASK)
-IS_SAMSUNG_CPU(exynos5250, EXYNOS5250_SOC_ID, EXYNOS5_SOC_MASK)
-IS_SAMSUNG_CPU(exynos5420, EXYNOS5420_SOC_ID, EXYNOS5_SOC_MASK)
-IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 
 #if defined(CONFIG_CPU_S3C2410) || defined(CONFIG_CPU_S3C2412) || \
     defined(CONFIG_CPU_S3C2416) || defined(CONFIG_CPU_S3C2440) || \
@@ -126,47 +110,6 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 # define soc_is_s5pv210()	0
 #endif
 
-#if defined(CONFIG_CPU_EXYNOS4210)
-# define soc_is_exynos4210()	is_samsung_exynos4210()
-#else
-# define soc_is_exynos4210()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS4212)
-# define soc_is_exynos4212()	is_samsung_exynos4212()
-#else
-# define soc_is_exynos4212()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS4412)
-# define soc_is_exynos4412()	is_samsung_exynos4412()
-#else
-# define soc_is_exynos4412()	0
-#endif
-
-#define EXYNOS4210_REV_0	(0x0)
-#define EXYNOS4210_REV_1_0	(0x10)
-#define EXYNOS4210_REV_1_1	(0x11)
-
-#if defined(CONFIG_SOC_EXYNOS5250)
-# define soc_is_exynos5250()	is_samsung_exynos5250()
-#else
-# define soc_is_exynos5250()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS5420)
-# define soc_is_exynos5420()	is_samsung_exynos5420()
-#else
-# define soc_is_exynos5420()	0
-#endif
-
-#if defined(CONFIG_SOC_EXYNOS5440)
-# define soc_is_exynos5440()	is_samsung_exynos5440()
-#else
-# define soc_is_exynos5440()	0
-#endif
-
-
 #define IODESC_ENT(x) { (unsigned long)S3C24XX_VA_##x, __phys_to_pfn(S3C24XX_PA_##x), S3C24XX_SZ_##x, MT_DEVICE }
 
 #ifndef KHZ
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index b4f9672..7b0ea1f 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1030,7 +1030,7 @@ static unsigned long exynos4_get_xom(void)
 	void __iomem *chipid_base;
 	struct device_node *np;
 
-	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
+	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4-chipid");
 	if (np) {
 		chipid_base = of_iomap(np, 0);
 
diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
index f99cfe2..458ddcc 100644
--- a/drivers/cpufreq/exynos-cpufreq.c
+++ b/drivers/cpufreq/exynos-cpufreq.c
@@ -17,8 +17,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/cpufreq.h>
 #include <linux/platform_device.h>
-
-#include <plat/cpu.h>
+#include <linux/exynos-soc.h>
 
 #include "exynos-cpufreq.h"
 
@@ -163,11 +162,11 @@ static int exynos_cpufreq_probe(struct platform_device *pdev)
 	if (!exynos_info)
 		return -ENOMEM;
 
-	if (soc_is_exynos4210())
+	if (exynos_soc_id == EXYNOS4210)
 		ret = exynos4210_cpufreq_init(exynos_info);
-	else if (soc_is_exynos4212() || soc_is_exynos4412())
+	else if ((exynos_soc_id == EXYNOS4212) || (exynos_soc_id == EXYNOS4412))
 		ret = exynos4x12_cpufreq_init(exynos_info);
-	else if (soc_is_exynos5250())
+	else if (exynos_soc_id == EXYNOS5250)
 		ret = exynos5250_cpufreq_init(exynos_info);
 	else
 		return 0;
diff --git a/drivers/cpufreq/exynos-cpufreq.h b/drivers/cpufreq/exynos-cpufreq.h
index 3ddade8..4201b6d 100644
--- a/drivers/cpufreq/exynos-cpufreq.h
+++ b/drivers/cpufreq/exynos-cpufreq.h
@@ -68,7 +68,6 @@ static inline int exynos5250_cpufreq_init(struct exynos_dvfs_info *info)
 }
 #endif
 
-#include <plat/cpu.h>
 #include <mach/map.h>
 
 #define EXYNOS4_CLKSRC_CPU			(S5P_VA_CMU + 0x14200)
diff --git a/drivers/cpufreq/exynos4x12-cpufreq.c b/drivers/cpufreq/exynos4x12-cpufreq.c
index 466c76a..e328c8f 100644
--- a/drivers/cpufreq/exynos4x12-cpufreq.c
+++ b/drivers/cpufreq/exynos4x12-cpufreq.c
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/cpufreq.h>
+#include <linux/exynos-soc.h>
 
 #include "exynos-cpufreq.h"
 
@@ -115,7 +116,7 @@ static void exynos4x12_set_clkdiv(unsigned int div_index)
 	tmp = apll_freq_4x12[div_index].clk_div_cpu1;
 
 	__raw_writel(tmp, EXYNOS4_CLKDIV_CPU1);
-	if (soc_is_exynos4212())
+	if (exynos_soc_id == EXYNOS4212)
 		stat_cpu1 = 0x11;
 	else
 		stat_cpu1 = 0x111;
@@ -184,7 +185,7 @@ int exynos4x12_cpufreq_init(struct exynos_dvfs_info *info)
 	if (IS_ERR(mout_apll))
 		goto err_mout_apll;
 
-	if (soc_is_exynos4212())
+	if (exynos_soc_id == EXYNOS4212)
 		apll_freq_4x12 = apll_freq_4212;
 	else
 		apll_freq_4x12 = apll_freq_4412;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-03  6:11 ` Pankaj Dubey
@ 2014-05-03 15:02   ` Arnd Bergmann
  -1 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-03 15:02 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Pankaj Dubey, linux-samsung-soc, linux-kernel, t.figa, kgene.kim, linux

On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
> This patch series attempts to get rid of soc_is_exynosXXXX macros
> and eventually with the help of this series we can probably get
> rid of CONFIG_SOC_EXYNOSXXXX in near future.
> Each Exynos SoC has ChipID block which can give information about
> SoC's product Id and revision number. Currently we have single
> DT binding information for this as "samsung,exynos4210-chipid".
> But Exynos4 and Exynos5 SoC series have one small difference in
> chip Id, with resepect to product id bit-masks. So it means we
> should have separate compatible string for these different series
> of SoCs. So I have created new binding information for handling
> this difference. Also currently I can think of putting this driver
> code under "drivers/misc/" but suggestions are welcome.
> Also current form of driver is missing platfrom driver and needs
> init function to be called from machine file (either exynos.c or
> platsmp.c). I hope lot of suggestions and comments to improve this
> further.
> 
> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
> and prepared on top of following patch series and it's dependent
> patch series.

I think putting it into drivers/soc would be most appropriate.
We already have a few drivers lined up that we want in there,
although the directory currently doesn't exist.

However, I would ask that you use the infrastructure provided by
drivers/base/soc.c when you add this driver, to also make the
information available to user space using a standard API.

Ideally this should be done by slightly restructuring the DT
source to make all on-chip devices appear below the soc node.
We'd have to think a bit about how to best do this while
preserving compatibility with existing dts files.

Regarding patch 4, this is not what I meant when I asked for
removing the soc_is_exynos* macros. You basically do a 1:1 replacement
using a different interface, but you still have code that does
things differently based on a global identification.
The only user left in device drivers is now the cpufreq driver,
which is going to be replaced anyway, so that is ok. Having
a global variable that is accessible to random device drivers
is probably not a good idea though, it will just lead to
bad coding in drivers again.

To give an example of how I think it should really be restructured,
let's look at one function:

static const struct exynos_wkup_irq exynos4_wkup_irq[] = {
        { 76, BIT(1) }, /* RTC alarm */
        { 77, BIT(2) }, /* RTC tick */
        { /* sentinel */ },
};

static const struct exynos_wkup_irq exynos5250_wkup_irq[] = {
        { 75, BIT(1) }, /* RTC alarm */
        { 76, BIT(2) }, /* RTC tick */
        { /* sentinel */ },
};

static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
{
        const struct exynos_wkup_irq *wkup_irq;

        if (soc_is_exynos5250())
                wkup_irq = exynos5250_wkup_irq;
        else
                wkup_irq = exynos4_wkup_irq;

	...
}

There are multiple problems with this code:

- As mentioned, you depend on a specific SoC identification for
  something that could be done completely generic.
- The knowledge about what is a wakeup source or not doesn't
  really belong here. We don't have a DT binding for wakeups
  as far as I'm aware of, but this should probably be handled
  locally in the RTC device node, possibly in the node that
  contains the S5P_WAKEUP_MASK register.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-03 15:02   ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-03 15:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
> This patch series attempts to get rid of soc_is_exynosXXXX macros
> and eventually with the help of this series we can probably get
> rid of CONFIG_SOC_EXYNOSXXXX in near future.
> Each Exynos SoC has ChipID block which can give information about
> SoC's product Id and revision number. Currently we have single
> DT binding information for this as "samsung,exynos4210-chipid".
> But Exynos4 and Exynos5 SoC series have one small difference in
> chip Id, with resepect to product id bit-masks. So it means we
> should have separate compatible string for these different series
> of SoCs. So I have created new binding information for handling
> this difference. Also currently I can think of putting this driver
> code under "drivers/misc/" but suggestions are welcome.
> Also current form of driver is missing platfrom driver and needs
> init function to be called from machine file (either exynos.c or
> platsmp.c). I hope lot of suggestions and comments to improve this
> further.
> 
> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
> and prepared on top of following patch series and it's dependent
> patch series.

I think putting it into drivers/soc would be most appropriate.
We already have a few drivers lined up that we want in there,
although the directory currently doesn't exist.

However, I would ask that you use the infrastructure provided by
drivers/base/soc.c when you add this driver, to also make the
information available to user space using a standard API.

Ideally this should be done by slightly restructuring the DT
source to make all on-chip devices appear below the soc node.
We'd have to think a bit about how to best do this while
preserving compatibility with existing dts files.

Regarding patch 4, this is not what I meant when I asked for
removing the soc_is_exynos* macros. You basically do a 1:1 replacement
using a different interface, but you still have code that does
things differently based on a global identification.
The only user left in device drivers is now the cpufreq driver,
which is going to be replaced anyway, so that is ok. Having
a global variable that is accessible to random device drivers
is probably not a good idea though, it will just lead to
bad coding in drivers again.

To give an example of how I think it should really be restructured,
let's look at one function:

static const struct exynos_wkup_irq exynos4_wkup_irq[] = {
        { 76, BIT(1) }, /* RTC alarm */
        { 77, BIT(2) }, /* RTC tick */
        { /* sentinel */ },
};

static const struct exynos_wkup_irq exynos5250_wkup_irq[] = {
        { 75, BIT(1) }, /* RTC alarm */
        { 76, BIT(2) }, /* RTC tick */
        { /* sentinel */ },
};

static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
{
        const struct exynos_wkup_irq *wkup_irq;

        if (soc_is_exynos5250())
                wkup_irq = exynos5250_wkup_irq;
        else
                wkup_irq = exynos4_wkup_irq;

	...
}

There are multiple problems with this code:

- As mentioned, you depend on a specific SoC identification for
  something that could be done completely generic.
- The knowledge about what is a wakeup source or not doesn't
  really belong here. We don't have a DT binding for wakeups
  as far as I'm aware of, but this should probably be handled
  locally in the RTC device node, possibly in the node that
  contains the S5P_WAKEUP_MASK register.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support
  2014-05-03  6:11   ` Pankaj Dubey
@ 2014-05-05  7:57     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2014-05-05  7:57 UTC (permalink / raw)
  To: Pankaj Dubey
  Cc: linux-samsung-soc, linux-kernel, linux-arm-kernel, kgene.kim,
	linux, Arnd Bergmann, t.figa, Greg Kroah-Hartman

On sob, 2014-05-03 at 15:11 +0900, Pankaj Dubey wrote:
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 7eb4b69..48c8fb5 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)		+= sram.o
>  obj-y				+= mic/
>  obj-$(CONFIG_GENWQE)		+= genwqe/
>  obj-$(CONFIG_ECHO)		+= echo/
> +obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
> diff --git a/drivers/misc/exynos-chipid.c b/drivers/misc/exynos-chipid.c
> new file mode 100644
> index 0000000..eb23339
> --- /dev/null
> +++ b/drivers/misc/exynos-chipid.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2014-2015 Samsung Electronics Co., Ltd.
> + *	      http://www.samsung.com/

I wonder why this is copyrighted in the future (2015)?

Best regards,
Krzysztof



^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support
@ 2014-05-05  7:57     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2014-05-05  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

On sob, 2014-05-03 at 15:11 +0900, Pankaj Dubey wrote:
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 7eb4b69..48c8fb5 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)		+= sram.o
>  obj-y				+= mic/
>  obj-$(CONFIG_GENWQE)		+= genwqe/
>  obj-$(CONFIG_ECHO)		+= echo/
> +obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
> diff --git a/drivers/misc/exynos-chipid.c b/drivers/misc/exynos-chipid.c
> new file mode 100644
> index 0000000..eb23339
> --- /dev/null
> +++ b/drivers/misc/exynos-chipid.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2014-2015 Samsung Electronics Co., Ltd.
> + *	      http://www.samsung.com/

I wonder why this is copyrighted in the future (2015)?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-03 15:02   ` Arnd Bergmann
@ 2014-05-05  9:23     ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-05  9:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, linux-samsung-soc, linux-kernel, t.figa,
	kgene.kim, linux

Hi Arnd,

Thanks for review and suggestions.

On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
>> This patch series attempts to get rid of soc_is_exynosXXXX macros
>> and eventually with the help of this series we can probably get
>> rid of CONFIG_SOC_EXYNOSXXXX in near future.
>> Each Exynos SoC has ChipID block which can give information about
>> SoC's product Id and revision number. Currently we have single
>> DT binding information for this as "samsung,exynos4210-chipid".
>> But Exynos4 and Exynos5 SoC series have one small difference in
>> chip Id, with resepect to product id bit-masks. So it means we
>> should have separate compatible string for these different series
>> of SoCs. So I have created new binding information for handling
>> this difference. Also currently I can think of putting this driver
>> code under "drivers/misc/" but suggestions are welcome.
>> Also current form of driver is missing platfrom driver and needs
>> init function to be called from machine file (either exynos.c or
>> platsmp.c). I hope lot of suggestions and comments to improve this
>> further.
>>
>> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
>> and prepared on top of following patch series and it's dependent
>> patch series.
> I think putting it into drivers/soc would be most appropriate.
> We already have a few drivers lined up that we want in there,
> although the directory currently doesn't exist.
OK. Will move to "drivers/soc". I can see already some patches
have been posted [1] by Andy Gross for creating directory
"drivers/soc" I will use those patches for adding samsung folder
under "drivers/soc".

[1]: https://lkml.org/lkml/2014/4/21/46

>
> However, I would ask that you use the infrastructure provided by
> drivers/base/soc.c when you add this driver, to also make the
> information available to user space using a standard API.

OK. Will update in next version.

>
> Ideally this should be done by slightly restructuring the DT
> source to make all on-chip devices appear below the soc node.

Currently I can't see soc nodes in exynos4 and exynos5 DT files.
So isn't it should be a separate patch first to modify all exynos4
exynos5 DT files to move all devices under soc node?
In that case existing chipid node will be also moved under soc node.

> We'd have to think a bit about how to best do this while
> preserving compatibility with existing dts files.

Is it necessary in this case?
As I have mentioned there is difference in bit-mask among exynos4
and exynos5's chipid. So is this reason not sufficient to keep separate
compatible for both?
Also even if we get some way to preserve existing compatibility, I afraid
in chipid driver that implementation will not look good, at least I am not
able to think of any good way. Any suggestions?

>
> Regarding patch 4, this is not what I meant when I asked for
> removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> using a different interface, but you still have code that does
> things differently based on a global identification.
I agree with what you are trying to say. But if you see recently we had some
patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
exynos machine files. So only leftover files using these macros are exynos.c
platsmp.c and pm.c.

For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
compatible string in patch 1 of this series. Please let me know if that is OK?

Also for platsmp.c and pm.c I can think of following approaches
1: Keep these macros till we get generic solution?
2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
till we get
generic solution. So that at least we can remove #ifdef  based macros
as soc_is_exynosXYZ.
3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
till we get
generic solution. For some cases where we want to know SoC revision let us
map chipid register and get revision there itself.

Please let me know what approach you think will be good?

[2]: http://thread.gmane.org/gmane.linux.kernel.samsung-soc/29085
[3]: https://lkml.org/lkml/2014/5/2/612
> The only user left in device drivers is now the cpufreq driver,
> which is going to be replaced anyway, so that is ok. Having
> a global variable that is accessible to random device drivers
> is probably not a good idea though, it will just lead to
> bad coding in drivers again.
>
> To give an example of how I think it should really be restructured,
> let's look at one function:
>
> static const struct exynos_wkup_irq exynos4_wkup_irq[] = {
>          { 76, BIT(1) }, /* RTC alarm */
>          { 77, BIT(2) }, /* RTC tick */
>          { /* sentinel */ },
> };
>
> static const struct exynos_wkup_irq exynos5250_wkup_irq[] = {
>          { 75, BIT(1) }, /* RTC alarm */
>          { 76, BIT(2) }, /* RTC tick */
>          { /* sentinel */ },
> };
>
> static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
> {
>          const struct exynos_wkup_irq *wkup_irq;
>
>          if (soc_is_exynos5250())
>                  wkup_irq = exynos5250_wkup_irq;
>          else
>                  wkup_irq = exynos4_wkup_irq;
>
> 	...
> }
>
> There are multiple problems with this code:
>
> - As mentioned, you depend on a specific SoC identification for
>    something that could be done completely generic.
> - The knowledge about what is a wakeup source or not doesn't
>    really belong here. We don't have a DT binding for wakeups
>    as far as I'm aware of, but this should probably be handled
>    locally in the RTC device node, possibly in the node that
>    contains the S5P_WAKEUP_MASK register.
>
> 	Arnd
>


-- 
Best Regards,
Pankaj Dubey


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-05  9:23     ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-05  9:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd,

Thanks for review and suggestions.

On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
>> This patch series attempts to get rid of soc_is_exynosXXXX macros
>> and eventually with the help of this series we can probably get
>> rid of CONFIG_SOC_EXYNOSXXXX in near future.
>> Each Exynos SoC has ChipID block which can give information about
>> SoC's product Id and revision number. Currently we have single
>> DT binding information for this as "samsung,exynos4210-chipid".
>> But Exynos4 and Exynos5 SoC series have one small difference in
>> chip Id, with resepect to product id bit-masks. So it means we
>> should have separate compatible string for these different series
>> of SoCs. So I have created new binding information for handling
>> this difference. Also currently I can think of putting this driver
>> code under "drivers/misc/" but suggestions are welcome.
>> Also current form of driver is missing platfrom driver and needs
>> init function to be called from machine file (either exynos.c or
>> platsmp.c). I hope lot of suggestions and comments to improve this
>> further.
>>
>> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
>> and prepared on top of following patch series and it's dependent
>> patch series.
> I think putting it into drivers/soc would be most appropriate.
> We already have a few drivers lined up that we want in there,
> although the directory currently doesn't exist.
OK. Will move to "drivers/soc". I can see already some patches
have been posted [1] by Andy Gross for creating directory
"drivers/soc" I will use those patches for adding samsung folder
under "drivers/soc".

[1]: https://lkml.org/lkml/2014/4/21/46

>
> However, I would ask that you use the infrastructure provided by
> drivers/base/soc.c when you add this driver, to also make the
> information available to user space using a standard API.

OK. Will update in next version.

>
> Ideally this should be done by slightly restructuring the DT
> source to make all on-chip devices appear below the soc node.

Currently I can't see soc nodes in exynos4 and exynos5 DT files.
So isn't it should be a separate patch first to modify all exynos4
exynos5 DT files to move all devices under soc node?
In that case existing chipid node will be also moved under soc node.

> We'd have to think a bit about how to best do this while
> preserving compatibility with existing dts files.

Is it necessary in this case?
As I have mentioned there is difference in bit-mask among exynos4
and exynos5's chipid. So is this reason not sufficient to keep separate
compatible for both?
Also even if we get some way to preserve existing compatibility, I afraid
in chipid driver that implementation will not look good, at least I am not
able to think of any good way. Any suggestions?

>
> Regarding patch 4, this is not what I meant when I asked for
> removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> using a different interface, but you still have code that does
> things differently based on a global identification.
I agree with what you are trying to say. But if you see recently we had some
patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
exynos machine files. So only leftover files using these macros are exynos.c
platsmp.c and pm.c.

For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
compatible string in patch 1 of this series. Please let me know if that is OK?

Also for platsmp.c and pm.c I can think of following approaches
1: Keep these macros till we get generic solution?
2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
till we get
generic solution. So that at least we can remove #ifdef  based macros
as soc_is_exynosXYZ.
3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
till we get
generic solution. For some cases where we want to know SoC revision let us
map chipid register and get revision there itself.

Please let me know what approach you think will be good?

[2]: http://thread.gmane.org/gmane.linux.kernel.samsung-soc/29085
[3]: https://lkml.org/lkml/2014/5/2/612
> The only user left in device drivers is now the cpufreq driver,
> which is going to be replaced anyway, so that is ok. Having
> a global variable that is accessible to random device drivers
> is probably not a good idea though, it will just lead to
> bad coding in drivers again.
>
> To give an example of how I think it should really be restructured,
> let's look at one function:
>
> static const struct exynos_wkup_irq exynos4_wkup_irq[] = {
>          { 76, BIT(1) }, /* RTC alarm */
>          { 77, BIT(2) }, /* RTC tick */
>          { /* sentinel */ },
> };
>
> static const struct exynos_wkup_irq exynos5250_wkup_irq[] = {
>          { 75, BIT(1) }, /* RTC alarm */
>          { 76, BIT(2) }, /* RTC tick */
>          { /* sentinel */ },
> };
>
> static int exynos_irq_set_wake(struct irq_data *data, unsigned int state)
> {
>          const struct exynos_wkup_irq *wkup_irq;
>
>          if (soc_is_exynos5250())
>                  wkup_irq = exynos5250_wkup_irq;
>          else
>                  wkup_irq = exynos4_wkup_irq;
>
> 	...
> }
>
> There are multiple problems with this code:
>
> - As mentioned, you depend on a specific SoC identification for
>    something that could be done completely generic.
> - The knowledge about what is a wakeup source or not doesn't
>    really belong here. We don't have a DT binding for wakeups
>    as far as I'm aware of, but this should probably be handled
>    locally in the RTC device node, possibly in the node that
>    contains the S5P_WAKEUP_MASK register.
>
> 	Arnd
>


-- 
Best Regards,
Pankaj Dubey

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support
  2014-05-05  7:57     ` Krzysztof Kozlowski
@ 2014-05-05  9:28       ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-05  9:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-samsung-soc, linux-kernel, linux-arm-kernel, kgene.kim,
	linux, Arnd Bergmann, t.figa, Greg Kroah-Hartman

On 05/05/2014 04:57 PM, Krzysztof Kozlowski wrote:
> On sob, 2014-05-03 at 15:11 +0900, Pankaj Dubey wrote:
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 7eb4b69..48c8fb5 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)		+= sram.o
>>   obj-y				+= mic/
>>   obj-$(CONFIG_GENWQE)		+= genwqe/
>>   obj-$(CONFIG_ECHO)		+= echo/
>> +obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
>> diff --git a/drivers/misc/exynos-chipid.c b/drivers/misc/exynos-chipid.c
>> new file mode 100644
>> index 0000000..eb23339
>> --- /dev/null
>> +++ b/drivers/misc/exynos-chipid.c
>> @@ -0,0 +1,83 @@
>> +/*
>> + * Copyright (c) 2014-2015 Samsung Electronics Co., Ltd.
>> + *	      http://www.samsung.com/
> I wonder why this is copyrighted in the future (2015)?

Thanks for pointing out, will correct it in next version.

>
> Best regards,
> Krzysztof
>
>
>


-- 
Best Regards,
Pankaj Dubey


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support
@ 2014-05-05  9:28       ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-05  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/05/2014 04:57 PM, Krzysztof Kozlowski wrote:
> On sob, 2014-05-03 at 15:11 +0900, Pankaj Dubey wrote:
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 7eb4b69..48c8fb5 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)		+= sram.o
>>   obj-y				+= mic/
>>   obj-$(CONFIG_GENWQE)		+= genwqe/
>>   obj-$(CONFIG_ECHO)		+= echo/
>> +obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
>> diff --git a/drivers/misc/exynos-chipid.c b/drivers/misc/exynos-chipid.c
>> new file mode 100644
>> index 0000000..eb23339
>> --- /dev/null
>> +++ b/drivers/misc/exynos-chipid.c
>> @@ -0,0 +1,83 @@
>> +/*
>> + * Copyright (c) 2014-2015 Samsung Electronics Co., Ltd.
>> + *	      http://www.samsung.com/
> I wonder why this is copyrighted in the future (2015)?

Thanks for pointing out, will correct it in next version.

>
> Best regards,
> Krzysztof
>
>
>


-- 
Best Regards,
Pankaj Dubey

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-05  9:23     ` Pankaj Dubey
@ 2014-05-05 14:58       ` Arnd Bergmann
  -1 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-05 14:58 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Pankaj Dubey, kgene.kim, linux, t.figa, linux-kernel, linux-samsung-soc

On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
> On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> > Ideally this should be done by slightly restructuring the DT
> > source to make all on-chip devices appear below the soc node.
> 
> Currently I can't see soc nodes in exynos4 and exynos5 DT files.
> So isn't it should be a separate patch first to modify all exynos4
> exynos5 DT files to move all devices under soc node?
> In that case existing chipid node will be also moved under soc node.

Yes, that would be good. In fact the soc node could be identical
to the chipid node, effectively moving everything under chipid.

> > We'd have to think a bit about how to best do this while
> > preserving compatibility with existing dts files.
> 
> Is it necessary in this case?
> As I have mentioned there is difference in bit-mask among exynos4
> and exynos5's chipid. So is this reason not sufficient to keep separate
> compatible for both?

Having two "compatible" values for exynos4 and exynos5 is not a problem,
and it absolutely makes sense to have more specific values in there
as well:

compatible = "samsung,exynos4210-chipid", "samsung,exynos4-chipid";

> Also even if we get some way to preserve existing compatibility, I afraid
> in chipid driver that implementation will not look good, at least I am not
> able to think of any good way. Any suggestions?

The compatibility I mean is to ensure everything keeps working if
the node is not present.

> > Regarding patch 4, this is not what I meant when I asked for
> > removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> > using a different interface, but you still have code that does
> > things differently based on a global identification.
> I agree with what you are trying to say. But if you see recently we had some
> patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
> exynos machine files. So only leftover files using these macros are exynos.c
> platsmp.c and pm.c.
> 
> For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
> compatible string in patch 1 of this series. Please let me know if that is OK?

I've taken a closer look at that file now. My preferred solution
would be to go back to having two machine descriptors as it
was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
exynos5 machine files", but keep it all in one file and consolidated
as much as possible, e.g.

static void __init exynos_dt_machine_init(void)
{
       exynos_cpuidle_init();
       exynos_cpufreq_init();

       of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}

static void __init exynos5_dt_machine_init(void)
{
       /*
        * Exynos5's legacy i2c controller and new high speed i2c
        * controller have muxed interrupt sources. By default the
        * interrupts for 4-channel HS-I2C controller are enabled.
        * If node for first four channels of legacy i2c controller
        * are available then re-configure the interrupts via the
        * system register.
        */
       struct device_node *i2c_np;
       const char *i2c_compat = "samsung,s3c2440-i2c";
       unsigned int tmp;
       int id;

       for_each_compatible_node(i2c_np, NULL, i2c_compat) {
              if (of_device_is_available(i2c_np)) {
                        id = of_alias_get_id(i2c_np, "i2c");
                        if (id < 4) {
                                  tmp = readl(EXYNOS5_SYS_I2C_CFG);
                                  writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
                        }
               }
       }

       exynos_dt_machine_init();
}

This way you can avoid having another check of the compatible node.
In the long run, all of the this code should go away: The cpuidle
and cpufreq drivers should become normal platform drivers that
get probed when the devices are present (just like it's required
for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
get set up by an appropriate driver, e.g. the i2c driver through
syscon, or a pinmux driver that changes the mux between the
sources based on DT information, whatever fits best.

Similarly for exynos_map_io(), with the sysram out of the picture,
it can be 

void __init exynos4_init_io(void)
{
        debug_ll_io_init();
	iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
}

void __init exynos5_init_io(void)
{
        debug_ll_io_init();
	iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos4_iodesc));
}

but in the long run, it would be nicer to completely eliminate
exynos4_iodesc and exynos5_iodesc as well, and remove the init_io
functions. Note that debug_ll_io_init() is already called when
you have a NULL .map_io callback.

> Also for platsmp.c and pm.c I can think of following approaches
> 1: Keep these macros till we get generic solution?
> 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> till we get
> generic solution. So that at least we can remove #ifdef  based macros
> as soc_is_exynosXYZ.
> 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> till we get
> generic solution. For some cases where we want to know SoC revision let us
> map chipid register and get revision there itself.
> 
> Please let me know what approach you think will be good?

I think 1 or 2 would be better than 3. Between those two, I'm undecided,
but I think either way the SoC specific values would be better kept in the
mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-05 14:58       ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-05 14:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
> On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> > Ideally this should be done by slightly restructuring the DT
> > source to make all on-chip devices appear below the soc node.
> 
> Currently I can't see soc nodes in exynos4 and exynos5 DT files.
> So isn't it should be a separate patch first to modify all exynos4
> exynos5 DT files to move all devices under soc node?
> In that case existing chipid node will be also moved under soc node.

Yes, that would be good. In fact the soc node could be identical
to the chipid node, effectively moving everything under chipid.

> > We'd have to think a bit about how to best do this while
> > preserving compatibility with existing dts files.
> 
> Is it necessary in this case?
> As I have mentioned there is difference in bit-mask among exynos4
> and exynos5's chipid. So is this reason not sufficient to keep separate
> compatible for both?

Having two "compatible" values for exynos4 and exynos5 is not a problem,
and it absolutely makes sense to have more specific values in there
as well:

compatible = "samsung,exynos4210-chipid", "samsung,exynos4-chipid";

> Also even if we get some way to preserve existing compatibility, I afraid
> in chipid driver that implementation will not look good, at least I am not
> able to think of any good way. Any suggestions?

The compatibility I mean is to ensure everything keeps working if
the node is not present.

> > Regarding patch 4, this is not what I meant when I asked for
> > removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> > using a different interface, but you still have code that does
> > things differently based on a global identification.
> I agree with what you are trying to say. But if you see recently we had some
> patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
> exynos machine files. So only leftover files using these macros are exynos.c
> platsmp.c and pm.c.
> 
> For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
> compatible string in patch 1 of this series. Please let me know if that is OK?

I've taken a closer look at that file now. My preferred solution
would be to go back to having two machine descriptors as it
was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
exynos5 machine files", but keep it all in one file and consolidated
as much as possible, e.g.

static void __init exynos_dt_machine_init(void)
{
       exynos_cpuidle_init();
       exynos_cpufreq_init();

       of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}

static void __init exynos5_dt_machine_init(void)
{
       /*
        * Exynos5's legacy i2c controller and new high speed i2c
        * controller have muxed interrupt sources. By default the
        * interrupts for 4-channel HS-I2C controller are enabled.
        * If node for first four channels of legacy i2c controller
        * are available then re-configure the interrupts via the
        * system register.
        */
       struct device_node *i2c_np;
       const char *i2c_compat = "samsung,s3c2440-i2c";
       unsigned int tmp;
       int id;

       for_each_compatible_node(i2c_np, NULL, i2c_compat) {
              if (of_device_is_available(i2c_np)) {
                        id = of_alias_get_id(i2c_np, "i2c");
                        if (id < 4) {
                                  tmp = readl(EXYNOS5_SYS_I2C_CFG);
                                  writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
                        }
               }
       }

       exynos_dt_machine_init();
}

This way you can avoid having another check of the compatible node.
In the long run, all of the this code should go away: The cpuidle
and cpufreq drivers should become normal platform drivers that
get probed when the devices are present (just like it's required
for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
get set up by an appropriate driver, e.g. the i2c driver through
syscon, or a pinmux driver that changes the mux between the
sources based on DT information, whatever fits best.

Similarly for exynos_map_io(), with the sysram out of the picture,
it can be 

void __init exynos4_init_io(void)
{
        debug_ll_io_init();
	iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
}

void __init exynos5_init_io(void)
{
        debug_ll_io_init();
	iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos4_iodesc));
}

but in the long run, it would be nicer to completely eliminate
exynos4_iodesc and exynos5_iodesc as well, and remove the init_io
functions. Note that debug_ll_io_init() is already called when
you have a NULL .map_io callback.

> Also for platsmp.c and pm.c I can think of following approaches
> 1: Keep these macros till we get generic solution?
> 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> till we get
> generic solution. So that at least we can remove #ifdef  based macros
> as soc_is_exynosXYZ.
> 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> till we get
> generic solution. For some cases where we want to know SoC revision let us
> map chipid register and get revision there itself.
> 
> Please let me know what approach you think will be good?

I think 1 or 2 would be better than 3. Between those two, I'm undecided,
but I think either way the SoC specific values would be better kept in the
mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-05 14:58       ` Arnd Bergmann
@ 2014-05-05 15:01         ` Arnd Bergmann
  -1 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-05 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Pankaj Dubey, kgene.kim, linux, t.figa, linux-kernel, linux-samsung-soc

On Monday 05 May 2014 16:58:14 Arnd Bergmann wrote:
> > Also for platsmp.c and pm.c I can think of following approaches
> > 1: Keep these macros till we get generic solution?
> > 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> > till we get
> > generic solution. So that at least we can remove #ifdef  based macros
> > as soc_is_exynosXYZ.
> > 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> > till we get
> > generic solution. For some cases where we want to know SoC revision let us
> > map chipid register and get revision there itself.
> > 
> > Please let me know what approach you think will be good?
> 
> I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> but I think either way the SoC specific values would be better kept in the
> mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

Actually, a good compromise for now would be to add the chipid driver
to mach-exynos instead of drivers/bus. This way you can keep the uses
of the ID local to the exynos platform code until it's no longer needed.
Then it can get moved out to drivers/soc to be shared with arm64.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-05 15:01         ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-05 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 05 May 2014 16:58:14 Arnd Bergmann wrote:
> > Also for platsmp.c and pm.c I can think of following approaches
> > 1: Keep these macros till we get generic solution?
> > 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> > till we get
> > generic solution. So that at least we can remove #ifdef  based macros
> > as soc_is_exynosXYZ.
> > 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> > till we get
> > generic solution. For some cases where we want to know SoC revision let us
> > map chipid register and get revision there itself.
> > 
> > Please let me know what approach you think will be good?
> 
> I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> but I think either way the SoC specific values would be better kept in the
> mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

Actually, a good compromise for now would be to add the chipid driver
to mach-exynos instead of drivers/bus. This way you can keep the uses
of the ID local to the exynos platform code until it's no longer needed.
Then it can get moved out to drivers/soc to be shared with arm64.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-03 15:02   ` Arnd Bergmann
  (?)
@ 2014-05-05 15:34     ` Rob Herring
  -1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2014-05-05 15:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Kukjin Kim, Russell King - ARM Linux,
	Pankaj Dubey, Tomasz Figa, linux-kernel, linux-samsung-soc

On Sat, May 3, 2014 at 10:02 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
>> This patch series attempts to get rid of soc_is_exynosXXXX macros
>> and eventually with the help of this series we can probably get
>> rid of CONFIG_SOC_EXYNOSXXXX in near future.
>> Each Exynos SoC has ChipID block which can give information about
>> SoC's product Id and revision number. Currently we have single
>> DT binding information for this as "samsung,exynos4210-chipid".
>> But Exynos4 and Exynos5 SoC series have one small difference in
>> chip Id, with resepect to product id bit-masks. So it means we
>> should have separate compatible string for these different series
>> of SoCs. So I have created new binding information for handling
>> this difference. Also currently I can think of putting this driver
>> code under "drivers/misc/" but suggestions are welcome.
>> Also current form of driver is missing platfrom driver and needs
>> init function to be called from machine file (either exynos.c or
>> platsmp.c). I hope lot of suggestions and comments to improve this
>> further.
>>
>> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
>> and prepared on top of following patch series and it's dependent
>> patch series.
>
> I think putting it into drivers/soc would be most appropriate.
> We already have a few drivers lined up that we want in there,
> although the directory currently doesn't exist.
>
> However, I would ask that you use the infrastructure provided by
> drivers/base/soc.c when you add this driver, to also make the
> information available to user space using a standard API.

Agreed.

> Ideally this should be done by slightly restructuring the DT
> source to make all on-chip devices appear below the soc node.
> We'd have to think a bit about how to best do this while
> preserving compatibility with existing dts files.

I don't agree. How is a block with chip ID info the parent of all the
other devices?

In doing some work to move default of_platform_populate out of
platforms, I noticed that most platforms using the soc device are
making it the parent of platform devices. I think this is either wrong
or all platforms should have a default soc device. It makes little
sense for some platforms to have a devices under a soc sysfs directory
while others do not. Or the location changes when a platform latter
adds the soc device.

Rob

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-05 15:34     ` Rob Herring
  0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2014-05-05 15:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Kukjin Kim, Russell King - ARM Linux,
	Pankaj Dubey, Tomasz Figa, linux-kernel, linux-samsung-soc

On Sat, May 3, 2014 at 10:02 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
>> This patch series attempts to get rid of soc_is_exynosXXXX macros
>> and eventually with the help of this series we can probably get
>> rid of CONFIG_SOC_EXYNOSXXXX in near future.
>> Each Exynos SoC has ChipID block which can give information about
>> SoC's product Id and revision number. Currently we have single
>> DT binding information for this as "samsung,exynos4210-chipid".
>> But Exynos4 and Exynos5 SoC series have one small difference in
>> chip Id, with resepect to product id bit-masks. So it means we
>> should have separate compatible string for these different series
>> of SoCs. So I have created new binding information for handling
>> this difference. Also currently I can think of putting this driver
>> code under "drivers/misc/" but suggestions are welcome.
>> Also current form of driver is missing platfrom driver and needs
>> init function to be called from machine file (either exynos.c or
>> platsmp.c). I hope lot of suggestions and comments to improve this
>> further.
>>
>> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
>> and prepared on top of following patch series and it's dependent
>> patch series.
>
> I think putting it into drivers/soc would be most appropriate.
> We already have a few drivers lined up that we want in there,
> although the directory currently doesn't exist.
>
> However, I would ask that you use the infrastructure provided by
> drivers/base/soc.c when you add this driver, to also make the
> information available to user space using a standard API.

Agreed.

> Ideally this should be done by slightly restructuring the DT
> source to make all on-chip devices appear below the soc node.
> We'd have to think a bit about how to best do this while
> preserving compatibility with existing dts files.

I don't agree. How is a block with chip ID info the parent of all the
other devices?

In doing some work to move default of_platform_populate out of
platforms, I noticed that most platforms using the soc device are
making it the parent of platform devices. I think this is either wrong
or all platforms should have a default soc device. It makes little
sense for some platforms to have a devices under a soc sysfs directory
while others do not. Or the location changes when a platform latter
adds the soc device.

Rob

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-05 15:34     ` Rob Herring
  0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2014-05-05 15:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, May 3, 2014 at 10:02 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 03 May 2014 15:11:36 Pankaj Dubey wrote:
>> This patch series attempts to get rid of soc_is_exynosXXXX macros
>> and eventually with the help of this series we can probably get
>> rid of CONFIG_SOC_EXYNOSXXXX in near future.
>> Each Exynos SoC has ChipID block which can give information about
>> SoC's product Id and revision number. Currently we have single
>> DT binding information for this as "samsung,exynos4210-chipid".
>> But Exynos4 and Exynos5 SoC series have one small difference in
>> chip Id, with resepect to product id bit-masks. So it means we
>> should have separate compatible string for these different series
>> of SoCs. So I have created new binding information for handling
>> this difference. Also currently I can think of putting this driver
>> code under "drivers/misc/" but suggestions are welcome.
>> Also current form of driver is missing platfrom driver and needs
>> init function to be called from machine file (either exynos.c or
>> platsmp.c). I hope lot of suggestions and comments to improve this
>> further.
>>
>> This patch series is based on Kukjin Kim's for-next (3.14_rc1 tag)
>> and prepared on top of following patch series and it's dependent
>> patch series.
>
> I think putting it into drivers/soc would be most appropriate.
> We already have a few drivers lined up that we want in there,
> although the directory currently doesn't exist.
>
> However, I would ask that you use the infrastructure provided by
> drivers/base/soc.c when you add this driver, to also make the
> information available to user space using a standard API.

Agreed.

> Ideally this should be done by slightly restructuring the DT
> source to make all on-chip devices appear below the soc node.
> We'd have to think a bit about how to best do this while
> preserving compatibility with existing dts files.

I don't agree. How is a block with chip ID info the parent of all the
other devices?

In doing some work to move default of_platform_populate out of
platforms, I noticed that most platforms using the soc device are
making it the parent of platform devices. I think this is either wrong
or all platforms should have a default soc device. It makes little
sense for some platforms to have a devices under a soc sysfs directory
while others do not. Or the location changes when a platform latter
adds the soc device.

Rob

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-05 14:58       ` Arnd Bergmann
@ 2014-05-06  6:57         ` Pankaj Dubey
  -1 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-06  6:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, kgene.kim, linux, t.figa, linux-kernel,
	linux-samsung-soc

On 05/05/2014 11:58 PM, Arnd Bergmann wrote:
> On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
>> On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
>>> Ideally this should be done by slightly restructuring the DT
>>> source to make all on-chip devices appear below the soc node.
>> Currently I can't see soc nodes in exynos4 and exynos5 DT files.
>> So isn't it should be a separate patch first to modify all exynos4
>> exynos5 DT files to move all devices under soc node?
>> In that case existing chipid node will be also moved under soc node.
> Yes, that would be good. In fact the soc node could be identical
> to the chipid node, effectively moving everything under chipid.

OK, in that case I would like to keep this as separate patch once
I do all other modifications.

>>> We'd have to think a bit about how to best do this while
>>> preserving compatibility with existing dts files.
>> Is it necessary in this case?
>> As I have mentioned there is difference in bit-mask among exynos4
>> and exynos5's chipid. So is this reason not sufficient to keep separate
>> compatible for both?
> Having two "compatible" values for exynos4 and exynos5 is not a problem,
> and it absolutely makes sense to have more specific values in there
> as well:
>
> compatible = "samsung,exynos4210-chipid", "samsung,exynos4-chipid";

OK, will keep compatible as you suggested.

>
>> Also even if we get some way to preserve existing compatibility, I afraid
>> in chipid driver that implementation will not look good, at least I am not
>> able to think of any good way. Any suggestions?
> The compatibility I mean is to ensure everything keeps working if
> the node is not present.
>
>>> Regarding patch 4, this is not what I meant when I asked for
>>> removing the soc_is_exynos* macros. You basically do a 1:1 replacement
>>> using a different interface, but you still have code that does
>>> things differently based on a global identification.
>> I agree with what you are trying to say. But if you see recently we had some
>> patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
>> exynos machine files. So only leftover files using these macros are exynos.c
>> platsmp.c and pm.c.
>>
>> For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
>> compatible string in patch 1 of this series. Please let me know if that is OK?
> I've taken a closer look at that file now. My preferred solution
> would be to go back to having two machine descriptors as it
> was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
> exynos5 machine files", but keep it all in one file and consolidated
> as much as possible, e.g.

Yes, that case I do not need to add another function to compare compatible 
strings.
So if there is no issues in having two separate machine descriptor I will 
do this
modification in next version of patch.

>
> static void __init exynos_dt_machine_init(void)
> {
>         exynos_cpuidle_init();
>         exynos_cpufreq_init();
>
>         of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
>
> static void __init exynos5_dt_machine_init(void)
> {
>         /*
>          * Exynos5's legacy i2c controller and new high speed i2c
>          * controller have muxed interrupt sources. By default the
>          * interrupts for 4-channel HS-I2C controller are enabled.
>          * If node for first four channels of legacy i2c controller
>          * are available then re-configure the interrupts via the
>          * system register.
>          */
>         struct device_node *i2c_np;
>         const char *i2c_compat = "samsung,s3c2440-i2c";
>         unsigned int tmp;
>         int id;
>
>         for_each_compatible_node(i2c_np, NULL, i2c_compat) {
>                if (of_device_is_available(i2c_np)) {
>                          id = of_alias_get_id(i2c_np, "i2c");
>                          if (id < 4) {
>                                    tmp = readl(EXYNOS5_SYS_I2C_CFG);
>                                    writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
>                          }
>                 }
>         }
>
>         exynos_dt_machine_init();
> }
>
> This way you can avoid having another check of the compatible node.
> In the long run, all of the this code should go away: The cpuidle
> and cpufreq drivers should become normal platform drivers that
> get probed when the devices are present (just like it's required
> for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
> get set up by an appropriate driver, e.g. the i2c driver through
> syscon, or a pinmux driver that changes the mux between the
> sources based on DT information, whatever fits best.

OK, will move this in i2c driver and will use sysreg as syscon phandle.

>
> Similarly for exynos_map_io(), with the sysram out of the picture,
> it can be
>
> void __init exynos4_init_io(void)
> {
>          debug_ll_io_init();
> 	iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
>
> void __init exynos5_init_io(void)
> {
>          debug_ll_io_init();
> 	iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
>
> but in the long run, it would be nicer to completely eliminate
> exynos4_iodesc and exynos5_iodesc as well, and remove the init_io
> functions. Note that debug_ll_io_init() is already called when
> you have a NULL .map_io callback.

Agreed.

>
>> Also for platsmp.c and pm.c I can think of following approaches
>> 1: Keep these macros till we get generic solution?
>> 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions
>> till we get
>> generic solution. So that at least we can remove #ifdef  based macros
>> as soc_is_exynosXYZ.
>> 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files
>> till we get
>> generic solution. For some cases where we want to know SoC revision let us
>> map chipid register and get revision there itself.
>>
>> Please let me know what approach you think will be good?
> I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> but I think either way the SoC specific values would be better kept in the
> mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

OK, let me introduce this driver via "drivers/soc" in second revision,
there also if we think it's not proper to expose such APIs or variable
outside of the driver, I will be think to move it in under machine 
directory itself.

> 	Arnd
>


-- 
Best Regards,
Pankaj Dubey


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-06  6:57         ` Pankaj Dubey
  0 siblings, 0 replies; 36+ messages in thread
From: Pankaj Dubey @ 2014-05-06  6:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/05/2014 11:58 PM, Arnd Bergmann wrote:
> On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
>> On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
>>> Ideally this should be done by slightly restructuring the DT
>>> source to make all on-chip devices appear below the soc node.
>> Currently I can't see soc nodes in exynos4 and exynos5 DT files.
>> So isn't it should be a separate patch first to modify all exynos4
>> exynos5 DT files to move all devices under soc node?
>> In that case existing chipid node will be also moved under soc node.
> Yes, that would be good. In fact the soc node could be identical
> to the chipid node, effectively moving everything under chipid.

OK, in that case I would like to keep this as separate patch once
I do all other modifications.

>>> We'd have to think a bit about how to best do this while
>>> preserving compatibility with existing dts files.
>> Is it necessary in this case?
>> As I have mentioned there is difference in bit-mask among exynos4
>> and exynos5's chipid. So is this reason not sufficient to keep separate
>> compatible for both?
> Having two "compatible" values for exynos4 and exynos5 is not a problem,
> and it absolutely makes sense to have more specific values in there
> as well:
>
> compatible = "samsung,exynos4210-chipid", "samsung,exynos4-chipid";

OK, will keep compatible as you suggested.

>
>> Also even if we get some way to preserve existing compatibility, I afraid
>> in chipid driver that implementation will not look good, at least I am not
>> able to think of any good way. Any suggestions?
> The compatibility I mean is to ensure everything keeps working if
> the node is not present.
>
>>> Regarding patch 4, this is not what I meant when I asked for
>>> removing the soc_is_exynos* macros. You basically do a 1:1 replacement
>>> using a different interface, but you still have code that does
>>> things differently based on a global identification.
>> I agree with what you are trying to say. But if you see recently we had some
>> patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
>> exynos machine files. So only leftover files using these macros are exynos.c
>> platsmp.c and pm.c.
>>
>> For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
>> compatible string in patch 1 of this series. Please let me know if that is OK?
> I've taken a closer look at that file now. My preferred solution
> would be to go back to having two machine descriptors as it
> was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
> exynos5 machine files", but keep it all in one file and consolidated
> as much as possible, e.g.

Yes, that case I do not need to add another function to compare compatible 
strings.
So if there is no issues in having two separate machine descriptor I will 
do this
modification in next version of patch.

>
> static void __init exynos_dt_machine_init(void)
> {
>         exynos_cpuidle_init();
>         exynos_cpufreq_init();
>
>         of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
>
> static void __init exynos5_dt_machine_init(void)
> {
>         /*
>          * Exynos5's legacy i2c controller and new high speed i2c
>          * controller have muxed interrupt sources. By default the
>          * interrupts for 4-channel HS-I2C controller are enabled.
>          * If node for first four channels of legacy i2c controller
>          * are available then re-configure the interrupts via the
>          * system register.
>          */
>         struct device_node *i2c_np;
>         const char *i2c_compat = "samsung,s3c2440-i2c";
>         unsigned int tmp;
>         int id;
>
>         for_each_compatible_node(i2c_np, NULL, i2c_compat) {
>                if (of_device_is_available(i2c_np)) {
>                          id = of_alias_get_id(i2c_np, "i2c");
>                          if (id < 4) {
>                                    tmp = readl(EXYNOS5_SYS_I2C_CFG);
>                                    writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
>                          }
>                 }
>         }
>
>         exynos_dt_machine_init();
> }
>
> This way you can avoid having another check of the compatible node.
> In the long run, all of the this code should go away: The cpuidle
> and cpufreq drivers should become normal platform drivers that
> get probed when the devices are present (just like it's required
> for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
> get set up by an appropriate driver, e.g. the i2c driver through
> syscon, or a pinmux driver that changes the mux between the
> sources based on DT information, whatever fits best.

OK, will move this in i2c driver and will use sysreg as syscon phandle.

>
> Similarly for exynos_map_io(), with the sysram out of the picture,
> it can be
>
> void __init exynos4_init_io(void)
> {
>          debug_ll_io_init();
> 	iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
>
> void __init exynos5_init_io(void)
> {
>          debug_ll_io_init();
> 	iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
>
> but in the long run, it would be nicer to completely eliminate
> exynos4_iodesc and exynos5_iodesc as well, and remove the init_io
> functions. Note that debug_ll_io_init() is already called when
> you have a NULL .map_io callback.

Agreed.

>
>> Also for platsmp.c and pm.c I can think of following approaches
>> 1: Keep these macros till we get generic solution?
>> 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions
>> till we get
>> generic solution. So that at least we can remove #ifdef  based macros
>> as soc_is_exynosXYZ.
>> 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files
>> till we get
>> generic solution. For some cases where we want to know SoC revision let us
>> map chipid register and get revision there itself.
>>
>> Please let me know what approach you think will be good?
> I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> but I think either way the SoC specific values would be better kept in the
> mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

OK, let me introduce this driver via "drivers/soc" in second revision,
there also if we think it's not proper to expose such APIs or variable
outside of the driver, I will be think to move it in under machine 
directory itself.

> 	Arnd
>


-- 
Best Regards,
Pankaj Dubey

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-05 15:34     ` Rob Herring
  (?)
@ 2014-05-06  7:22       ` Arnd Bergmann
  -1 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-06  7:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, Kukjin Kim, Russell King - ARM Linux,
	Pankaj Dubey, Tomasz Figa, linux-kernel, linux-samsung-soc

On Monday 05 May 2014 10:34:02 Rob Herring wrote:
> 
> > Ideally this should be done by slightly restructuring the DT
> > source to make all on-chip devices appear below the soc node.
> > We'd have to think a bit about how to best do this while
> > preserving compatibility with existing dts files.
> 
> I don't agree. How is a block with chip ID info the parent of all the
> other devices?
> 
> In doing some work to move default of_platform_populate out of
> platforms, I noticed that most platforms using the soc device are
> making it the parent of platform devices. I think this is either wrong
> or all platforms should have a default soc device. It makes little
> sense for some platforms to have a devices under a soc sysfs directory
> while others do not. Or the location changes when a platform latter
> adds the soc device.

We had a long discussion about this when we introduced the drivers/soc
framework. The intention is that the /sys/devices/soc* node is meant to
describe the SoC in its entirety, the same way you have a pci0000:00 node
as the root of all PCI devices of the first pci host bridge on a PC
system. This also reflects the reality of a SoC, which normally has one
bus that the CPU is connected to and that has all the other devices as
children.

Having the chipid registers as part of the top-level bus should not
be interpreted as having the other devices as children of the chipid
device, but rather the chipid registers as a property of the soc itself
as opposed to a random device that happens to be part of the soc.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-06  7:22       ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-06  7:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, Kukjin Kim, Russell King - ARM Linux,
	Pankaj Dubey, Tomasz Figa, linux-kernel, linux-samsung-soc

On Monday 05 May 2014 10:34:02 Rob Herring wrote:
> 
> > Ideally this should be done by slightly restructuring the DT
> > source to make all on-chip devices appear below the soc node.
> > We'd have to think a bit about how to best do this while
> > preserving compatibility with existing dts files.
> 
> I don't agree. How is a block with chip ID info the parent of all the
> other devices?
> 
> In doing some work to move default of_platform_populate out of
> platforms, I noticed that most platforms using the soc device are
> making it the parent of platform devices. I think this is either wrong
> or all platforms should have a default soc device. It makes little
> sense for some platforms to have a devices under a soc sysfs directory
> while others do not. Or the location changes when a platform latter
> adds the soc device.

We had a long discussion about this when we introduced the drivers/soc
framework. The intention is that the /sys/devices/soc* node is meant to
describe the SoC in its entirety, the same way you have a pci0000:00 node
as the root of all PCI devices of the first pci host bridge on a PC
system. This also reflects the reality of a SoC, which normally has one
bus that the CPU is connected to and that has all the other devices as
children.

Having the chipid registers as part of the top-level bus should not
be interpreted as having the other devices as children of the chipid
device, but rather the chipid registers as a property of the soc itself
as opposed to a random device that happens to be part of the soc.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-06  7:22       ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-06  7:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 05 May 2014 10:34:02 Rob Herring wrote:
> 
> > Ideally this should be done by slightly restructuring the DT
> > source to make all on-chip devices appear below the soc node.
> > We'd have to think a bit about how to best do this while
> > preserving compatibility with existing dts files.
> 
> I don't agree. How is a block with chip ID info the parent of all the
> other devices?
> 
> In doing some work to move default of_platform_populate out of
> platforms, I noticed that most platforms using the soc device are
> making it the parent of platform devices. I think this is either wrong
> or all platforms should have a default soc device. It makes little
> sense for some platforms to have a devices under a soc sysfs directory
> while others do not. Or the location changes when a platform latter
> adds the soc device.

We had a long discussion about this when we introduced the drivers/soc
framework. The intention is that the /sys/devices/soc* node is meant to
describe the SoC in its entirety, the same way you have a pci0000:00 node
as the root of all PCI devices of the first pci host bridge on a PC
system. This also reflects the reality of a SoC, which normally has one
bus that the CPU is connected to and that has all the other devices as
children.

Having the chipid registers as part of the top-level bus should not
be interpreted as having the other devices as children of the chipid
device, but rather the chipid registers as a property of the soc itself
as opposed to a random device that happens to be part of the soc.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-06  6:57         ` Pankaj Dubey
@ 2014-05-06  7:24           ` Arnd Bergmann
  -1 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-06  7:24 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Pankaj Dubey, linux-samsung-soc, linux, t.figa, linux-kernel, kgene.kim

On Tuesday 06 May 2014 15:57:24 Pankaj Dubey wrote:
> On 05/05/2014 11:58 PM, Arnd Bergmann wrote:
> > On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
> >> On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> >>> Ideally this should be done by slightly restructuring the DT
> >>> source to make all on-chip devices appear below the soc node.
> >> Currently I can't see soc nodes in exynos4 and exynos5 DT files.
> >> So isn't it should be a separate patch first to modify all exynos4
> >> exynos5 DT files to move all devices under soc node?
> >> In that case existing chipid node will be also moved under soc node.
> > Yes, that would be good. In fact the soc node could be identical
> > to the chipid node, effectively moving everything under chipid.
> 
> OK, in that case I would like to keep this as separate patch once
> I do all other modifications.

Yes, makes sense. Let's see if we can convince Rob first though, he
has some reservations.

> >> Also even if we get some way to preserve existing compatibility, I afraid
> >> in chipid driver that implementation will not look good, at least I am not
> >> able to think of any good way. Any suggestions?
> > The compatibility I mean is to ensure everything keeps working if
> > the node is not present.
> >
> >>> Regarding patch 4, this is not what I meant when I asked for
> >>> removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> >>> using a different interface, but you still have code that does
> >>> things differently based on a global identification.
> >> I agree with what you are trying to say. But if you see recently we had some
> >> patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
> >> exynos machine files. So only leftover files using these macros are exynos.c
> >> platsmp.c and pm.c.
> >>
> >> For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
> >> compatible string in patch 1 of this series. Please let me know if that is OK?
> > I've taken a closer look at that file now. My preferred solution
> > would be to go back to having two machine descriptors as it
> > was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
> > exynos5 machine files", but keep it all in one file and consolidated
> > as much as possible, e.g.
> 
> Yes, that case I do not need to add another function to compare compatible 
> strings.
> So if there is no issues in having two separate machine descriptor I will 
> do this
> modification in next version of patch.

ok.

> > static void __init exynos_dt_machine_init(void)
> > {
> >         exynos_cpuidle_init();
> >         exynos_cpufreq_init();
> >
> >         of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > }
> >
> > static void __init exynos5_dt_machine_init(void)
> > {
> >         /*
> >          * Exynos5's legacy i2c controller and new high speed i2c
> >          * controller have muxed interrupt sources. By default the
> >          * interrupts for 4-channel HS-I2C controller are enabled.
> >          * If node for first four channels of legacy i2c controller
> >          * are available then re-configure the interrupts via the
> >          * system register.
> >          */
> >         struct device_node *i2c_np;
> >         const char *i2c_compat = "samsung,s3c2440-i2c";
> >         unsigned int tmp;
> >         int id;
> >
> >         for_each_compatible_node(i2c_np, NULL, i2c_compat) {
> >                if (of_device_is_available(i2c_np)) {
> >                          id = of_alias_get_id(i2c_np, "i2c");
> >                          if (id < 4) {
> >                                    tmp = readl(EXYNOS5_SYS_I2C_CFG);
> >                                    writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
> >                          }
> >                 }
> >         }
> >
> >         exynos_dt_machine_init();
> > }
> >
> > This way you can avoid having another check of the compatible node.
> > In the long run, all of the this code should go away: The cpuidle
> > and cpufreq drivers should become normal platform drivers that
> > get probed when the devices are present (just like it's required
> > for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
> > get set up by an appropriate driver, e.g. the i2c driver through
> > syscon, or a pinmux driver that changes the mux between the
> > sources based on DT information, whatever fits best.
> 
> OK, will move this in i2c driver and will use sysreg as syscon phandle.

Ok, cool.

> >> Also for platsmp.c and pm.c I can think of following approaches
> >> 1: Keep these macros till we get generic solution?
> >> 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions
> >> till we get
> >> generic solution. So that at least we can remove #ifdef  based macros
> >> as soc_is_exynosXYZ.
> >> 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files
> >> till we get
> >> generic solution. For some cases where we want to know SoC revision let us
> >> map chipid register and get revision there itself.
> >>
> >> Please let me know what approach you think will be good?
> > I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> > but I think either way the SoC specific values would be better kept in the
> > mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.
> 
> OK, let me introduce this driver via "drivers/soc" in second revision,
> there also if we think it's not proper to expose such APIs or variable
> outside of the driver, I will be think to move it in under machine 
> directory itself.

Ok.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-06  7:24           ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-06  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 06 May 2014 15:57:24 Pankaj Dubey wrote:
> On 05/05/2014 11:58 PM, Arnd Bergmann wrote:
> > On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
> >> On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> >>> Ideally this should be done by slightly restructuring the DT
> >>> source to make all on-chip devices appear below the soc node.
> >> Currently I can't see soc nodes in exynos4 and exynos5 DT files.
> >> So isn't it should be a separate patch first to modify all exynos4
> >> exynos5 DT files to move all devices under soc node?
> >> In that case existing chipid node will be also moved under soc node.
> > Yes, that would be good. In fact the soc node could be identical
> > to the chipid node, effectively moving everything under chipid.
> 
> OK, in that case I would like to keep this as separate patch once
> I do all other modifications.

Yes, makes sense. Let's see if we can convince Rob first though, he
has some reservations.

> >> Also even if we get some way to preserve existing compatibility, I afraid
> >> in chipid driver that implementation will not look good, at least I am not
> >> able to think of any good way. Any suggestions?
> > The compatibility I mean is to ensure everything keeps working if
> > the node is not present.
> >
> >>> Regarding patch 4, this is not what I meant when I asked for
> >>> removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> >>> using a different interface, but you still have code that does
> >>> things differently based on a global identification.
> >> I agree with what you are trying to say. But if you see recently we had some
> >> patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
> >> exynos machine files. So only leftover files using these macros are exynos.c
> >> platsmp.c and pm.c.
> >>
> >> For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
> >> compatible string in patch 1 of this series. Please let me know if that is OK?
> > I've taken a closer look at that file now. My preferred solution
> > would be to go back to having two machine descriptors as it
> > was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
> > exynos5 machine files", but keep it all in one file and consolidated
> > as much as possible, e.g.
> 
> Yes, that case I do not need to add another function to compare compatible 
> strings.
> So if there is no issues in having two separate machine descriptor I will 
> do this
> modification in next version of patch.

ok.

> > static void __init exynos_dt_machine_init(void)
> > {
> >         exynos_cpuidle_init();
> >         exynos_cpufreq_init();
> >
> >         of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> > }
> >
> > static void __init exynos5_dt_machine_init(void)
> > {
> >         /*
> >          * Exynos5's legacy i2c controller and new high speed i2c
> >          * controller have muxed interrupt sources. By default the
> >          * interrupts for 4-channel HS-I2C controller are enabled.
> >          * If node for first four channels of legacy i2c controller
> >          * are available then re-configure the interrupts via the
> >          * system register.
> >          */
> >         struct device_node *i2c_np;
> >         const char *i2c_compat = "samsung,s3c2440-i2c";
> >         unsigned int tmp;
> >         int id;
> >
> >         for_each_compatible_node(i2c_np, NULL, i2c_compat) {
> >                if (of_device_is_available(i2c_np)) {
> >                          id = of_alias_get_id(i2c_np, "i2c");
> >                          if (id < 4) {
> >                                    tmp = readl(EXYNOS5_SYS_I2C_CFG);
> >                                    writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
> >                          }
> >                 }
> >         }
> >
> >         exynos_dt_machine_init();
> > }
> >
> > This way you can avoid having another check of the compatible node.
> > In the long run, all of the this code should go away: The cpuidle
> > and cpufreq drivers should become normal platform drivers that
> > get probed when the devices are present (just like it's required
> > for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
> > get set up by an appropriate driver, e.g. the i2c driver through
> > syscon, or a pinmux driver that changes the mux between the
> > sources based on DT information, whatever fits best.
> 
> OK, will move this in i2c driver and will use sysreg as syscon phandle.

Ok, cool.

> >> Also for platsmp.c and pm.c I can think of following approaches
> >> 1: Keep these macros till we get generic solution?
> >> 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions
> >> till we get
> >> generic solution. So that at least we can remove #ifdef  based macros
> >> as soc_is_exynosXYZ.
> >> 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files
> >> till we get
> >> generic solution. For some cases where we want to know SoC revision let us
> >> map chipid register and get revision there itself.
> >>
> >> Please let me know what approach you think will be good?
> > I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> > but I think either way the SoC specific values would be better kept in the
> > mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.
> 
> OK, let me introduce this driver via "drivers/soc" in second revision,
> there also if we think it's not proper to expose such APIs or variable
> outside of the driver, I will be think to move it in under machine 
> directory itself.

Ok.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-05 14:58       ` Arnd Bergmann
@ 2014-05-12  1:47         ` Olof Johansson
  -1 siblings, 0 replies; 36+ messages in thread
From: Olof Johansson @ 2014-05-12  1:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Pankaj Dubey, kgene.kim, linux, t.figa,
	linux-kernel, linux-samsung-soc

(Taking the discussion here since Panjak pointed me to this thread when
I commented on the latest patch set)

On Mon, May 05, 2014 at 04:58:14PM +0200, Arnd Bergmann wrote:
> On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
> > On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> > > Ideally this should be done by slightly restructuring the DT
> > > source to make all on-chip devices appear below the soc node.
> > 
> > Currently I can't see soc nodes in exynos4 and exynos5 DT files.
> > So isn't it should be a separate patch first to modify all exynos4
> > exynos5 DT files to move all devices under soc node?
> > In that case existing chipid node will be also moved under soc node.
> 
> Yes, that would be good. In fact the soc node could be identical
> to the chipid node, effectively moving everything under chipid.
> 
> > > We'd have to think a bit about how to best do this while
> > > preserving compatibility with existing dts files.
> > 
> > Is it necessary in this case?
> > As I have mentioned there is difference in bit-mask among exynos4
> > and exynos5's chipid. So is this reason not sufficient to keep separate
> > compatible for both?
> 
> Having two "compatible" values for exynos4 and exynos5 is not a problem,
> and it absolutely makes sense to have more specific values in there
> as well:
> 
> compatible = "samsung,exynos4210-chipid", "samsung,exynos4-chipid";

Actually, "generic" compatibles for a device isn't the right thing to do
here. For machine-level compat it makes sense, but not for devices. There
it should instead use the first version of the IP block.

> > Also even if we get some way to preserve existing compatibility, I afraid
> > in chipid driver that implementation will not look good, at least I am not
> > able to think of any good way. Any suggestions?
> 
> The compatibility I mean is to ensure everything keeps working if
> the node is not present.
> 
> > > Regarding patch 4, this is not what I meant when I asked for
> > > removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> > > using a different interface, but you still have code that does
> > > things differently based on a global identification.
> > I agree with what you are trying to say. But if you see recently we had some
> > patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
> > exynos machine files. So only leftover files using these macros are exynos.c
> > platsmp.c and pm.c.
> > 
> > For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
> > compatible string in patch 1 of this series. Please let me know if that is OK?
> 
> I've taken a closer look at that file now. My preferred solution
> would be to go back to having two machine descriptors as it
> was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
> exynos5 machine files", but keep it all in one file and consolidated
> as much as possible, e.g.
> 
> static void __init exynos_dt_machine_init(void)
> {
>        exynos_cpuidle_init();
>        exynos_cpufreq_init();
> 
>        of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
> 
> static void __init exynos5_dt_machine_init(void)
> {
>        /*
>         * Exynos5's legacy i2c controller and new high speed i2c
>         * controller have muxed interrupt sources. By default the
>         * interrupts for 4-channel HS-I2C controller are enabled.
>         * If node for first four channels of legacy i2c controller
>         * are available then re-configure the interrupts via the
>         * system register.
>         */
>        struct device_node *i2c_np;
>        const char *i2c_compat = "samsung,s3c2440-i2c";
>        unsigned int tmp;
>        int id;
> 
>        for_each_compatible_node(i2c_np, NULL, i2c_compat) {
>               if (of_device_is_available(i2c_np)) {
>                         id = of_alias_get_id(i2c_np, "i2c");
>                         if (id < 4) {
>                                   tmp = readl(EXYNOS5_SYS_I2C_CFG);
>                                   writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
>                         }
>                }
>        }
> 
>        exynos_dt_machine_init();
> }
> 
> This way you can avoid having another check of the compatible node.
> In the long run, all of the this code should go away: The cpuidle
> and cpufreq drivers should become normal platform drivers that
> get probed when the devices are present (just like it's required
> for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
> get set up by an appropriate driver, e.g. the i2c driver through
> syscon, or a pinmux driver that changes the mux between the
> sources based on DT information, whatever fits best.
> 
> Similarly for exynos_map_io(), with the sysram out of the picture,
> it can be 
> 
> void __init exynos4_init_io(void)
> {
>         debug_ll_io_init();
> 	iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
> 
> void __init exynos5_init_io(void)
> {
>         debug_ll_io_init();
> 	iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
> 
> but in the long run, it would be nicer to completely eliminate
> exynos4_iodesc and exynos5_iodesc as well, and remove the init_io
> functions. Note that debug_ll_io_init() is already called when
> you have a NULL .map_io callback.
> 
> > Also for platsmp.c and pm.c I can think of following approaches
> > 1: Keep these macros till we get generic solution?
> > 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> > till we get
> > generic solution. So that at least we can remove #ifdef  based macros
> > as soc_is_exynosXYZ.
> > 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> > till we get
> > generic solution. For some cases where we want to know SoC revision let us
> > map chipid register and get revision there itself.
> > 
> > Please let me know what approach you think will be good?
> 
> I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> but I think either way the SoC specific values would be better kept in the
> mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

The generic solution is already there: of_machine_is_compatible() is perfectly
sensible to use for _some_ of these inits. Cpufreq is one of the few that comes
to mind, and maybe some of the platsmp and pm stuff.

Note that none of them should be used in runtime, i.e. you should only use them
at probe/setup time and maybe have a local state in the driver if needed.

I'd rather get people used to that format than everybody needing to implement
a chipid driver now too, especially on platforms that might not even have a
suitable chipid block to base a driver around -- not to mention having to
fill the namespace with is_soc_*() stuff.


-Olof


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-12  1:47         ` Olof Johansson
  0 siblings, 0 replies; 36+ messages in thread
From: Olof Johansson @ 2014-05-12  1:47 UTC (permalink / raw)
  To: linux-arm-kernel

(Taking the discussion here since Panjak pointed me to this thread when
I commented on the latest patch set)

On Mon, May 05, 2014 at 04:58:14PM +0200, Arnd Bergmann wrote:
> On Monday 05 May 2014 18:23:55 Pankaj Dubey wrote:
> > On 05/04/2014 12:02 AM, Arnd Bergmann wrote:
> > > Ideally this should be done by slightly restructuring the DT
> > > source to make all on-chip devices appear below the soc node.
> > 
> > Currently I can't see soc nodes in exynos4 and exynos5 DT files.
> > So isn't it should be a separate patch first to modify all exynos4
> > exynos5 DT files to move all devices under soc node?
> > In that case existing chipid node will be also moved under soc node.
> 
> Yes, that would be good. In fact the soc node could be identical
> to the chipid node, effectively moving everything under chipid.
> 
> > > We'd have to think a bit about how to best do this while
> > > preserving compatibility with existing dts files.
> > 
> > Is it necessary in this case?
> > As I have mentioned there is difference in bit-mask among exynos4
> > and exynos5's chipid. So is this reason not sufficient to keep separate
> > compatible for both?
> 
> Having two "compatible" values for exynos4 and exynos5 is not a problem,
> and it absolutely makes sense to have more specific values in there
> as well:
> 
> compatible = "samsung,exynos4210-chipid", "samsung,exynos4-chipid";

Actually, "generic" compatibles for a device isn't the right thing to do
here. For machine-level compat it makes sense, but not for devices. There
it should instead use the first version of the IP block.

> > Also even if we get some way to preserve existing compatibility, I afraid
> > in chipid driver that implementation will not look good, at least I am not
> > able to think of any good way. Any suggestions?
> 
> The compatibility I mean is to ensure everything keeps working if
> the node is not present.
> 
> > > Regarding patch 4, this is not what I meant when I asked for
> > > removing the soc_is_exynos* macros. You basically do a 1:1 replacement
> > > using a different interface, but you still have code that does
> > > things differently based on a global identification.
> > I agree with what you are trying to say. But if you see recently we had some
> > patches (cpu_idle.c: [2], pmu.c: [3])  to remove usage of such macros from
> > exynos machine files. So only leftover files using these macros are exynos.c
> > platsmp.c and pm.c.
> > 
> > For exynos.c I have tried to remove soc_is_exynos4/exynos5 by matching with
> > compatible string in patch 1 of this series. Please let me know if that is OK?
> 
> I've taken a closer look at that file now. My preferred solution
> would be to go back to having two machine descriptors as it
> was before Sachin Kamat's "ARM: EXYNOS: Consolidate exynos4 and
> exynos5 machine files", but keep it all in one file and consolidated
> as much as possible, e.g.
> 
> static void __init exynos_dt_machine_init(void)
> {
>        exynos_cpuidle_init();
>        exynos_cpufreq_init();
> 
>        of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
> 
> static void __init exynos5_dt_machine_init(void)
> {
>        /*
>         * Exynos5's legacy i2c controller and new high speed i2c
>         * controller have muxed interrupt sources. By default the
>         * interrupts for 4-channel HS-I2C controller are enabled.
>         * If node for first four channels of legacy i2c controller
>         * are available then re-configure the interrupts via the
>         * system register.
>         */
>        struct device_node *i2c_np;
>        const char *i2c_compat = "samsung,s3c2440-i2c";
>        unsigned int tmp;
>        int id;
> 
>        for_each_compatible_node(i2c_np, NULL, i2c_compat) {
>               if (of_device_is_available(i2c_np)) {
>                         id = of_alias_get_id(i2c_np, "i2c");
>                         if (id < 4) {
>                                   tmp = readl(EXYNOS5_SYS_I2C_CFG);
>                                   writel(tmp & ~(0x1 << id),  EXYNOS5_SYS_I2C_CFG);
>                         }
>                }
>        }
> 
>        exynos_dt_machine_init();
> }
> 
> This way you can avoid having another check of the compatible node.
> In the long run, all of the this code should go away: The cpuidle
> and cpufreq drivers should become normal platform drivers that
> get probed when the devices are present (just like it's required
> for arm64 anyway), and the EXYNOS5_SYS_I2C_CFG register should
> get set up by an appropriate driver, e.g. the i2c driver through
> syscon, or a pinmux driver that changes the mux between the
> sources based on DT information, whatever fits best.
> 
> Similarly for exynos_map_io(), with the sysram out of the picture,
> it can be 
> 
> void __init exynos4_init_io(void)
> {
>         debug_ll_io_init();
> 	iotable_init(exynos4_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
> 
> void __init exynos5_init_io(void)
> {
>         debug_ll_io_init();
> 	iotable_init(exynos5_iodesc, ARRAY_SIZE(exynos4_iodesc));
> }
> 
> but in the long run, it would be nicer to completely eliminate
> exynos4_iodesc and exynos5_iodesc as well, and remove the init_io
> functions. Note that debug_ll_io_init() is already called when
> you have a NULL .map_io callback.
> 
> > Also for platsmp.c and pm.c I can think of following approaches
> > 1: Keep these macros till we get generic solution?
> > 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> > till we get
> > generic solution. So that at least we can remove #ifdef  based macros
> > as soc_is_exynosXYZ.
> > 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> > till we get
> > generic solution. For some cases where we want to know SoC revision let us
> > map chipid register and get revision there itself.
> > 
> > Please let me know what approach you think will be good?
> 
> I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> but I think either way the SoC specific values would be better kept in the
> mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.

The generic solution is already there: of_machine_is_compatible() is perfectly
sensible to use for _some_ of these inits. Cpufreq is one of the few that comes
to mind, and maybe some of the platsmp and pm stuff.

Note that none of them should be used in runtime, i.e. you should only use them
at probe/setup time and maybe have a local state in the driver if needed.

I'd rather get people used to that format than everybody needing to implement
a chipid driver now too, especially on platforms that might not even have a
suitable chipid block to base a driver around -- not to mention having to
fill the namespace with is_soc_*() stuff.


-Olof

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 0/4] Introducing Exynos ChipId driver
  2014-05-12  1:47         ` Olof Johansson
@ 2014-05-12  9:47           ` Arnd Bergmann
  -1 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-12  9:47 UTC (permalink / raw)
  To: Olof Johansson
  Cc: linux-arm-kernel, Pankaj Dubey, kgene.kim, linux, t.figa,
	linux-kernel, linux-samsung-soc

On Sunday 11 May 2014 18:47:28 Olof Johansson wrote:
> > > Also for platsmp.c and pm.c I can think of following approaches
> > > 1: Keep these macros till we get generic solution?
> > > 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> > > till we get
> > > generic solution. So that at least we can remove #ifdef  based macros
> > > as soc_is_exynosXYZ.
> > > 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> > > till we get
> > > generic solution. For some cases where we want to know SoC revision let us
> > > map chipid register and get revision there itself.
> > > 
> > > Please let me know what approach you think will be good?
> > 
> > I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> > but I think either way the SoC specific values would be better kept in the
> > mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.
> 
> The generic solution is already there: of_machine_is_compatible() is perfectly
> sensible to use for _some_ of these inits. Cpufreq is one of the few that comes
> to mind, and maybe some of the platsmp and pm stuff.
> 
> Note that none of them should be used in runtime, i.e. you should only use them
> at probe/setup time and maybe have a local state in the driver if needed.
> 
> I'd rather get people used to that format than everybody needing to implement
> a chipid driver now too, especially on platforms that might not even have a
> suitable chipid block to base a driver around -- not to mention having to
> fill the namespace with is_soc_*() stuff.

I was coming from the other angle: exynos is already using soc_is_*() in too
many places. I'd like to first see the ones cleaned up that really should be
doing something else because they have a device-local ID to look at.

If we end up with a couple of instances that don't have a good alternative,
we can use of_machine_is_compatible() for those, but I'd like to avoid doing
a blind conversion because that would likely lead to more instances in the
future, not fewer.

I agree that we should have to introduce new chip ID drivers on other
platforms for the purpose of finding out the SoC version, especially not
with private APIs.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/4] Introducing Exynos ChipId driver
@ 2014-05-12  9:47           ` Arnd Bergmann
  0 siblings, 0 replies; 36+ messages in thread
From: Arnd Bergmann @ 2014-05-12  9:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Sunday 11 May 2014 18:47:28 Olof Johansson wrote:
> > > Also for platsmp.c and pm.c I can think of following approaches
> > > 1: Keep these macros till we get generic solution?
> > > 2: Allow chipid driver to expose APIs to check SoC id and SoC revisions 
> > > till we get
> > > generic solution. So that at least we can remove #ifdef  based macros
> > > as soc_is_exynosXYZ.
> > > 3: Use of "of_flat_dt_is_compatible" or similar APIs in these machine files 
> > > till we get
> > > generic solution. For some cases where we want to know SoC revision let us
> > > map chipid register and get revision there itself.
> > > 
> > > Please let me know what approach you think will be good?
> > 
> > I think 1 or 2 would be better than 3. Between those two, I'm undecided,
> > but I think either way the SoC specific values would be better kept in the
> > mach-samsung directory than in plat/cpu.h or linux/exynos-chipid.h.
> 
> The generic solution is already there: of_machine_is_compatible() is perfectly
> sensible to use for _some_ of these inits. Cpufreq is one of the few that comes
> to mind, and maybe some of the platsmp and pm stuff.
> 
> Note that none of them should be used in runtime, i.e. you should only use them
> at probe/setup time and maybe have a local state in the driver if needed.
> 
> I'd rather get people used to that format than everybody needing to implement
> a chipid driver now too, especially on platforms that might not even have a
> suitable chipid block to base a driver around -- not to mention having to
> fill the namespace with is_soc_*() stuff.

I was coming from the other angle: exynos is already using soc_is_*() in too
many places. I'd like to first see the ones cleaned up that really should be
doing something else because they have a device-local ID to look at.

If we end up with a couple of instances that don't have a good alternative,
we can use of_machine_is_compatible() for those, but I'd like to avoid doing
a blind conversion because that would likely lead to more instances in the
future, not fewer.

I agree that we should have to introduce new chip ID drivers on other
platforms for the purpose of finding out the SoC version, especially not
with private APIs.

	Arnd

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2014-05-12  9:47 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-03  6:11 [PATCH 0/4] Introducing Exynos ChipId driver Pankaj Dubey
2014-05-03  6:11 ` Pankaj Dubey
2014-05-03  6:11 ` [PATCH 1/4] ARM: EXYNOS: remove soc_is_exynos4/5 from exynos.c Pankaj Dubey
2014-05-03  6:11   ` Pankaj Dubey
2014-05-03  6:11 ` [PATCH 2/4] ARM: EXYNOS: remove unused header inclusion from hotplug.c Pankaj Dubey
2014-05-03  6:11   ` Pankaj Dubey
2014-05-03  6:11 ` [PATCH 3/4] misc: exynos-chipid: Add Exynos Chipid driver support Pankaj Dubey
2014-05-03  6:11   ` Pankaj Dubey
2014-05-05  7:57   ` Krzysztof Kozlowski
2014-05-05  7:57     ` Krzysztof Kozlowski
2014-05-05  9:28     ` Pankaj Dubey
2014-05-05  9:28       ` Pankaj Dubey
2014-05-03  6:11 ` [PATCH 4/4] ARM: EXYNOS: Refactoring to remove soc_is_exynosXXXX macros from exynos Pankaj Dubey
2014-05-03  6:11   ` Pankaj Dubey
2014-05-03 15:02 ` [PATCH 0/4] Introducing Exynos ChipId driver Arnd Bergmann
2014-05-03 15:02   ` Arnd Bergmann
2014-05-05  9:23   ` Pankaj Dubey
2014-05-05  9:23     ` Pankaj Dubey
2014-05-05 14:58     ` Arnd Bergmann
2014-05-05 14:58       ` Arnd Bergmann
2014-05-05 15:01       ` Arnd Bergmann
2014-05-05 15:01         ` Arnd Bergmann
2014-05-06  6:57       ` Pankaj Dubey
2014-05-06  6:57         ` Pankaj Dubey
2014-05-06  7:24         ` Arnd Bergmann
2014-05-06  7:24           ` Arnd Bergmann
2014-05-12  1:47       ` Olof Johansson
2014-05-12  1:47         ` Olof Johansson
2014-05-12  9:47         ` Arnd Bergmann
2014-05-12  9:47           ` Arnd Bergmann
2014-05-05 15:34   ` Rob Herring
2014-05-05 15:34     ` Rob Herring
2014-05-05 15:34     ` Rob Herring
2014-05-06  7:22     ` Arnd Bergmann
2014-05-06  7:22       ` Arnd Bergmann
2014-05-06  7:22       ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.