* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-16 17:37 ` Krzysztof Kozlowski
` (3 more replies)
2016-12-10 13:08 ` [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS Pankaj Dubey
` (7 subsequent siblings)
8 siblings, 4 replies; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
This patch intends to provide initialization code for all these functionalities,
at the same time it provides some sysfs entries for accessing these information
to user-space.
This driver uses existing binding for exynos-chipid.
CC: Grant Likely <grant.likely@linaro.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
[m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/soc/samsung/Kconfig | 5 ++
drivers/soc/samsung/Makefile | 1 +
drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
3 files changed, 122 insertions(+)
create mode 100644 drivers/soc/samsung/exynos-chipid.c
diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
index 2455339..f9ab858 100644
--- a/drivers/soc/samsung/Kconfig
+++ b/drivers/soc/samsung/Kconfig
@@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
bool "Exynos PM domains" if COMPILE_TEST
depends on PM_GENERIC_DOMAINS || COMPILE_TEST
+config EXYNOS_CHIPID
+ bool "Exynos Chipid controller driver" if COMPILE_TEST
+ depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
+ select SOC_BUS
+
endif
diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
index 3619f2e..2a8a85e 100644
--- a/drivers/soc/samsung/Makefile
+++ b/drivers/soc/samsung/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
exynos5250-pmu.o exynos5420-pmu.o
obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
+obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
new file mode 100644
index 0000000..cf0128b
--- /dev/null
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2016 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/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+
+#define EXYNOS_SUBREV_MASK (0xF << 4)
+#define EXYNOS_MAINREV_MASK (0xF << 0)
+#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
+
+static const struct exynos_soc_id {
+ const char *name;
+ unsigned int id;
+ unsigned int mask;
+} soc_ids[] = {
+ { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
+ { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
+ { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
+ { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
+ { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
+ { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
+ { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
+ { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
+ { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
+ { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
+ { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
+ { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
+};
+
+static const char * __init product_id_to_soc_id(unsigned int product_id)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
+ if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
+ return soc_ids[i].name;
+ return "UNKNOWN";
+}
+
+static const struct of_device_id of_exynos_chipid_ids[] = {
+ {
+ .compatible = "samsung,exynos4210-chipid",
+ },
+ {},
+};
+
+/**
+ * exynos_chipid_early_init: Early chipid initialization
+ */
+int __init exynos_chipid_early_init(void)
+{
+ struct soc_device_attribute *soc_dev_attr;
+ struct soc_device *soc_dev;
+ struct device_node *root;
+ struct device_node *np;
+ void __iomem *exynos_chipid_base;
+ const struct of_device_id *match;
+ u32 product_id;
+ u32 revision;
+
+ np = of_find_matching_node_and_match(NULL,
+ of_exynos_chipid_ids, &match);
+ if (!np)
+ return -ENODEV;
+
+ exynos_chipid_base = of_iomap(np, 0);
+
+ if (!exynos_chipid_base)
+ return PTR_ERR(exynos_chipid_base);
+
+ product_id = readl_relaxed(exynos_chipid_base);
+ revision = product_id & EXYNOS_REV_MASK;
+ iounmap(exynos_chipid_base);
+
+ soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+ if (!soc_dev_attr)
+ return -ENODEV;
+
+ soc_dev_attr->family = "Samsung Exynos";
+
+ root = of_find_node_by_path("/");
+ of_property_read_string(root, "model", &soc_dev_attr->machine);
+ of_node_put(root);
+
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
+ soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
+
+
+ pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
+ product_id_to_soc_id(product_id), revision);
+
+ soc_dev = soc_device_register(soc_dev_attr);
+ if (IS_ERR(soc_dev)) {
+ kfree(soc_dev_attr->revision);
+ kfree_const(soc_dev_attr->soc_id);
+ kfree(soc_dev_attr);
+ return PTR_ERR(soc_dev);
+ }
+
+ return 0;
+}
+early_initcall(exynos_chipid_early_init);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-10 13:08 ` [PATCH v8 1/8] soc: samsung: add exynos chipid driver support Pankaj Dubey
@ 2016-12-16 17:37 ` Krzysztof Kozlowski
2016-12-17 4:06 ` Pankaj Dubey
2016-12-19 11:59 ` Markus Reichl
` (2 subsequent siblings)
3 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-16 17:37 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:36PM +0530, Pankaj Dubey wrote:
> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> This patch intends to provide initialization code for all these functionalities,
> at the same time it provides some sysfs entries for accessing these information
> to user-space.
>
> This driver uses existing binding for exynos-chipid.
>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/soc/samsung/Kconfig | 5 ++
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 122 insertions(+)
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2455339..f9ab858 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
> bool "Exynos PM domains" if COMPILE_TEST
> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>
> +config EXYNOS_CHIPID
> + bool "Exynos Chipid controller driver" if COMPILE_TEST
> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
1. Why this can be compile tested only on ARM architectures?
2. Don't you need also SOC_BUS?
> + select SOC_BUS
> +
> endif
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 3619f2e..2a8a85e 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
> exynos5250-pmu.o exynos5420-pmu.o
> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
Please put it before EXYNOS_PMU, keeping this sorted alphabetical helps
avoiding conflicts of continuous edits.
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..cf0128b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright (c) 2016 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/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +
> +#define EXYNOS_SUBREV_MASK (0xF << 4)
> +#define EXYNOS_MAINREV_MASK (0xF << 0)
> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> +
> +static const struct exynos_soc_id {
> + const char *name;
> + unsigned int id;
> + unsigned int mask;
> +} soc_ids[] = {
> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
> +};
> +
> +static const char * __init product_id_to_soc_id(unsigned int product_id)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
> + return soc_ids[i].name;
> + return "UNKNOWN";
> +}
> +
> +static const struct of_device_id of_exynos_chipid_ids[] = {
> + {
> + .compatible = "samsung,exynos4210-chipid",
> + },
> + {},
> +};
> +
> +/**
> + * exynos_chipid_early_init: Early chipid initialization
> + */
This comment is meaningless, it duplicates the name of function.
> +int __init exynos_chipid_early_init(void)
> +{
> + struct soc_device_attribute *soc_dev_attr;
> + struct soc_device *soc_dev;
> + struct device_node *root;
> + struct device_node *np;
> + void __iomem *exynos_chipid_base;
> + const struct of_device_id *match;
> + u32 product_id;
> + u32 revision;
> +
> + np = of_find_matching_node_and_match(NULL,
> + of_exynos_chipid_ids, &match);
You don't use the match here, so how about either
of_find_matching_node() or of_find_compatible_node()? The latter looks
better (less calls inside) and actually you want just check one
compatible field?
> + if (!np)
> + return -ENODEV;
> +
> + exynos_chipid_base = of_iomap(np, 0);
> +
> + if (!exynos_chipid_base)
> + return PTR_ERR(exynos_chipid_base);
> +
> + product_id = readl_relaxed(exynos_chipid_base);
Duplicated space before '='.
> + revision = product_id & EXYNOS_REV_MASK;
> + iounmap(exynos_chipid_base);
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENODEV;
> +
> + soc_dev_attr->family = "Samsung Exynos";
> +
> + root = of_find_node_by_path("/");
> + of_property_read_string(root, "model", &soc_dev_attr->machine);
> + of_node_put(root);
> +
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
> +
> +
> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
> + product_id_to_soc_id(product_id), revision);
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + kfree(soc_dev_attr->revision);
> + kfree_const(soc_dev_attr->soc_id);
It wasn't allocated with *_const, so no need to free it.
Best regards,
Krzysztof
> + kfree(soc_dev_attr);
> + return PTR_ERR(soc_dev);
> + }
> +
> + return 0;
> +}
> +early_initcall(exynos_chipid_early_init);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-16 17:37 ` Krzysztof Kozlowski
@ 2016-12-17 4:06 ` Pankaj Dubey
2016-12-17 12:03 ` Krzysztof Kozlowski
0 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-17 4:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
On 16 December 2016 at 23:07, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:36PM +0530, Pankaj Dubey wrote:
>> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
>> This patch intends to provide initialization code for all these functionalities,
>> at the same time it provides some sysfs entries for accessing these information
>> to user-space.
>>
>> This driver uses existing binding for exynos-chipid.
>>
>> CC: Grant Likely <grant.likely@linaro.org>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/soc/samsung/Kconfig | 5 ++
>> drivers/soc/samsung/Makefile | 1 +
>> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 122 insertions(+)
>> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>>
>> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
>> index 2455339..f9ab858 100644
>> --- a/drivers/soc/samsung/Kconfig
>> +++ b/drivers/soc/samsung/Kconfig
>> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
>> bool "Exynos PM domains" if COMPILE_TEST
>> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>>
>> +config EXYNOS_CHIPID
>> + bool "Exynos Chipid controller driver" if COMPILE_TEST
>> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
>
> 1. Why this can be compile tested only on ARM architectures?
Well I just used dependency same as EXYNOS_PMU, but I can see it will
be enabled for compile test on ARM64 isn't it?
> 2. Don't you need also SOC_BUS?
CHIPID needs SoC_BUS and for the same reason it is selecting SOC_BUS
in the next line.
If we mark it as a dependency (under depends on), even then we need to
select this either
under same EXYNOS_CHIPID config or ARCH_EXYNOS config.
>
>> + select SOC_BUS
>> +
>> endif
>> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
>> index 3619f2e..2a8a85e 100644
>> --- a/drivers/soc/samsung/Makefile
>> +++ b/drivers/soc/samsung/Makefile
>> @@ -1,3 +1,4 @@
>> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
>> exynos5250-pmu.o exynos5420-pmu.o
>> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
>> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
>
> Please put it before EXYNOS_PMU, keeping this sorted alphabetical helps
> avoiding conflicts of continuous edits.
>
>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>> new file mode 100644
>> index 0000000..cf0128b
>> --- /dev/null
>> +++ b/drivers/soc/samsung/exynos-chipid.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * Copyright (c) 2016 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/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/sys_soc.h>
>> +
>> +#define EXYNOS_SUBREV_MASK (0xF << 4)
>> +#define EXYNOS_MAINREV_MASK (0xF << 0)
>> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
>> +
>> +static const struct exynos_soc_id {
>> + const char *name;
>> + unsigned int id;
>> + unsigned int mask;
>> +} soc_ids[] = {
>> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
>> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
>> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
>> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
>> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
>> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
>> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
>> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
>> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
>> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
>> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
>> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
>> +};
>> +
>> +static const char * __init product_id_to_soc_id(unsigned int product_id)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
>> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
>> + return soc_ids[i].name;
>> + return "UNKNOWN";
>> +}
>> +
>> +static const struct of_device_id of_exynos_chipid_ids[] = {
>> + {
>> + .compatible = "samsung,exynos4210-chipid",
>> + },
>> + {},
>> +};
>> +
>> +/**
>> + * exynos_chipid_early_init: Early chipid initialization
>> + */
>
> This comment is meaningless, it duplicates the name of function.
Ok, will remove this in v9.
>
>> +int __init exynos_chipid_early_init(void)
>> +{
>> + struct soc_device_attribute *soc_dev_attr;
>> + struct soc_device *soc_dev;
>> + struct device_node *root;
>> + struct device_node *np;
>> + void __iomem *exynos_chipid_base;
>> + const struct of_device_id *match;
>> + u32 product_id;
>> + u32 revision;
>> +
>> + np = of_find_matching_node_and_match(NULL,
>> + of_exynos_chipid_ids, &match);
>
> You don't use the match here, so how about either
> of_find_matching_node() or of_find_compatible_node()? The latter looks
> better (less calls inside) and actually you want just check one
> compatible field?
>
Ok, will adopt this in v9.
>> + if (!np)
>> + return -ENODEV;
>> +
>> + exynos_chipid_base = of_iomap(np, 0);
>> +
>> + if (!exynos_chipid_base)
>> + return PTR_ERR(exynos_chipid_base);
>> +
>> + product_id = readl_relaxed(exynos_chipid_base);
>
> Duplicated space before '='.
Ok, will fix this.
>
>> + revision = product_id & EXYNOS_REV_MASK;
>> + iounmap(exynos_chipid_base);
>> +
>> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>> + if (!soc_dev_attr)
>> + return -ENODEV;
>> +
>> + soc_dev_attr->family = "Samsung Exynos";
>> +
>> + root = of_find_node_by_path("/");
>> + of_property_read_string(root, "model", &soc_dev_attr->machine);
>> + of_node_put(root);
>> +
>> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
>> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
>> +
>> +
>> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
>> + product_id_to_soc_id(product_id), revision);
>> +
>> + soc_dev = soc_device_register(soc_dev_attr);
>> + if (IS_ERR(soc_dev)) {
>> + kfree(soc_dev_attr->revision);
>> + kfree_const(soc_dev_attr->soc_id);
>
> It wasn't allocated with *_const, so no need to free it.
Yes, will fix this.
>
> Best regards,
> Krzysztof
Thanks for review.
Pankaj Dubey
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-17 4:06 ` Pankaj Dubey
@ 2016-12-17 12:03 ` Krzysztof Kozlowski
0 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-17 12:03 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 17, 2016 at 09:36:59AM +0530, Pankaj Dubey wrote:
> Hi Krzysztof,
>
> On 16 December 2016 at 23:07, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On Sat, Dec 10, 2016 at 06:38:36PM +0530, Pankaj Dubey wrote:
> >> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> >> This patch intends to provide initialization code for all these functionalities,
> >> at the same time it provides some sysfs entries for accessing these information
> >> to user-space.
> >>
> >> This driver uses existing binding for exynos-chipid.
> >>
> >> CC: Grant Likely <grant.likely@linaro.org>
> >> CC: Rob Herring <robh+dt@kernel.org>
> >> CC: Linus Walleij <linus.walleij@linaro.org>
> >> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> >> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >> drivers/soc/samsung/Kconfig | 5 ++
> >> drivers/soc/samsung/Makefile | 1 +
> >> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 122 insertions(+)
> >> create mode 100644 drivers/soc/samsung/exynos-chipid.c
> >>
> >> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> >> index 2455339..f9ab858 100644
> >> --- a/drivers/soc/samsung/Kconfig
> >> +++ b/drivers/soc/samsung/Kconfig
> >> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
> >> bool "Exynos PM domains" if COMPILE_TEST
> >> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
> >>
> >> +config EXYNOS_CHIPID
> >> + bool "Exynos Chipid controller driver" if COMPILE_TEST
> >> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
> >
> > 1. Why this can be compile tested only on ARM architectures?
>
> Well I just used dependency same as EXYNOS_PMU, but I can see it will
> be enabled for compile test on ARM64 isn't it?
I don't remember the PMU case... maybe it used clocks or something. Here
there are no arch-specific build dependencies.
>
> > 2. Don't you need also SOC_BUS?
>
> CHIPID needs SoC_BUS and for the same reason it is selecting SOC_BUS
> in the next line.
> If we mark it as a dependency (under depends on), even then we need to
> select this either
> under same EXYNOS_CHIPID config or ARCH_EXYNOS config.
Ah, I missed that line... Argh, sorry for noise.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-10 13:08 ` [PATCH v8 1/8] soc: samsung: add exynos chipid driver support Pankaj Dubey
2016-12-16 17:37 ` Krzysztof Kozlowski
@ 2016-12-19 11:59 ` Markus Reichl
2016-12-19 13:29 ` pankaj.dubey
2016-12-21 14:08 ` Andrzej Hajda
[not found] ` <CGME20161227140255epcas1p1a2c2b50ede0e9b2378c8ac052b1617f6@epcas1p1.samsung.com>
3 siblings, 1 reply; 31+ messages in thread
From: Markus Reichl @ 2016-12-19 11:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi Pankaj,
tested your patches 1/8 and 2/8 + Javiers diff for verbose output:
https://www.spinics.net/lists/linux-samsung-soc/msg56576.html
on Odroid U3:
[ 0.080178] Exynos: CPU[UNKNOWN] CPU_REV[0x20] PKG_ID[0x602d058] AUX_INFO[0x0]
on Odroid X:
[ 0.080169] Exynos: CPU[UNKNOWN] CPU_REV[0x11] PKG_ID[0x1b0f6008] AUX_INFO[0x0]
XU4: [ 0.080039] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1c04832a] AUX_INFO[0x43]
XU3: [ 0.080034] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1604832a] AUX_INFO[0x43]
XU3-lite:[ 0.080033] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x5a12832a] AUX_INFO[0x13000054]
Regards,
--
Markus Reichl
Am 10.12.2016 um 14:08 schrieb Pankaj Dubey:
> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> This patch intends to provide initialization code for all these functionalities,
> at the same time it provides some sysfs entries for accessing these information
> to user-space.
>
> This driver uses existing binding for exynos-chipid.
>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/soc/samsung/Kconfig | 5 ++
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 122 insertions(+)
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2455339..f9ab858 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
> bool "Exynos PM domains" if COMPILE_TEST
> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>
> +config EXYNOS_CHIPID
> + bool "Exynos Chipid controller driver" if COMPILE_TEST
> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
> + select SOC_BUS
> +
> endif
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 3619f2e..2a8a85e 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
> exynos5250-pmu.o exynos5420-pmu.o
> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..cf0128b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright (c) 2016 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/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +
> +#define EXYNOS_SUBREV_MASK (0xF << 4)
> +#define EXYNOS_MAINREV_MASK (0xF << 0)
> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> +
> +static const struct exynos_soc_id {
> + const char *name;
> + unsigned int id;
> + unsigned int mask;
> +} soc_ids[] = {
> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
> +};
> +
> +static const char * __init product_id_to_soc_id(unsigned int product_id)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
> + return soc_ids[i].name;
> + return "UNKNOWN";
> +}
> +
> +static const struct of_device_id of_exynos_chipid_ids[] = {
> + {
> + .compatible = "samsung,exynos4210-chipid",
> + },
> + {},
> +};
> +
> +/**
> + * exynos_chipid_early_init: Early chipid initialization
> + */
> +int __init exynos_chipid_early_init(void)
> +{
> + struct soc_device_attribute *soc_dev_attr;
> + struct soc_device *soc_dev;
> + struct device_node *root;
> + struct device_node *np;
> + void __iomem *exynos_chipid_base;
> + const struct of_device_id *match;
> + u32 product_id;
> + u32 revision;
> +
> + np = of_find_matching_node_and_match(NULL,
> + of_exynos_chipid_ids, &match);
> + if (!np)
> + return -ENODEV;
> +
> + exynos_chipid_base = of_iomap(np, 0);
> +
> + if (!exynos_chipid_base)
> + return PTR_ERR(exynos_chipid_base);
> +
> + product_id = readl_relaxed(exynos_chipid_base);
> + revision = product_id & EXYNOS_REV_MASK;
> + iounmap(exynos_chipid_base);
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENODEV;
> +
> + soc_dev_attr->family = "Samsung Exynos";
> +
> + root = of_find_node_by_path("/");
> + of_property_read_string(root, "model", &soc_dev_attr->machine);
> + of_node_put(root);
> +
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
> +
> +
> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
> + product_id_to_soc_id(product_id), revision);
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + kfree(soc_dev_attr->revision);
> + kfree_const(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr);
> + return PTR_ERR(soc_dev);
> + }
> +
> + return 0;
> +}
> +early_initcall(exynos_chipid_early_init);
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-19 11:59 ` Markus Reichl
@ 2016-12-19 13:29 ` pankaj.dubey
2016-12-19 18:03 ` Krzysztof Kozlowski
0 siblings, 1 reply; 31+ messages in thread
From: pankaj.dubey @ 2016-12-19 13:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Markus,
On Monday 19 December 2016 05:29 PM, Markus Reichl wrote:
> Hi Pankaj,
>
> tested your patches 1/8 and 2/8 + Javiers diff for verbose output:
> https://www.spinics.net/lists/linux-samsung-soc/msg56576.html
>
> on Odroid U3:
> [ 0.080178] Exynos: CPU[UNKNOWN] CPU_REV[0x20] PKG_ID[0x602d058] AUX_INFO[0x0]
> on Odroid X:
> [ 0.080169] Exynos: CPU[UNKNOWN] CPU_REV[0x11] PKG_ID[0x1b0f6008] AUX_INFO[0x0]
>
> XU4: [ 0.080039] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1c04832a] AUX_INFO[0x43]
> XU3: [ 0.080034] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x1604832a] AUX_INFO[0x43]
> XU3-lite:[ 0.080033] Exynos: CPU[EXYNOS5800] CPU_REV[0x1] PKG_ID[0x5a12832a] AUX_INFO[0x13000054]
>
> Regards,
> --
> Markus Reichl
Thanks for testing and letting us know.
Hopefully this driver will be of more use in near future, as Javiers
pointed out it can help us to select better ASV tables based on PKG_ID
and AUX_INFO. Even though these fields are not part of the driver in my
patch, it can be extended as per need.
Thanks,
Pankaj Dubey
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-19 13:29 ` pankaj.dubey
@ 2016-12-19 18:03 ` Krzysztof Kozlowski
2016-12-21 7:52 ` pankaj.dubey
0 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-19 18:03 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 19, 2016 at 3:29 PM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
>
> Hi Markus,
>
> On Monday 19 December 2016 05:29 PM, Markus Reichl wrote:
> > Hi Pankaj,
> >
> > tested your patches 1/8 and 2/8 + Javiers diff for verbose output:
> > https://www.spinics.net/lists/linux-samsung-soc/msg56576.html
> >
> > on Odroid U3:
> > [ 0.080178] Exynos: CPU[UNKNOWN] CPU_REV[0x20] PKG_ID[0x602d058] AUX_INFO[0x0]
> > on Odroid X:
> > [ 0.080169] Exynos: CPU[UNKNOWN] CPU_REV[0x11] PKG_ID[0x1b0f6008] AUX_INFO[0x0]
Hmm.... this needs fixes.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-19 18:03 ` Krzysztof Kozlowski
@ 2016-12-21 7:52 ` pankaj.dubey
0 siblings, 0 replies; 31+ messages in thread
From: pankaj.dubey @ 2016-12-21 7:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Markus, Krzysztof,
On Monday 19 December 2016 11:33 PM, Krzysztof Kozlowski wrote:
> On Mon, Dec 19, 2016 at 3:29 PM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
>>
>> Hi Markus,
>>
>> On Monday 19 December 2016 05:29 PM, Markus Reichl wrote:
>>> Hi Pankaj,
>>>
>>> tested your patches 1/8 and 2/8 + Javiers diff for verbose output:
>>> https://www.spinics.net/lists/linux-samsung-soc/msg56576.html
>>>
>>> on Odroid U3:
>>> [ 0.080178] Exynos: CPU[UNKNOWN] CPU_REV[0x20] PKG_ID[0x602d058] AUX_INFO[0x0]
>>> on Odroid X:
>>> [ 0.080169] Exynos: CPU[UNKNOWN] CPU_REV[0x11] PKG_ID[0x1b0f6008] AUX_INFO[0x0]
>
> Hmm.... this needs fixes.
>
Ah.. I missed to notice this.
I found issue as: in case of Exynos4412 CPU_ID is mentioned wrong as
(0xE4412000) in the driver code which caused this issue. Correct value
of CPU_ID is 0xE4412200. I will fix this in next version. Once again
thanks for testing and letting me know about this issue.
Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-10 13:08 ` [PATCH v8 1/8] soc: samsung: add exynos chipid driver support Pankaj Dubey
2016-12-16 17:37 ` Krzysztof Kozlowski
2016-12-19 11:59 ` Markus Reichl
@ 2016-12-21 14:08 ` Andrzej Hajda
[not found] ` <CGME20161227140255epcas1p1a2c2b50ede0e9b2378c8ac052b1617f6@epcas1p1.samsung.com>
3 siblings, 0 replies; 31+ messages in thread
From: Andrzej Hajda @ 2016-12-21 14:08 UTC (permalink / raw)
To: linux-arm-kernel
On 10.12.2016 14:08, Pankaj Dubey wrote:
> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> This patch intends to provide initialization code for all these functionalities,
> at the same time it provides some sysfs entries for accessing these information
> to user-space.
>
> This driver uses existing binding for exynos-chipid.
>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/soc/samsung/Kconfig | 5 ++
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 122 insertions(+)
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2455339..f9ab858 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
> bool "Exynos PM domains" if COMPILE_TEST
> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>
> +config EXYNOS_CHIPID
> + bool "Exynos Chipid controller driver" if COMPILE_TEST
> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
> + select SOC_BUS
> +
> endif
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 3619f2e..2a8a85e 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
> exynos5250-pmu.o exynos5420-pmu.o
> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..cf0128b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright (c) 2016 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/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sys_soc.h>
> +
> +#define EXYNOS_SUBREV_MASK (0xF << 4)
> +#define EXYNOS_MAINREV_MASK (0xF << 0)
> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> +
> +static const struct exynos_soc_id {
> + const char *name;
> + unsigned int id;
> + unsigned int mask;
> +} soc_ids[] = {
> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
I wonder why there are different masks in case of Exynos42*?
In specs I have access to mask is always 0xFFFFF000.
> +};
> +
> +static const char * __init product_id_to_soc_id(unsigned int product_id)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
> + return soc_ids[i].name;
> + return "UNKNOWN";
> +}
> +
> +static const struct of_device_id of_exynos_chipid_ids[] = {
> + {
> + .compatible = "samsung,exynos4210-chipid",
> + },
> + {},
> +};
> +
> +/**
> + * exynos_chipid_early_init: Early chipid initialization
> + */
> +int __init exynos_chipid_early_init(void)
> +{
> + struct soc_device_attribute *soc_dev_attr;
> + struct soc_device *soc_dev;
> + struct device_node *root;
> + struct device_node *np;
> + void __iomem *exynos_chipid_base;
> + const struct of_device_id *match;
> + u32 product_id;
> + u32 revision;
> +
> + np = of_find_matching_node_and_match(NULL,
> + of_exynos_chipid_ids, &match);
> + if (!np)
> + return -ENODEV;
> +
> + exynos_chipid_base = of_iomap(np, 0);
> +
> + if (!exynos_chipid_base)
> + return PTR_ERR(exynos_chipid_base);
> +
> + product_id = readl_relaxed(exynos_chipid_base);
> + revision = product_id & EXYNOS_REV_MASK;
> + iounmap(exynos_chipid_base);
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return -ENODEV;
> +
> + soc_dev_attr->family = "Samsung Exynos";
> +
> + root = of_find_node_by_path("/");
> + of_property_read_string(root, "model", &soc_dev_attr->machine);
> + of_node_put(root);
> +
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
> +
> +
> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
> + product_id_to_soc_id(product_id), revision);
I wonder if it wouldn't be good to log numeric value of whole PRO_ID
register, this way we will have also useful log for chips not know to
soc_ids array.
It will show also some missing bits: masked bits, package_information
and reserved bit. Btw. package_information can be used to distinguish
S5PC210 and S5PV310.
And small optimization, you can reuse soc_dev_attr->soc_id instead of
calling the same function again.
And finally all log could be moved after soc_device_register, this way
you can use:
struc device *dev = soc_device_to_device(soc_dev);
dev_info(dev, ...);
Regards
Andrzej
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + kfree(soc_dev_attr->revision);
> + kfree_const(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr);
> + return PTR_ERR(soc_dev);
> + }
> +
> + return 0;
> +}
> +early_initcall(exynos_chipid_early_init);
^ permalink raw reply [flat|nested] 31+ messages in thread[parent not found: <CGME20161227140255epcas1p1a2c2b50ede0e9b2378c8ac052b1617f6@epcas1p1.samsung.com>]
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
[not found] ` <CGME20161227140255epcas1p1a2c2b50ede0e9b2378c8ac052b1617f6@epcas1p1.samsung.com>
@ 2016-12-27 14:02 ` Bartlomiej Zolnierkiewicz
2016-12-28 2:38 ` Pankaj Dubey
0 siblings, 1 reply; 31+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2016-12-27 14:02 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Saturday, December 10, 2016 06:38:36 PM Pankaj Dubey wrote:
> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> This patch intends to provide initialization code for all these functionalities,
> at the same time it provides some sysfs entries for accessing these information
> to user-space.
>
> This driver uses existing binding for exynos-chipid.
>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/soc/samsung/Kconfig | 5 ++
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 122 insertions(+)
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
[...]
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..cf0128b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
[...]
> +/**
> + * exynos_chipid_early_init: Early chipid initialization
> + */
> +int __init exynos_chipid_early_init(void)
> +{
> + struct soc_device_attribute *soc_dev_attr;
> + struct soc_device *soc_dev;
> + struct device_node *root;
> + struct device_node *np;
> + void __iomem *exynos_chipid_base;
> + const struct of_device_id *match;
> + u32 product_id;
> + u32 revision;
> +
> + np = of_find_matching_node_and_match(NULL,
> + of_exynos_chipid_ids, &match);
> + if (!np)
> + return -ENODEV;
> +
> + exynos_chipid_base = of_iomap(np, 0);
of_node_put(np) is missing here.
> + if (!exynos_chipid_base)
> + return PTR_ERR(exynos_chipid_base);
PTR_ERR use here is incorrect - of_iomap() returns valid pointer or
NULL. Please just return -NODEV on of_iomap() failure.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
2016-12-27 14:02 ` Bartlomiej Zolnierkiewicz
@ 2016-12-28 2:38 ` Pankaj Dubey
0 siblings, 0 replies; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-28 2:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi Bartilomiej,
On 27 December 2016 at 19:32, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> Hi,
>
> On Saturday, December 10, 2016 06:38:36 PM Pankaj Dubey wrote:
>> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
>> This patch intends to provide initialization code for all these functionalities,
>> at the same time it provides some sysfs entries for accessing these information
>> to user-space.
>>
>> This driver uses existing binding for exynos-chipid.
>>
>> CC: Grant Likely <grant.likely@linaro.org>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/soc/samsung/Kconfig | 5 ++
>> drivers/soc/samsung/Makefile | 1 +
>> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 122 insertions(+)
>> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> [...]
>
>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>> new file mode 100644
>> index 0000000..cf0128b
>> --- /dev/null
>> +++ b/drivers/soc/samsung/exynos-chipid.c
>
> [...]
>
>> +/**
>> + * exynos_chipid_early_init: Early chipid initialization
>> + */
>> +int __init exynos_chipid_early_init(void)
>> +{
>> + struct soc_device_attribute *soc_dev_attr;
>> + struct soc_device *soc_dev;
>> + struct device_node *root;
>> + struct device_node *np;
>> + void __iomem *exynos_chipid_base;
>> + const struct of_device_id *match;
>> + u32 product_id;
>> + u32 revision;
>> +
>> + np = of_find_matching_node_and_match(NULL,
>> + of_exynos_chipid_ids, &match);
>> + if (!np)
>> + return -ENODEV;
>> +
>> + exynos_chipid_base = of_iomap(np, 0);
>
> of_node_put(np) is missing here.
>
OK.
>> + if (!exynos_chipid_base)
>> + return PTR_ERR(exynos_chipid_base);
>
> PTR_ERR use here is incorrect - of_iomap() returns valid pointer or
> NULL. Please just return -NODEV on of_iomap() failure.
>
OK.
Thanks for review. I will address these issues in next version.
Thanks,
Pankaj Dubey
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
2016-12-10 13:08 ` [PATCH v8 1/8] soc: samsung: add exynos chipid driver support Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-16 18:04 ` Krzysztof Kozlowski
2016-12-10 13:08 ` [PATCH v8 3/8] ARM64: " Pankaj Dubey
` (6 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
As now we have chipid driver to initialize SoC related information
lets include it in build by default.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 0bb63b8..29065c5 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -16,6 +16,7 @@ menuconfig ARCH_EXYNOS
select ARM_AMBA
select ARM_GIC
select COMMON_CLK_SAMSUNG
+ select EXYNOS_CHIPID
select EXYNOS_THERMAL
select EXYNOS_PMU
select EXYNOS_SROM
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
2016-12-10 13:08 ` [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS Pankaj Dubey
@ 2016-12-16 18:04 ` Krzysztof Kozlowski
2016-12-17 4:15 ` Pankaj Dubey
0 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-16 18:04 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:37PM +0530, Pankaj Dubey wrote:
> As now we have chipid driver to initialize SoC related information
> lets include it in build by default.
s/lets/let's/
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 0bb63b8..29065c5 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -16,6 +16,7 @@ menuconfig ARCH_EXYNOS
> select ARM_AMBA
> select ARM_GIC
> select COMMON_CLK_SAMSUNG
> + select EXYNOS_CHIPID
> select EXYNOS_THERMAL
> select EXYNOS_PMU
> select EXYNOS_SROM
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
2016-12-16 18:04 ` Krzysztof Kozlowski
@ 2016-12-17 4:15 ` Pankaj Dubey
0 siblings, 0 replies; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-17 4:15 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
On 16 December 2016 at 23:34, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:37PM +0530, Pankaj Dubey wrote:
>> As now we have chipid driver to initialize SoC related information
>> lets include it in build by default.
> s/lets/let's/
>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>
Thanks for review. Will fix minor nitpick in v9 and include your
Reviewed-by tag.
Pankaj Dubey
> Best regards,
> Krzysztof
>
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
>> index 0bb63b8..29065c5 100644
>> --- a/arch/arm/mach-exynos/Kconfig
>> +++ b/arch/arm/mach-exynos/Kconfig
>> @@ -16,6 +16,7 @@ menuconfig ARCH_EXYNOS
>> select ARM_AMBA
>> select ARM_GIC
>> select COMMON_CLK_SAMSUNG
>> + select EXYNOS_CHIPID
>> select EXYNOS_THERMAL
>> select EXYNOS_PMU
>> select EXYNOS_SROM
>> --
>> 2.7.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 3/8] ARM64: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
2016-12-10 13:08 ` [PATCH v8 1/8] soc: samsung: add exynos chipid driver support Pankaj Dubey
2016-12-10 13:08 ` [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-16 17:38 ` Krzysztof Kozlowski
2016-12-10 13:08 ` [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines Pankaj Dubey
` (5 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
This patch enables exynos_chipid driver for ARCH_EXYNOS
based SoC.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm64/Kconfig.platforms | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 72f4eac..e6aa5c2 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -53,6 +53,7 @@ config ARCH_BRCMSTB
config ARCH_EXYNOS
bool "ARMv8 based Samsung Exynos SoC family"
+ select EXYNOS_CHIPID
select COMMON_CLK_SAMSUNG
select HAVE_S3C2410_WATCHDOG if WATCHDOG
select HAVE_S3C_RTC if RTC_CLASS
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 3/8] ARM64: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
2016-12-10 13:08 ` [PATCH v8 3/8] ARM64: " Pankaj Dubey
@ 2016-12-16 17:38 ` Krzysztof Kozlowski
0 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-16 17:38 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:38PM +0530, Pankaj Dubey wrote:
> This patch enables exynos_chipid driver for ARCH_EXYNOS
> based SoC.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm64/Kconfig.platforms | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 72f4eac..e6aa5c2 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -53,6 +53,7 @@ config ARCH_BRCMSTB
>
> config ARCH_EXYNOS
> bool "ARMv8 based Samsung Exynos SoC family"
> + select EXYNOS_CHIPID
You allowed CHIPID only on ARMv7, not ARMv8.
Best regards,
Krzysztof
> select COMMON_CLK_SAMSUNG
> select HAVE_S3C2410_WATCHDOG if WATCHDOG
> select HAVE_S3C_RTC if RTC_CLASS
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
` (2 preceding siblings ...)
2016-12-10 13:08 ` [PATCH v8 3/8] ARM64: " Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-16 18:25 ` Krzysztof Kozlowski
2016-12-10 13:08 ` [PATCH v8 5/8] ARM: EXYNOS: refactor power management " Pankaj Dubey
` (4 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
To remove dependency on soc_is_exynosMMMM macros and remove multiple
checks for such macros lets refactor code in firmware.c file.
SoC specific firmware_ops are separated and registered during
exynos_firmware_init based on matching machine compatible.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/firmware.c | 100 ++++++++++++++++++++++++++++++----------
1 file changed, 75 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index fd6da54..525fbd9 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -35,6 +35,25 @@ static void exynos_save_cp15(void)
: : "cc");
}
+static int exynos3250_do_idle(unsigned long mode)
+{
+ switch (mode) {
+ case FW_DO_IDLE_AFTR:
+ writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
+ sysram_ns_base_addr + 0x24);
+ writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
+ flush_cache_all();
+ exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
+ SMC_POWERSTATE_IDLE, 0);
+ exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
+ SMC_POWERSTATE_IDLE, 0);
+ break;
+ case FW_DO_IDLE_SLEEP:
+ exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
+ }
+ return 0;
+}
+
static int exynos_do_idle(unsigned long mode)
{
switch (mode) {
@@ -44,14 +63,7 @@ static int exynos_do_idle(unsigned long mode)
writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
sysram_ns_base_addr + 0x24);
writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
- if (soc_is_exynos3250()) {
- flush_cache_all();
- exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
- SMC_POWERSTATE_IDLE, 0);
- exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
- SMC_POWERSTATE_IDLE, 0);
- } else
- exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
+ exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
break;
case FW_DO_IDLE_SLEEP:
exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
@@ -59,28 +71,25 @@ static int exynos_do_idle(unsigned long mode)
return 0;
}
-static int exynos_cpu_boot(int cpu)
+static int exynos4412_cpu_boot(int cpu)
{
/*
- * Exynos3250 doesn't need to send smc command for secondary CPU boot
- * because Exynos3250 removes WFE in secure mode.
- */
- if (soc_is_exynos3250())
- return 0;
-
- /*
* The second parameter of SMC_CMD_CPU1BOOT command means CPU id.
* But, Exynos4212 has only one secondary CPU so second parameter
* isn't used for informing secure firmware about CPU id.
*/
- if (soc_is_exynos4212())
- cpu = 0;
+ cpu = 0;
+ exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
+ return 0;
+}
+static int exynos_cpu_boot(int cpu)
+{
exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
return 0;
}
-static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
+static int exynos4412_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
void __iomem *boot_reg;
@@ -94,14 +103,24 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
* additional offset for every CPU, with Exynos4412 being the only
* exception.
*/
- if (soc_is_exynos4412())
- boot_reg += 4 * cpu;
+ boot_reg += 4 * cpu;
+ writel_relaxed(boot_addr, boot_reg);
+ return 0;
+}
+
+static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
+{
+ void __iomem *boot_reg;
+ if (!sysram_ns_base_addr)
+ return -ENODEV;
+
+ boot_reg = sysram_ns_base_addr + 0x1c;
writel_relaxed(boot_addr, boot_reg);
return 0;
}
-static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
+static int exynos4412_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
{
void __iomem *boot_reg;
@@ -109,10 +128,19 @@ static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
return -ENODEV;
boot_reg = sysram_ns_base_addr + 0x1c;
+ boot_reg += 4 * cpu;
+ *boot_addr = readl_relaxed(boot_reg);
+ return 0;
+}
+
+static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
+{
+ void __iomem *boot_reg;
- if (soc_is_exynos4412())
- boot_reg += 4 * cpu;
+ if (!sysram_ns_base_addr)
+ return -ENODEV;
+ boot_reg = sysram_ns_base_addr + 0x1c;
*boot_addr = readl_relaxed(boot_reg);
return 0;
}
@@ -148,6 +176,23 @@ static int exynos_resume(void)
return 0;
}
+static const struct firmware_ops exynos3250_firmware_ops = {
+ .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos3250_do_idle : NULL,
+ .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
+ .get_cpu_boot_addr = exynos_get_cpu_boot_addr,
+ .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
+ .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
+};
+
+static const struct firmware_ops exynos4412_firmware_ops = {
+ .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
+ .set_cpu_boot_addr = exynos4412_set_cpu_boot_addr,
+ .get_cpu_boot_addr = exynos4412_get_cpu_boot_addr,
+ .cpu_boot = exynos4412_cpu_boot,
+ .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
+ .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
+};
+
static const struct firmware_ops exynos_firmware_ops = {
.do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
.set_cpu_boot_addr = exynos_set_cpu_boot_addr,
@@ -212,7 +257,12 @@ void __init exynos_firmware_init(void)
pr_info("Running under secure firmware.\n");
- register_firmware_ops(&exynos_firmware_ops);
+ if (of_machine_is_compatible("samsung,exynos3250"))
+ register_firmware_ops(&exynos3250_firmware_ops);
+ else if (of_machine_is_compatible("samsung,exynos4412"))
+ register_firmware_ops(&exynos4412_firmware_ops);
+ else
+ register_firmware_ops(&exynos_firmware_ops);
/*
* Exynos 4 SoCs (based on Cortex A9 and equipped with L2C-310),
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines
2016-12-10 13:08 ` [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines Pankaj Dubey
@ 2016-12-16 18:25 ` Krzysztof Kozlowski
2016-12-17 3:50 ` Pankaj Dubey
0 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-16 18:25 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:39PM +0530, Pankaj Dubey wrote:
> To remove dependency on soc_is_exynosMMMM macros and remove multiple
> checks for such macros lets refactor code in firmware.c file.
> SoC specific firmware_ops are separated and registered during
> exynos_firmware_init based on matching machine compatible.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/firmware.c | 100 ++++++++++++++++++++++++++++++----------
> 1 file changed, 75 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index fd6da54..525fbd9 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -35,6 +35,25 @@ static void exynos_save_cp15(void)
> : : "cc");
> }
>
> +static int exynos3250_do_idle(unsigned long mode)
> +{
> + switch (mode) {
> + case FW_DO_IDLE_AFTR:
> + writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
> + sysram_ns_base_addr + 0x24);
> + writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
> + flush_cache_all();
> + exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
> + SMC_POWERSTATE_IDLE, 0);
> + exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
> + SMC_POWERSTATE_IDLE, 0);
> + break;
> + case FW_DO_IDLE_SLEEP:
> + exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
> + }
> + return 0;
> +}
> +
> static int exynos_do_idle(unsigned long mode)
> {
> switch (mode) {
> @@ -44,14 +63,7 @@ static int exynos_do_idle(unsigned long mode)
> writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
> sysram_ns_base_addr + 0x24);
> writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
> - if (soc_is_exynos3250()) {
> - flush_cache_all();
> - exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
> - SMC_POWERSTATE_IDLE, 0);
> - exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
> - SMC_POWERSTATE_IDLE, 0);
> - } else
> - exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
> + exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
> break;
> case FW_DO_IDLE_SLEEP:
> exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
> @@ -59,28 +71,25 @@ static int exynos_do_idle(unsigned long mode)
> return 0;
> }
>
> -static int exynos_cpu_boot(int cpu)
> +static int exynos4412_cpu_boot(int cpu)
> {
> /*
> - * Exynos3250 doesn't need to send smc command for secondary CPU boot
> - * because Exynos3250 removes WFE in secure mode.
> - */
> - if (soc_is_exynos3250())
> - return 0;
> -
> - /*
> * The second parameter of SMC_CMD_CPU1BOOT command means CPU id.
> * But, Exynos4212 has only one secondary CPU so second parameter
> * isn't used for informing secure firmware about CPU id.
> */
> - if (soc_is_exynos4212())
> - cpu = 0;
> + cpu = 0;
Why are you clearing the cpu for Exynos4412? Was it tested on
Exynos4412?
> + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
> + return 0;
> +}
>
> +static int exynos_cpu_boot(int cpu)
> +{
> exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
This will be executed on Exynos4212...
> return 0;
> }
>
> -static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> +static int exynos4412_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> {
> void __iomem *boot_reg;
>
> @@ -94,14 +103,24 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> * additional offset for every CPU, with Exynos4412 being the only
> * exception.
> */
> - if (soc_is_exynos4412())
> - boot_reg += 4 * cpu;
> + boot_reg += 4 * cpu;
> + writel_relaxed(boot_addr, boot_reg);
> + return 0;
> +}
> +
> +static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> +{
> + void __iomem *boot_reg;
>
> + if (!sysram_ns_base_addr)
> + return -ENODEV;
> +
> + boot_reg = sysram_ns_base_addr + 0x1c;
> writel_relaxed(boot_addr, boot_reg);
> return 0;
> }
>
> -static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> +static int exynos4412_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> {
> void __iomem *boot_reg;
>
> @@ -109,10 +128,19 @@ static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> return -ENODEV;
>
> boot_reg = sysram_ns_base_addr + 0x1c;
> + boot_reg += 4 * cpu;
> + *boot_addr = readl_relaxed(boot_reg);
> + return 0;
> +}
> +
> +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
> +{
> + void __iomem *boot_reg;
>
> - if (soc_is_exynos4412())
> - boot_reg += 4 * cpu;
> + if (!sysram_ns_base_addr)
> + return -ENODEV;
>
> + boot_reg = sysram_ns_base_addr + 0x1c;
> *boot_addr = readl_relaxed(boot_reg);
> return 0;
> }
> @@ -148,6 +176,23 @@ static int exynos_resume(void)
> return 0;
> }
>
> +static const struct firmware_ops exynos3250_firmware_ops = {
> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos3250_do_idle : NULL,
> + .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
> + .get_cpu_boot_addr = exynos_get_cpu_boot_addr,
You know that lack of cpu_boot() is not equivalent to previous
'return 0' code? Now -ENOSYS will be returned... which is not a problem
because return values for cpu_boot are ignored... just wondering whether
this was planned.
> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
> +};
> +
> +static const struct firmware_ops exynos4412_firmware_ops = {
> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
> + .set_cpu_boot_addr = exynos4412_set_cpu_boot_addr,
> + .get_cpu_boot_addr = exynos4412_get_cpu_boot_addr,
> + .cpu_boot = exynos4412_cpu_boot,
> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
> +};
> +
> static const struct firmware_ops exynos_firmware_ops = {
> .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
> .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
> @@ -212,7 +257,12 @@ void __init exynos_firmware_init(void)
>
> pr_info("Running under secure firmware.\n");
>
> - register_firmware_ops(&exynos_firmware_ops);
> + if (of_machine_is_compatible("samsung,exynos3250"))
> + register_firmware_ops(&exynos3250_firmware_ops);
> + else if (of_machine_is_compatible("samsung,exynos4412"))
> + register_firmware_ops(&exynos4412_firmware_ops);
> + else
> + register_firmware_ops(&exynos_firmware_ops);
I prefer one register_firmware_ops() call, so something like:
const struct firmware_ops *ops;
if (...)
ops = &exynos3250_firmware_ops;
else if ()
...
register_firmware_ops(ops);
It is a matter of taste but for me it is more common pattern, looks more
readable and it reduces number of callers to register_firmware_ops() (so
it is easier to find them).
Krzysztof
>
> /*
> * Exynos 4 SoCs (based on Cortex A9 and equipped with L2C-310),
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines
2016-12-16 18:25 ` Krzysztof Kozlowski
@ 2016-12-17 3:50 ` Pankaj Dubey
0 siblings, 0 replies; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-17 3:50 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
On 16 December 2016 at 23:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:39PM +0530, Pankaj Dubey wrote:
>> To remove dependency on soc_is_exynosMMMM macros and remove multiple
>> checks for such macros lets refactor code in firmware.c file.
>> SoC specific firmware_ops are separated and registered during
>> exynos_firmware_init based on matching machine compatible.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/firmware.c | 100 ++++++++++++++++++++++++++++++----------
>> 1 file changed, 75 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
>> index fd6da54..525fbd9 100644
>> --- a/arch/arm/mach-exynos/firmware.c
>> +++ b/arch/arm/mach-exynos/firmware.c
>> @@ -35,6 +35,25 @@ static void exynos_save_cp15(void)
>> : : "cc");
>> }
>>
>> +static int exynos3250_do_idle(unsigned long mode)
>> +{
>> + switch (mode) {
>> + case FW_DO_IDLE_AFTR:
>> + writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
>> + sysram_ns_base_addr + 0x24);
>> + writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
>> + flush_cache_all();
>> + exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
>> + SMC_POWERSTATE_IDLE, 0);
>> + exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
>> + SMC_POWERSTATE_IDLE, 0);
>> + break;
>> + case FW_DO_IDLE_SLEEP:
>> + exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
>> + }
>> + return 0;
>> +}
>> +
>> static int exynos_do_idle(unsigned long mode)
>> {
>> switch (mode) {
>> @@ -44,14 +63,7 @@ static int exynos_do_idle(unsigned long mode)
>> writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
>> sysram_ns_base_addr + 0x24);
>> writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
>> - if (soc_is_exynos3250()) {
>> - flush_cache_all();
>> - exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
>> - SMC_POWERSTATE_IDLE, 0);
>> - exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
>> - SMC_POWERSTATE_IDLE, 0);
>> - } else
>> - exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
>> + exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
>> break;
>> case FW_DO_IDLE_SLEEP:
>> exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
>> @@ -59,28 +71,25 @@ static int exynos_do_idle(unsigned long mode)
>> return 0;
>> }
>>
>> -static int exynos_cpu_boot(int cpu)
>> +static int exynos4412_cpu_boot(int cpu)
>> {
>> /*
>> - * Exynos3250 doesn't need to send smc command for secondary CPU boot
>> - * because Exynos3250 removes WFE in secure mode.
>> - */
>> - if (soc_is_exynos3250())
>> - return 0;
>> -
>> - /*
>> * The second parameter of SMC_CMD_CPU1BOOT command means CPU id.
>> * But, Exynos4212 has only one secondary CPU so second parameter
>> * isn't used for informing secure firmware about CPU id.
>> */
>> - if (soc_is_exynos4212())
>> - cpu = 0;
>> + cpu = 0;
>
> Why are you clearing the cpu for Exynos4412? Was it tested on
> Exynos4412?
>
No I have not tested on Exynos4412.
I can see I missed this, and we are suppose clear the cpu only for Exynos4212.
I will fix this in v9 and resubmit again. Thanks for noticing this and
pointing out.
>> + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>> + return 0;
>> +}
>>
>> +static int exynos_cpu_boot(int cpu)
>> +{
>> exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>
> This will be executed on Exynos4212...
>
Yes, which is wrong. This should be for Exynos4412 and previous one
(exynos4412_cpu_boot) is applicable for Exynos4212. I will fix this in v9.
>> return 0;
>> }
>>
>> -static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> +static int exynos4412_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> {
>> void __iomem *boot_reg;
>>
>> @@ -94,14 +103,24 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> * additional offset for every CPU, with Exynos4412 being the only
>> * exception.
>> */
>> - if (soc_is_exynos4412())
>> - boot_reg += 4 * cpu;
>> + boot_reg += 4 * cpu;
>> + writel_relaxed(boot_addr, boot_reg);
>> + return 0;
>> +}
>> +
>> +static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> +{
>> + void __iomem *boot_reg;
>>
>> + if (!sysram_ns_base_addr)
>> + return -ENODEV;
>> +
>> + boot_reg = sysram_ns_base_addr + 0x1c;
>> writel_relaxed(boot_addr, boot_reg);
>> return 0;
>> }
>>
>> -static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> +static int exynos4412_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> {
>> void __iomem *boot_reg;
>>
>> @@ -109,10 +128,19 @@ static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> return -ENODEV;
>>
>> boot_reg = sysram_ns_base_addr + 0x1c;
>> + boot_reg += 4 * cpu;
>> + *boot_addr = readl_relaxed(boot_reg);
>> + return 0;
>> +}
>> +
>> +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> +{
>> + void __iomem *boot_reg;
>>
>> - if (soc_is_exynos4412())
>> - boot_reg += 4 * cpu;
>> + if (!sysram_ns_base_addr)
>> + return -ENODEV;
>>
>> + boot_reg = sysram_ns_base_addr + 0x1c;
>> *boot_addr = readl_relaxed(boot_reg);
>> return 0;
>> }
>> @@ -148,6 +176,23 @@ static int exynos_resume(void)
>> return 0;
>> }
>>
>> +static const struct firmware_ops exynos3250_firmware_ops = {
>> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos3250_do_idle : NULL,
>> + .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
>> + .get_cpu_boot_addr = exynos_get_cpu_boot_addr,
>
> You know that lack of cpu_boot() is not equivalent to previous
> 'return 0' code? Now -ENOSYS will be returned... which is not a problem
> because return values for cpu_boot are ignored... just wondering whether
> this was planned.
Yes, I feel it should return -ENOSYS, if the particular ops is not
relevant or applicable
for some SoC, rather having blank implementation and returning 0 is
should return error
code.
>
>> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
>> +};
>> +
>> +static const struct firmware_ops exynos4412_firmware_ops = {
>> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>> + .set_cpu_boot_addr = exynos4412_set_cpu_boot_addr,
>> + .get_cpu_boot_addr = exynos4412_get_cpu_boot_addr,
>> + .cpu_boot = exynos4412_cpu_boot,
>> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
>> +};
>> +
>> static const struct firmware_ops exynos_firmware_ops = {
>> .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>> .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
>> @@ -212,7 +257,12 @@ void __init exynos_firmware_init(void)
>>
>> pr_info("Running under secure firmware.\n");
>>
>> - register_firmware_ops(&exynos_firmware_ops);
>> + if (of_machine_is_compatible("samsung,exynos3250"))
>> + register_firmware_ops(&exynos3250_firmware_ops);
>> + else if (of_machine_is_compatible("samsung,exynos4412"))
>> + register_firmware_ops(&exynos4412_firmware_ops);
>> + else
>> + register_firmware_ops(&exynos_firmware_ops);
>
> I prefer one register_firmware_ops() call, so something like:
> const struct firmware_ops *ops;
> if (...)
> ops = &exynos3250_firmware_ops;
> else if ()
> ...
> register_firmware_ops(ops);
>
> It is a matter of taste but for me it is more common pattern, looks more
> readable and it reduces number of callers to register_firmware_ops() (so
> it is easier to find them).
>
This suggestion looks good to me as well. Will adopt this in v9.
Thanks for your review.
Pankaj Dubey
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 5/8] ARM: EXYNOS: refactor power management specific routines
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
` (3 preceding siblings ...)
2016-12-10 13:08 ` [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-17 14:54 ` Krzysztof Kozlowski
2016-12-10 13:08 ` [PATCH v8 6/8] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus Pankaj Dubey
` (3 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
To remove dependency on soc_is_exynosMMMM macros and remove
multiple checks for such macros lets refactor code in pm.c
This patch matches SoC specific information via private data
of soc_device_attribute and initialize it at one place and then
uses it all other places
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/pm.c | 185 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 155 insertions(+), 30 deletions(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c0b46c3..d43bea8 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -20,6 +20,7 @@
#include <linux/of.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
#include <linux/soc/samsung/exynos-pmu.h>
+#include <linux/sys_soc.h>
#include <asm/firmware.h>
#include <asm/smp_scu.h>
@@ -28,22 +29,49 @@
#include "common.h"
+struct exynos_s2r_data {
+ void __iomem* (*boot_vector_addr)(void);
+ void __iomem* (*boot_vector_flag)(void);
+ void (*set_wakeupmask)(void);
+ void (*enter_aftr)(void);
+};
+
+static const struct exynos_s2r_data *s2r_data;
+
+static void __iomem *exynos4210_rev11_boot_vector_addr(void)
+{
+ return pmu_base_addr + S5P_INFORM7;
+}
+
+static void __iomem *exynos4210_rev10_boot_vector_addr(void)
+{
+ return sysram_base_addr + 0x24;
+}
+
static inline void __iomem *exynos_boot_vector_addr(void)
{
- if (samsung_rev() == EXYNOS4210_REV_1_1)
- return pmu_base_addr + S5P_INFORM7;
- else if (samsung_rev() == EXYNOS4210_REV_1_0)
- return sysram_base_addr + 0x24;
- return pmu_base_addr + S5P_INFORM0;
+ if (s2r_data && s2r_data->boot_vector_addr)
+ return s2r_data->boot_vector_addr();
+ else
+ return pmu_base_addr + S5P_INFORM0;
+}
+
+static void __iomem *exynos4210_rev11_boot_vector_flag(void)
+{
+ return pmu_base_addr + S5P_INFORM6;
+}
+
+static void __iomem *exynos4210_rev10_boot_vector_flag(void)
+{
+ return sysram_base_addr + 0x20;
}
static inline void __iomem *exynos_boot_vector_flag(void)
{
- if (samsung_rev() == EXYNOS4210_REV_1_1)
- return pmu_base_addr + S5P_INFORM6;
- else if (samsung_rev() == EXYNOS4210_REV_1_0)
- return sysram_base_addr + 0x20;
- return pmu_base_addr + S5P_INFORM1;
+ if (s2r_data && s2r_data->boot_vector_flag)
+ return s2r_data->boot_vector_flag();
+ else
+ return pmu_base_addr + S5P_INFORM1;
}
#define S5P_CHECK_AFTR 0xFCBA0D10
@@ -120,12 +148,19 @@ int exynos_pm_central_resume(void)
return 0;
}
+static void exynos3250_set_wakeupmask(void)
+{
+ pmu_raw_writel(0x40003ffe, S5P_WAKEUP_MASK);
+ pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
+}
+
/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
-static void exynos_set_wakeupmask(long mask)
+static void exynos_set_wakeupmask(void)
{
- pmu_raw_writel(mask, S5P_WAKEUP_MASK);
- if (soc_is_exynos3250())
- pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
+ if (s2r_data && s2r_data->set_wakeupmask)
+ s2r_data->set_wakeupmask();
+ else
+ pmu_raw_writel(0x0000ff3e, S5P_WAKEUP_MASK);
}
static void exynos_cpu_set_boot_vector(long flags)
@@ -139,7 +174,7 @@ static int exynos_aftr_finisher(unsigned long flags)
{
int ret;
- exynos_set_wakeupmask(soc_is_exynos3250() ? 0x40003ffe : 0x0000ff3e);
+ exynos_set_wakeupmask();
/* Set value of power down register for aftr mode */
exynos_sys_powerdown_conf(SYS_AFTR);
@@ -154,23 +189,30 @@ static int exynos_aftr_finisher(unsigned long flags)
return 1;
}
-void exynos_enter_aftr(void)
+static void exynos3250_enter_aftr(void)
{
unsigned int cpuid = smp_processor_id();
cpu_pm_enter();
- if (soc_is_exynos3250())
- exynos_set_boot_flag(cpuid, C2_STATE);
+ exynos_set_boot_flag(cpuid, C2_STATE);
+ exynos_pm_central_suspend();
+ cpu_suspend(0, exynos_aftr_finisher);
+ exynos_pm_central_resume();
+ exynos_clear_boot_flag(cpuid, C2_STATE);
+
+ cpu_pm_exit();
+}
+
+void exynos4x12_enter_aftr(void)
+{
+ cpu_pm_enter();
exynos_pm_central_suspend();
- if (of_machine_is_compatible("samsung,exynos4212") ||
- of_machine_is_compatible("samsung,exynos4412")) {
- /* Setting SEQ_OPTION register */
- pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0,
- S5P_CENTRAL_SEQ_OPTION);
- }
+ /* Setting SEQ_OPTION register */
+ pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0,
+ S5P_CENTRAL_SEQ_OPTION);
cpu_suspend(0, exynos_aftr_finisher);
@@ -182,12 +224,95 @@ void exynos_enter_aftr(void)
exynos_pm_central_resume();
- if (soc_is_exynos3250())
- exynos_clear_boot_flag(cpuid, C2_STATE);
+ cpu_pm_exit();
+}
+
+void exynos_common_enter_aftr(void)
+{
+ cpu_pm_enter();
+
+ exynos_pm_central_suspend();
+ cpu_suspend(0, exynos_aftr_finisher);
+
+ if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
+ exynos_scu_enable();
+ if (call_firmware_op(resume) == -ENOSYS)
+ exynos_cpu_restore_register();
+ }
+
+ exynos_pm_central_resume();
cpu_pm_exit();
}
+void exynos_enter_aftr(void)
+{
+ if (s2r_data && s2r_data->enter_aftr)
+ s2r_data->enter_aftr();
+}
+
+static const struct exynos_s2r_data exynos_common_s2r_data = {
+ .enter_aftr = exynos_common_enter_aftr,
+};
+
+static const struct exynos_s2r_data exynos3250_s2r_data = {
+ .set_wakeupmask = exynos3250_set_wakeupmask,
+ .enter_aftr = exynos3250_enter_aftr,
+};
+
+static const struct exynos_s2r_data exynos4210_rev11_s2r_data = {
+ .boot_vector_addr = exynos4210_rev11_boot_vector_addr,
+ .boot_vector_flag = exynos4210_rev11_boot_vector_flag,
+ .enter_aftr = exynos_common_enter_aftr,
+};
+
+static const struct exynos_s2r_data exynos4210_rev10_s2r_data = {
+ .boot_vector_addr = exynos4210_rev10_boot_vector_addr,
+ .boot_vector_flag = exynos4210_rev10_boot_vector_flag,
+ .enter_aftr = exynos_common_enter_aftr,
+};
+
+static const struct exynos_s2r_data exynos4x12_s2r_data = {
+ .enter_aftr = exynos4x12_enter_aftr,
+};
+
+static const struct soc_device_attribute exynos_soc_revision[] = {
+ { .soc_id = "EXYNOS3250", .data = &exynos3250_s2r_data },
+ {
+ .soc_id = "EXYNOS4210", .revision = "11",
+ .data = &exynos4210_rev11_s2r_data
+ },
+ {
+ .soc_id = "EXYNOS4210", .revision = "10",
+ .data = &exynos4210_rev10_s2r_data
+ },
+ { .soc_id = "EXYNOS4212", .data = &exynos4x12_s2r_data },
+ { .soc_id = "EXYNOS4412", .data = &exynos4x12_s2r_data },
+ { .soc_id = "EXYNOS4415", .data = &exynos_common_s2r_data },
+ { .soc_id = "EXYNOS5250", .data = &exynos_common_s2r_data },
+ { .soc_id = "EXYNOS5260", .data = &exynos_common_s2r_data },
+ { .soc_id = "EXYNOS5410", .data = &exynos_common_s2r_data },
+ { .soc_id = "EXYNOS5420", .data = &exynos_common_s2r_data },
+ { .soc_id = "EXYNOS5440", .data = &exynos_common_s2r_data },
+ { .soc_id = "EXYNOS5800", .data = &exynos_common_s2r_data },
+};
+
+int __init exynos_s2r_init(void)
+{
+ const struct soc_device_attribute *match;
+
+ match = soc_device_match(exynos_soc_revision);
+
+ if (match)
+ s2r_data = (const struct exynos_s2r_data *) match->data;
+
+ if (!s2r_data)
+ return -ENODEV;
+
+ return 0;
+}
+arch_initcall(exynos_s2r_init);
+
#if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
static atomic_t cpu1_wakeup = ATOMIC_INIT(0);
@@ -253,7 +378,7 @@ static int exynos_cpu0_enter_aftr(void)
while (exynos_cpu_power_state(1) != S5P_CORE_LOCAL_PWR_EN)
cpu_relax();
- if (soc_is_exynos3250()) {
+ if (of_machine_is_compatible("samsung,exynos3250")) {
while (!pmu_raw_readl(S5P_PMU_SPARE2) &&
!atomic_read(&cpu1_wakeup))
cpu_relax();
@@ -275,7 +400,7 @@ static int exynos_cpu0_enter_aftr(void)
call_firmware_op(cpu_boot, 1);
- if (soc_is_exynos3250())
+ if (of_machine_is_compatible("samsung,exynos3250"))
dsb_sev();
else
arch_send_wakeup_ipi_mask(cpumask_of(1));
@@ -287,7 +412,7 @@ static int exynos_cpu0_enter_aftr(void)
static int exynos_wfi_finisher(unsigned long flags)
{
- if (soc_is_exynos3250())
+ if (of_machine_is_compatible("samsung,exynos3250"))
flush_cache_all();
cpu_do_idle();
@@ -309,7 +434,7 @@ static int exynos_cpu1_powerdown(void)
*/
exynos_cpu_power_down(1);
- if (soc_is_exynos3250())
+ if (of_machine_is_compatible("samsung,exynos3250"))
pmu_raw_writel(0, S5P_PMU_SPARE2);
ret = cpu_suspend(0, exynos_wfi_finisher);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 5/8] ARM: EXYNOS: refactor power management specific routines
2016-12-10 13:08 ` [PATCH v8 5/8] ARM: EXYNOS: refactor power management " Pankaj Dubey
@ 2016-12-17 14:54 ` Krzysztof Kozlowski
0 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-17 14:54 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:40PM +0530, Pankaj Dubey wrote:
> To remove dependency on soc_is_exynosMMMM macros and remove
> multiple checks for such macros lets refactor code in pm.c
>
> This patch matches SoC specific information via private data
> of soc_device_attribute and initialize it at one place and then
> uses it all other places
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/pm.c | 185 ++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 155 insertions(+), 30 deletions(-)
>
Too many things in one patch and all the renaming make reviewing more
difficult. Could you split some of self-contained changes to separate
patches? At least last soc_is_exynos3250->of_machine_is_compatible is
nicely separatable. I also wonder whether it made sense to split it
more into:
1. Introduce exynos_s2r_data() and one instance - generic, with the same
code (and with soc_is_...()).
2. Introduce new exynos_s2r_data instances and get remove soc_is_...()?
Best regards,
Krzysztof
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c0b46c3..d43bea8 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -20,6 +20,7 @@
> #include <linux/of.h>
> #include <linux/soc/samsung/exynos-regs-pmu.h>
> #include <linux/soc/samsung/exynos-pmu.h>
> +#include <linux/sys_soc.h>
>
> #include <asm/firmware.h>
> #include <asm/smp_scu.h>
> @@ -28,22 +29,49 @@
>
> #include "common.h"
>
> +struct exynos_s2r_data {
> + void __iomem* (*boot_vector_addr)(void);
> + void __iomem* (*boot_vector_flag)(void);
> + void (*set_wakeupmask)(void);
> + void (*enter_aftr)(void);
> +};
> +
> +static const struct exynos_s2r_data *s2r_data;
> +
> +static void __iomem *exynos4210_rev11_boot_vector_addr(void)
> +{
> + return pmu_base_addr + S5P_INFORM7;
> +}
> +
> +static void __iomem *exynos4210_rev10_boot_vector_addr(void)
> +{
> + return sysram_base_addr + 0x24;
> +}
> +
> static inline void __iomem *exynos_boot_vector_addr(void)
> {
> - if (samsung_rev() == EXYNOS4210_REV_1_1)
> - return pmu_base_addr + S5P_INFORM7;
> - else if (samsung_rev() == EXYNOS4210_REV_1_0)
> - return sysram_base_addr + 0x24;
> - return pmu_base_addr + S5P_INFORM0;
> + if (s2r_data && s2r_data->boot_vector_addr)
> + return s2r_data->boot_vector_addr();
> + else
> + return pmu_base_addr + S5P_INFORM0;
> +}
> +
> +static void __iomem *exynos4210_rev11_boot_vector_flag(void)
> +{
> + return pmu_base_addr + S5P_INFORM6;
> +}
> +
> +static void __iomem *exynos4210_rev10_boot_vector_flag(void)
> +{
> + return sysram_base_addr + 0x20;
> }
>
> static inline void __iomem *exynos_boot_vector_flag(void)
> {
> - if (samsung_rev() == EXYNOS4210_REV_1_1)
> - return pmu_base_addr + S5P_INFORM6;
> - else if (samsung_rev() == EXYNOS4210_REV_1_0)
> - return sysram_base_addr + 0x20;
> - return pmu_base_addr + S5P_INFORM1;
> + if (s2r_data && s2r_data->boot_vector_flag)
> + return s2r_data->boot_vector_flag();
> + else
> + return pmu_base_addr + S5P_INFORM1;
> }
>
> #define S5P_CHECK_AFTR 0xFCBA0D10
> @@ -120,12 +148,19 @@ int exynos_pm_central_resume(void)
> return 0;
> }
>
> +static void exynos3250_set_wakeupmask(void)
> +{
> + pmu_raw_writel(0x40003ffe, S5P_WAKEUP_MASK);
> + pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
> +}
> +
> /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> -static void exynos_set_wakeupmask(long mask)
> +static void exynos_set_wakeupmask(void)
> {
> - pmu_raw_writel(mask, S5P_WAKEUP_MASK);
> - if (soc_is_exynos3250())
> - pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
> + if (s2r_data && s2r_data->set_wakeupmask)
> + s2r_data->set_wakeupmask();
> + else
> + pmu_raw_writel(0x0000ff3e, S5P_WAKEUP_MASK);
> }
>
> static void exynos_cpu_set_boot_vector(long flags)
> @@ -139,7 +174,7 @@ static int exynos_aftr_finisher(unsigned long flags)
> {
> int ret;
>
> - exynos_set_wakeupmask(soc_is_exynos3250() ? 0x40003ffe : 0x0000ff3e);
> + exynos_set_wakeupmask();
> /* Set value of power down register for aftr mode */
> exynos_sys_powerdown_conf(SYS_AFTR);
>
> @@ -154,23 +189,30 @@ static int exynos_aftr_finisher(unsigned long flags)
> return 1;
> }
>
> -void exynos_enter_aftr(void)
> +static void exynos3250_enter_aftr(void)
> {
> unsigned int cpuid = smp_processor_id();
>
> cpu_pm_enter();
>
> - if (soc_is_exynos3250())
> - exynos_set_boot_flag(cpuid, C2_STATE);
> + exynos_set_boot_flag(cpuid, C2_STATE);
> + exynos_pm_central_suspend();
> + cpu_suspend(0, exynos_aftr_finisher);
> + exynos_pm_central_resume();
> + exynos_clear_boot_flag(cpuid, C2_STATE);
> +
> + cpu_pm_exit();
> +}
> +
> +void exynos4x12_enter_aftr(void)
> +{
> + cpu_pm_enter();
>
> exynos_pm_central_suspend();
>
> - if (of_machine_is_compatible("samsung,exynos4212") ||
> - of_machine_is_compatible("samsung,exynos4412")) {
> - /* Setting SEQ_OPTION register */
> - pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0,
> - S5P_CENTRAL_SEQ_OPTION);
> - }
> + /* Setting SEQ_OPTION register */
> + pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0,
> + S5P_CENTRAL_SEQ_OPTION);
>
> cpu_suspend(0, exynos_aftr_finisher);
>
> @@ -182,12 +224,95 @@ void exynos_enter_aftr(void)
>
> exynos_pm_central_resume();
>
> - if (soc_is_exynos3250())
> - exynos_clear_boot_flag(cpuid, C2_STATE);
> + cpu_pm_exit();
> +}
> +
> +void exynos_common_enter_aftr(void)
> +{
> + cpu_pm_enter();
> +
> + exynos_pm_central_suspend();
> + cpu_suspend(0, exynos_aftr_finisher);
> +
> + if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
> + exynos_scu_enable();
> + if (call_firmware_op(resume) == -ENOSYS)
> + exynos_cpu_restore_register();
> + }
> +
> + exynos_pm_central_resume();
>
> cpu_pm_exit();
> }
>
> +void exynos_enter_aftr(void)
> +{
> + if (s2r_data && s2r_data->enter_aftr)
> + s2r_data->enter_aftr();
> +}
> +
> +static const struct exynos_s2r_data exynos_common_s2r_data = {
> + .enter_aftr = exynos_common_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos3250_s2r_data = {
> + .set_wakeupmask = exynos3250_set_wakeupmask,
> + .enter_aftr = exynos3250_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos4210_rev11_s2r_data = {
> + .boot_vector_addr = exynos4210_rev11_boot_vector_addr,
> + .boot_vector_flag = exynos4210_rev11_boot_vector_flag,
> + .enter_aftr = exynos_common_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos4210_rev10_s2r_data = {
> + .boot_vector_addr = exynos4210_rev10_boot_vector_addr,
> + .boot_vector_flag = exynos4210_rev10_boot_vector_flag,
> + .enter_aftr = exynos_common_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos4x12_s2r_data = {
> + .enter_aftr = exynos4x12_enter_aftr,
> +};
> +
> +static const struct soc_device_attribute exynos_soc_revision[] = {
> + { .soc_id = "EXYNOS3250", .data = &exynos3250_s2r_data },
> + {
> + .soc_id = "EXYNOS4210", .revision = "11",
> + .data = &exynos4210_rev11_s2r_data
> + },
> + {
> + .soc_id = "EXYNOS4210", .revision = "10",
> + .data = &exynos4210_rev10_s2r_data
> + },
> + { .soc_id = "EXYNOS4212", .data = &exynos4x12_s2r_data },
> + { .soc_id = "EXYNOS4412", .data = &exynos4x12_s2r_data },
> + { .soc_id = "EXYNOS4415", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5250", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5260", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5410", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5420", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5440", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5800", .data = &exynos_common_s2r_data },
> +};
> +
> +int __init exynos_s2r_init(void)
> +{
> + const struct soc_device_attribute *match;
> +
> + match = soc_device_match(exynos_soc_revision);
> +
> + if (match)
> + s2r_data = (const struct exynos_s2r_data *) match->data;
> +
> + if (!s2r_data)
> + return -ENODEV;
> +
> + return 0;
> +}
> +arch_initcall(exynos_s2r_init);
> +
> #if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
> static atomic_t cpu1_wakeup = ATOMIC_INIT(0);
>
> @@ -253,7 +378,7 @@ static int exynos_cpu0_enter_aftr(void)
> while (exynos_cpu_power_state(1) != S5P_CORE_LOCAL_PWR_EN)
> cpu_relax();
>
> - if (soc_is_exynos3250()) {
> + if (of_machine_is_compatible("samsung,exynos3250")) {
> while (!pmu_raw_readl(S5P_PMU_SPARE2) &&
> !atomic_read(&cpu1_wakeup))
> cpu_relax();
> @@ -275,7 +400,7 @@ static int exynos_cpu0_enter_aftr(void)
>
> call_firmware_op(cpu_boot, 1);
>
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> dsb_sev();
> else
> arch_send_wakeup_ipi_mask(cpumask_of(1));
> @@ -287,7 +412,7 @@ static int exynos_cpu0_enter_aftr(void)
>
> static int exynos_wfi_finisher(unsigned long flags)
> {
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> flush_cache_all();
> cpu_do_idle();
>
> @@ -309,7 +434,7 @@ static int exynos_cpu1_powerdown(void)
> */
> exynos_cpu_power_down(1);
>
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> pmu_raw_writel(0, S5P_PMU_SPARE2);
>
> ret = cpu_suspend(0, exynos_wfi_finisher);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 6/8] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
` (4 preceding siblings ...)
2016-12-10 13:08 ` [PATCH v8 5/8] ARM: EXYNOS: refactor power management " Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-17 18:48 ` Krzysztof Kozlowski
2016-12-10 13:08 ` [PATCH v8 7/8] ARM: EXYNOS: refactor smp specific code and routines Pankaj Dubey
` (2 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
We are taking care of setting secondary cpu boot address in
exynos_boot_secondary just before sending ipi to secondary CPUs,
so we can safely remove this setting from smp_prepare_cpus.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/platsmp.c | 25 -------------------------
1 file changed, 25 deletions(-)
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 43eec10..4de254e 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -403,8 +403,6 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
{
- int i;
-
exynos_sysram_init();
exynos_set_delayed_reset_assertion(true);
@@ -414,29 +412,6 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
if (exynos_scu_enable())
return;
}
- /*
- * Write the address of secondary startup into the
- * system-wide flags register. The boot monitor waits
- * until it receives a soft interrupt, and then the
- * secondary CPU branches to this address.
- *
- * Try using firmware operation first and fall back to
- * boot register if it fails.
- */
- for (i = 1; i < max_cpus; ++i) {
- unsigned long boot_addr;
- u32 mpidr;
- u32 core_id;
- int ret;
-
- mpidr = cpu_logical_map(i);
- core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
- boot_addr = virt_to_phys(exynos4_secondary_startup);
-
- ret = exynos_set_boot_addr(core_id, boot_addr);
- if (ret)
- break;
- }
}
#ifdef CONFIG_HOTPLUG_CPU
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 6/8] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus
2016-12-10 13:08 ` [PATCH v8 6/8] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus Pankaj Dubey
@ 2016-12-17 18:48 ` Krzysztof Kozlowski
2016-12-17 22:06 ` Chanwoo Choi
0 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-17 18:48 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:41PM +0530, Pankaj Dubey wrote:
> We are taking care of setting secondary cpu boot address in
> exynos_boot_secondary just before sending ipi to secondary CPUs,
> so we can safely remove this setting from smp_prepare_cpus.
Indeed setting boot address is doubled. However between them, the power
is turned on and Exynos3250 is being restarted.
It would be nice to test it on Exynos3250 (Artik5 or Gear2?) and some of
Exynos4 family. Beside that looks good to me, I just want to see some
tested-by.
Best regards,
Krzysztof
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/platsmp.c | 25 -------------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 43eec10..4de254e 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -403,8 +403,6 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
>
> static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
> {
> - int i;
> -
> exynos_sysram_init();
>
> exynos_set_delayed_reset_assertion(true);
> @@ -414,29 +412,6 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
> if (exynos_scu_enable())
> return;
> }
> - /*
> - * Write the address of secondary startup into the
> - * system-wide flags register. The boot monitor waits
> - * until it receives a soft interrupt, and then the
> - * secondary CPU branches to this address.
> - *
> - * Try using firmware operation first and fall back to
> - * boot register if it fails.
> - */
> - for (i = 1; i < max_cpus; ++i) {
> - unsigned long boot_addr;
> - u32 mpidr;
> - u32 core_id;
> - int ret;
> -
> - mpidr = cpu_logical_map(i);
> - core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> - boot_addr = virt_to_phys(exynos4_secondary_startup);
> -
> - ret = exynos_set_boot_addr(core_id, boot_addr);
> - if (ret)
> - break;
> - }
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 6/8] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus
2016-12-17 18:48 ` Krzysztof Kozlowski
@ 2016-12-17 22:06 ` Chanwoo Choi
0 siblings, 0 replies; 31+ messages in thread
From: Chanwoo Choi @ 2016-12-17 22:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
2016-12-18 3:48 GMT+09:00 Krzysztof Kozlowski <krzk@kernel.org>:
> On Sat, Dec 10, 2016 at 06:38:41PM +0530, Pankaj Dubey wrote:
>> We are taking care of setting secondary cpu boot address in
>> exynos_boot_secondary just before sending ipi to secondary CPUs,
>> so we can safely remove this setting from smp_prepare_cpus.
>
> Indeed setting boot address is doubled. However between them, the power
> is turned on and Exynos3250 is being restarted.
>
> It would be nice to test it on Exynos3250 (Artik5 or Gear2?) and some of
> Exynos4 family. Beside that looks good to me, I just want to see some
> tested-by.
I'll test it on Exynos3250(gear2,artik5) and Exynos4(trats2) on Monday
and send the test result.
Best Regards,
Chanwoo Choi
[snip]
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 7/8] ARM: EXYNOS: refactor smp specific code and routines
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
` (5 preceding siblings ...)
2016-12-10 13:08 ` [PATCH v8 6/8] ARM: EXYNOS: remove secondary startup initialization from smp_prepare_cpus Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-17 18:59 ` Krzysztof Kozlowski
2016-12-10 13:08 ` [PATCH v8 8/8] ARM: EXYNOS: refactor of mach-exynos to use chipid information Pankaj Dubey
2016-12-16 13:11 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Marek Szyprowski
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
To remove dependency on soc_is_exynosMMMM macros and remove
multiple checks for such macros lets refactor code in platsmp.c.
This patch introduces new structure as exynos_cpu_info to
separate such variable information and routines which vary from
one Exynos SoC to other SoC. During smp_prepare_cpus lets match
SoC specific information to select appropriate exynos_cpu_info
and use it in all other places
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/platsmp.c | 244 +++++++++++++++++++++++++++++++++++------
1 file changed, 210 insertions(+), 34 deletions(-)
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 4de254e..759a184 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -19,6 +19,7 @@
#include <linux/smp.h>
#include <linux/io.h>
#include <linux/of_address.h>
+#include <linux/sys_soc.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
#include <asm/cacheflush.h>
@@ -27,12 +28,26 @@
#include <asm/smp_scu.h>
#include <asm/firmware.h>
-#include <mach/map.h>
-
#include "common.h"
extern void exynos4_secondary_startup(void);
+/*
+ * struct exynos_cpu_info - Exynos CPU related info/operations
+ * @cpu_boot_reg: computes cpu boot address for requested cpu
+ * @cpu_power_down: handles cpu power down routine for requested cpu
+ * @cpu_power_up: handles cpu power up routine for requested cpu
+ * @cpu_restart: handles cpu restart routine for requested cpu
+ */
+struct exynos_cpu_info {
+ void __iomem* (*cpu_boot_reg)(u32 cpu);
+ void (*cpu_power_down)(u32 cpu);
+ void (*cpu_power_up)(u32 cpu);
+ void (*cpu_restart)(u32 cpu);
+};
+
+static const struct exynos_cpu_info *cpu_info;
+
#ifdef CONFIG_HOTPLUG_CPU
static inline void cpu_leave_lowpower(u32 core_id)
{
@@ -81,19 +96,39 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
}
#endif /* CONFIG_HOTPLUG_CPU */
-/**
- * exynos_core_power_down : power down the specified cpu
+/*
+ * exynos_cpu_power_down - power down the specified cpu
* @cpu : the cpu to power down
*
- * Power down the specified cpu. The sequence must be finished by a
- * call to cpu_do_idle()
- *
+ * The sequence must be finished by a call to cpu_do_idle()
*/
void exynos_cpu_power_down(int cpu)
{
+ if (cpu_info && cpu_info->cpu_power_down)
+ cpu_info->cpu_power_down(cpu);
+}
+
+/*
+ * exynos_common_cpu_power_down - common cpu power down routine for Exynos SoC
+ * @cpu : the cpu to power down
+ */
+static void exynos_common_cpu_power_down(u32 cpu)
+{
u32 core_conf;
- if (cpu == 0 && (soc_is_exynos5420() || soc_is_exynos5800())) {
+ core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+ core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
+ pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+}
+
+/*
+ * exynos5420_cpu_power_down - Exynos5420/Exynos5800 specific cpu power down
+ * routine
+ * @cpu : the cpu to power down
+ */
+static void exynos5420_cpu_power_down(u32 cpu)
+{
+ if (cpu == 0) {
/*
* Bypass power down for CPU0 during suspend. Check for
* the SYS_PWR_REG value to decide if we are suspending
@@ -105,24 +140,39 @@ void exynos_cpu_power_down(int cpu)
return;
}
- core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
- core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
- pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+ exynos_common_cpu_power_down(cpu);
}
/**
* exynos_cpu_power_up : power up the specified cpu
* @cpu : the cpu to power up
- *
- * Power up the specified cpu
*/
void exynos_cpu_power_up(int cpu)
{
+ if (cpu_info && cpu_info->cpu_power_up)
+ cpu_info->cpu_power_up(cpu);
+}
+
+/*
+ * exynos_common_cpu_power_up - common cpu power up routine for Exynos SoC
+ * @cpu : the cpu to power up
+ */
+static void exynos_common_cpu_power_up(u32 cpu)
+{
u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
+ pmu_raw_writel(core_conf,
+ EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+}
- if (soc_is_exynos3250())
- core_conf |= S5P_CORE_AUTOWAKEUP_EN;
+/*
+ * exynos3250_cpu_power_up - Exynos3250 specific cpu power up routine
+ * @cpu : the cpu to power down
+ */
+static void exynos3250_cpu_power_up(u32 cpu)
+{
+ u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
+ core_conf |= S5P_CORE_AUTOWAKEUP_EN;
pmu_raw_writel(core_conf,
EXYNOS_ARM_CORE_CONFIGURATION(cpu));
}
@@ -130,7 +180,6 @@ void exynos_cpu_power_up(int cpu)
/**
* exynos_cpu_power_state : returns the power state of the cpu
* @cpu : the cpu to retrieve the power state from
- *
*/
int exynos_cpu_power_state(int cpu)
{
@@ -189,39 +238,76 @@ int exynos_scu_enable(void)
return 0;
}
-static void __iomem *cpu_boot_reg_base(void)
+static inline void __iomem *exynos_cpu_boot_reg(int cpu)
+{
+ if (cpu_info && cpu_info->cpu_boot_reg)
+ return cpu_info->cpu_boot_reg(cpu);
+ return NULL;
+}
+
+static void __iomem *exynos_common_cpu_boot_reg(u32 cpu)
{
- if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
- return pmu_base_addr + S5P_INFORM5;
+ if (!sysram_base_addr)
+ return IOMEM_ERR_PTR(-ENODEV);
+
return sysram_base_addr;
}
-static inline void __iomem *cpu_boot_reg(int cpu)
+static void __iomem *exynos4210_cpu_boot_reg(u32 cpu)
{
void __iomem *boot_reg;
- boot_reg = cpu_boot_reg_base();
- if (!boot_reg)
+ if (!pmu_base_addr)
return IOMEM_ERR_PTR(-ENODEV);
- if (soc_is_exynos4412())
- boot_reg += 4*cpu;
- else if (soc_is_exynos5420() || soc_is_exynos5800())
- boot_reg += 4;
+ boot_reg = pmu_base_addr + S5P_INFORM5;
+
+ return boot_reg;
+}
+
+static void __iomem *exynos4412_cpu_boot_reg(u32 cpu)
+{
+ void __iomem *boot_reg;
+
+ if (!sysram_base_addr)
+ return IOMEM_ERR_PTR(-ENODEV);
+
+ boot_reg = sysram_base_addr;
+ boot_reg += 4*cpu;
+
+ return boot_reg;
+}
+
+static void __iomem *exynos5420_cpu_boot_reg(u32 cpu)
+{
+ void __iomem *boot_reg;
+
+ if (!sysram_base_addr)
+ return IOMEM_ERR_PTR(-ENODEV);
+
+ boot_reg = sysram_base_addr;
+ boot_reg += 4;
+
return boot_reg;
}
+/**
+ * exynos_core_restart : restart the specified cpu
+ * @core_id : the cpu to be restarted
+ */
+void exynos_core_restart(u32 core_id)
+{
+ if (cpu_info && cpu_info->cpu_restart)
+ cpu_info->cpu_restart(core_id);
+}
+
/*
* Set wake up by local power mode and execute software reset for given core.
- *
* Currently this is needed only when booting secondary CPU on Exynos3250.
*/
-void exynos_core_restart(u32 core_id)
+static void exynos3250_core_restart(u32 core_id)
{
u32 val;
- if (!of_machine_is_compatible("samsung,exynos3250"))
- return;
-
while (!pmu_raw_readl(S5P_PMU_SPARE2))
udelay(10);
udelay(10);
@@ -274,7 +360,7 @@ int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr)
if (ret && ret != -ENOSYS)
goto fail;
if (ret == -ENOSYS) {
- void __iomem *boot_reg = cpu_boot_reg(core_id);
+ void __iomem *boot_reg = exynos_cpu_boot_reg(core_id);
if (IS_ERR(boot_reg)) {
ret = PTR_ERR(boot_reg);
@@ -299,7 +385,7 @@ int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
if (ret && ret != -ENOSYS)
goto fail;
if (ret == -ENOSYS) {
- void __iomem *boot_reg = cpu_boot_reg(core_id);
+ void __iomem *boot_reg = exynos_cpu_boot_reg(core_id);
if (IS_ERR(boot_reg)) {
ret = PTR_ERR(boot_reg);
@@ -312,13 +398,93 @@ int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
return ret;
}
+static const struct exynos_cpu_info exynos3250_cpu_info = {
+ .cpu_boot_reg = exynos_common_cpu_boot_reg,
+ .cpu_power_down = exynos_common_cpu_power_down,
+ .cpu_power_up = exynos3250_cpu_power_up,
+ .cpu_restart = exynos3250_core_restart,
+};
+
+static const struct exynos_cpu_info exynos5420_cpu_info = {
+ .cpu_boot_reg = exynos5420_cpu_boot_reg,
+ .cpu_power_down = exynos5420_cpu_power_down,
+ .cpu_power_up = exynos_common_cpu_power_up,
+};
+
+static const struct exynos_cpu_info exynos4210_cpu_info = {
+ .cpu_boot_reg = exynos4210_cpu_boot_reg,
+ .cpu_power_down = exynos_common_cpu_power_down,
+ .cpu_power_up = exynos_common_cpu_power_up,
+};
+
+static const struct exynos_cpu_info exynos4412_cpu_info = {
+ .cpu_boot_reg = exynos4412_cpu_boot_reg,
+ .cpu_power_down = exynos_common_cpu_power_down,
+ .cpu_power_up = exynos_common_cpu_power_up,
+};
+
+static const struct exynos_cpu_info exynos_common_cpu_info = {
+ .cpu_boot_reg = exynos_common_cpu_boot_reg,
+ .cpu_power_down = exynos_common_cpu_power_down,
+ .cpu_power_up = exynos_common_cpu_power_up,
+};
+
+static const struct soc_device_attribute exynos_soc_revision[] __initconst = {
+ {
+ .soc_id = "EXYNOS4210", .revision = "11",
+ .data = &exynos4210_cpu_info
+ }, {
+ .soc_id = "EXYNOS4210", .revision = "10",
+ .data = &exynos_common_cpu_info
+ }
+};
+
+static const struct of_device_id exynos_pmu_of_device_ids[] __initconst = {
+ {
+ .compatible = "samsung,exynos3250",
+ .data = &exynos3250_cpu_info
+ }, {
+ .compatible = "samsung,exynos4212",
+ .data = &exynos_common_cpu_info
+ }, {
+ .compatible = "samsung,exynos4412",
+ .data = &exynos4412_cpu_info
+ }, {
+ .compatible = "samsung,exynos5250",
+ .data = &exynos_common_cpu_info
+ }, {
+ .compatible = "samsung,exynos5260",
+ .data = &exynos_common_cpu_info
+ }, {
+ .compatible = "samsung,exynos5410",
+ .data = &exynos_common_cpu_info
+ }, {
+ .compatible = "samsung,exynos5420",
+ .data = &exynos5420_cpu_info
+ }, {
+ .compatible = "samsung,exynos5440",
+ .data = &exynos_common_cpu_info
+ }, {
+ .compatible = "samsung,exynos5800",
+ .data = &exynos5420_cpu_info
+ },
+ { /*sentinel*/ },
+};
+
static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
unsigned long timeout;
+ const struct soc_device_attribute *match;
u32 mpidr = cpu_logical_map(cpu);
u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
int ret = -ENOSYS;
+ if (of_machine_is_compatible("samsung,exynos4210")) {
+ match = soc_device_match(exynos_soc_revision);
+ if (match)
+ cpu_info = (const struct exynos_cpu_info *) match->data;
+ }
+
/*
* Set synchronisation state between this boot processor
* and the secondary one
@@ -377,7 +543,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
call_firmware_op(cpu_boot, core_id);
- if (soc_is_exynos3250())
+ if (of_machine_is_compatible("samsung,exynos3250"))
dsb_sev();
else
arch_send_wakeup_ipi_mask(cpumask_of(cpu));
@@ -403,6 +569,16 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
{
+ const struct of_device_id *match;
+ struct device_node *np;
+
+ np = of_find_matching_node_and_match(NULL,
+ exynos_pmu_of_device_ids, &match);
+ if (!np)
+ pr_err("failed to find supported CPU\n");
+ else
+ cpu_info = (const struct exynos_cpu_info *) match->data;
+
exynos_sysram_init();
exynos_set_delayed_reset_assertion(true);
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 7/8] ARM: EXYNOS: refactor smp specific code and routines
2016-12-10 13:08 ` [PATCH v8 7/8] ARM: EXYNOS: refactor smp specific code and routines Pankaj Dubey
@ 2016-12-17 18:59 ` Krzysztof Kozlowski
0 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-17 18:59 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:42PM +0530, Pankaj Dubey wrote:
> To remove dependency on soc_is_exynosMMMM macros and remove
> multiple checks for such macros lets refactor code in platsmp.c.
> This patch introduces new structure as exynos_cpu_info to
> separate such variable information and routines which vary from
> one Exynos SoC to other SoC. During smp_prepare_cpus lets match
> SoC specific information to select appropriate exynos_cpu_info
> and use it in all other places
>
At first glance, looks fine. I see you deal with cpu_power_down and
disallowed use of of_xxxx() calls. One error plus one nit at then end.
However just like for patch 5, because of renames it is difficult to
spot subtle differences (and possible errors) - could you try to split
this patch a bit?
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/platsmp.c | 244 +++++++++++++++++++++++++++++++++++------
> 1 file changed, 210 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 4de254e..759a184 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -19,6 +19,7 @@
> #include <linux/smp.h>
> #include <linux/io.h>
> #include <linux/of_address.h>
> +#include <linux/sys_soc.h>
> #include <linux/soc/samsung/exynos-regs-pmu.h>
>
> #include <asm/cacheflush.h>
> @@ -27,12 +28,26 @@
> #include <asm/smp_scu.h>
> #include <asm/firmware.h>
>
> -#include <mach/map.h>
> -
> #include "common.h"
>
> extern void exynos4_secondary_startup(void);
>
> +/*
> + * struct exynos_cpu_info - Exynos CPU related info/operations
> + * @cpu_boot_reg: computes cpu boot address for requested cpu
> + * @cpu_power_down: handles cpu power down routine for requested cpu
> + * @cpu_power_up: handles cpu power up routine for requested cpu
> + * @cpu_restart: handles cpu restart routine for requested cpu
> + */
> +struct exynos_cpu_info {
> + void __iomem* (*cpu_boot_reg)(u32 cpu);
> + void (*cpu_power_down)(u32 cpu);
> + void (*cpu_power_up)(u32 cpu);
> + void (*cpu_restart)(u32 cpu);
> +};
> +
> +static const struct exynos_cpu_info *cpu_info;
> +
> #ifdef CONFIG_HOTPLUG_CPU
> static inline void cpu_leave_lowpower(u32 core_id)
> {
> @@ -81,19 +96,39 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> }
> #endif /* CONFIG_HOTPLUG_CPU */
>
> -/**
> - * exynos_core_power_down : power down the specified cpu
> +/*
> + * exynos_cpu_power_down - power down the specified cpu
> * @cpu : the cpu to power down
> *
> - * Power down the specified cpu. The sequence must be finished by a
> - * call to cpu_do_idle()
> - *
> + * The sequence must be finished by a call to cpu_do_idle()
> */
> void exynos_cpu_power_down(int cpu)
> {
> + if (cpu_info && cpu_info->cpu_power_down)
> + cpu_info->cpu_power_down(cpu);
> +}
> +
> +/*
> + * exynos_common_cpu_power_down - common cpu power down routine for Exynos SoC
> + * @cpu : the cpu to power down
> + */
> +static void exynos_common_cpu_power_down(u32 cpu)
> +{
> u32 core_conf;
>
> - if (cpu == 0 && (soc_is_exynos5420() || soc_is_exynos5800())) {
> + core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> + core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
> + pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> +}
> +
> +/*
> + * exynos5420_cpu_power_down - Exynos5420/Exynos5800 specific cpu power down
> + * routine
> + * @cpu : the cpu to power down
> + */
> +static void exynos5420_cpu_power_down(u32 cpu)
> +{
> + if (cpu == 0) {
> /*
> * Bypass power down for CPU0 during suspend. Check for
> * the SYS_PWR_REG value to decide if we are suspending
> @@ -105,24 +140,39 @@ void exynos_cpu_power_down(int cpu)
> return;
> }
>
> - core_conf = pmu_raw_readl(EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> - core_conf &= ~S5P_CORE_LOCAL_PWR_EN;
> - pmu_raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> + exynos_common_cpu_power_down(cpu);
> }
>
> /**
> * exynos_cpu_power_up : power up the specified cpu
> * @cpu : the cpu to power up
> - *
> - * Power up the specified cpu
> */
> void exynos_cpu_power_up(int cpu)
> {
> + if (cpu_info && cpu_info->cpu_power_up)
> + cpu_info->cpu_power_up(cpu);
> +}
> +
> +/*
> + * exynos_common_cpu_power_up - common cpu power up routine for Exynos SoC
> + * @cpu : the cpu to power up
> + */
> +static void exynos_common_cpu_power_up(u32 cpu)
> +{
> u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
> + pmu_raw_writel(core_conf,
> + EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> +}
>
> - if (soc_is_exynos3250())
> - core_conf |= S5P_CORE_AUTOWAKEUP_EN;
> +/*
> + * exynos3250_cpu_power_up - Exynos3250 specific cpu power up routine
> + * @cpu : the cpu to power down
> + */
> +static void exynos3250_cpu_power_up(u32 cpu)
> +{
> + u32 core_conf = S5P_CORE_LOCAL_PWR_EN;
>
> + core_conf |= S5P_CORE_AUTOWAKEUP_EN;
> pmu_raw_writel(core_conf,
> EXYNOS_ARM_CORE_CONFIGURATION(cpu));
> }
> @@ -130,7 +180,6 @@ void exynos_cpu_power_up(int cpu)
> /**
> * exynos_cpu_power_state : returns the power state of the cpu
> * @cpu : the cpu to retrieve the power state from
> - *
> */
> int exynos_cpu_power_state(int cpu)
> {
> @@ -189,39 +238,76 @@ int exynos_scu_enable(void)
> return 0;
> }
>
> -static void __iomem *cpu_boot_reg_base(void)
> +static inline void __iomem *exynos_cpu_boot_reg(int cpu)
> +{
> + if (cpu_info && cpu_info->cpu_boot_reg)
> + return cpu_info->cpu_boot_reg(cpu);
> + return NULL;
> +}
> +
> +static void __iomem *exynos_common_cpu_boot_reg(u32 cpu)
> {
> - if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
> - return pmu_base_addr + S5P_INFORM5;
> + if (!sysram_base_addr)
> + return IOMEM_ERR_PTR(-ENODEV);
> +
> return sysram_base_addr;
> }
>
> -static inline void __iomem *cpu_boot_reg(int cpu)
> +static void __iomem *exynos4210_cpu_boot_reg(u32 cpu)
> {
> void __iomem *boot_reg;
>
> - boot_reg = cpu_boot_reg_base();
> - if (!boot_reg)
> + if (!pmu_base_addr)
> return IOMEM_ERR_PTR(-ENODEV);
> - if (soc_is_exynos4412())
> - boot_reg += 4*cpu;
> - else if (soc_is_exynos5420() || soc_is_exynos5800())
> - boot_reg += 4;
> + boot_reg = pmu_base_addr + S5P_INFORM5;
> +
> + return boot_reg;
> +}
> +
> +static void __iomem *exynos4412_cpu_boot_reg(u32 cpu)
> +{
> + void __iomem *boot_reg;
> +
> + if (!sysram_base_addr)
> + return IOMEM_ERR_PTR(-ENODEV);
> +
> + boot_reg = sysram_base_addr;
> + boot_reg += 4*cpu;
> +
> + return boot_reg;
> +}
> +
> +static void __iomem *exynos5420_cpu_boot_reg(u32 cpu)
> +{
> + void __iomem *boot_reg;
> +
> + if (!sysram_base_addr)
> + return IOMEM_ERR_PTR(-ENODEV);
> +
> + boot_reg = sysram_base_addr;
> + boot_reg += 4;
> +
> return boot_reg;
> }
>
> +/**
> + * exynos_core_restart : restart the specified cpu
> + * @core_id : the cpu to be restarted
> + */
> +void exynos_core_restart(u32 core_id)
> +{
> + if (cpu_info && cpu_info->cpu_restart)
> + cpu_info->cpu_restart(core_id);
> +}
> +
> /*
> * Set wake up by local power mode and execute software reset for given core.
> - *
> * Currently this is needed only when booting secondary CPU on Exynos3250.
> */
> -void exynos_core_restart(u32 core_id)
> +static void exynos3250_core_restart(u32 core_id)
> {
> u32 val;
>
> - if (!of_machine_is_compatible("samsung,exynos3250"))
> - return;
> -
> while (!pmu_raw_readl(S5P_PMU_SPARE2))
> udelay(10);
> udelay(10);
> @@ -274,7 +360,7 @@ int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr)
> if (ret && ret != -ENOSYS)
> goto fail;
> if (ret == -ENOSYS) {
> - void __iomem *boot_reg = cpu_boot_reg(core_id);
> + void __iomem *boot_reg = exynos_cpu_boot_reg(core_id);
>
> if (IS_ERR(boot_reg)) {
> ret = PTR_ERR(boot_reg);
> @@ -299,7 +385,7 @@ int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
> if (ret && ret != -ENOSYS)
> goto fail;
> if (ret == -ENOSYS) {
> - void __iomem *boot_reg = cpu_boot_reg(core_id);
> + void __iomem *boot_reg = exynos_cpu_boot_reg(core_id);
>
> if (IS_ERR(boot_reg)) {
> ret = PTR_ERR(boot_reg);
> @@ -312,13 +398,93 @@ int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr)
> return ret;
> }
>
> +static const struct exynos_cpu_info exynos3250_cpu_info = {
> + .cpu_boot_reg = exynos_common_cpu_boot_reg,
> + .cpu_power_down = exynos_common_cpu_power_down,
> + .cpu_power_up = exynos3250_cpu_power_up,
> + .cpu_restart = exynos3250_core_restart,
> +};
> +
> +static const struct exynos_cpu_info exynos5420_cpu_info = {
> + .cpu_boot_reg = exynos5420_cpu_boot_reg,
> + .cpu_power_down = exynos5420_cpu_power_down,
> + .cpu_power_up = exynos_common_cpu_power_up,
> +};
> +
> +static const struct exynos_cpu_info exynos4210_cpu_info = {
> + .cpu_boot_reg = exynos4210_cpu_boot_reg,
> + .cpu_power_down = exynos_common_cpu_power_down,
> + .cpu_power_up = exynos_common_cpu_power_up,
> +};
> +
> +static const struct exynos_cpu_info exynos4412_cpu_info = {
> + .cpu_boot_reg = exynos4412_cpu_boot_reg,
> + .cpu_power_down = exynos_common_cpu_power_down,
> + .cpu_power_up = exynos_common_cpu_power_up,
> +};
> +
> +static const struct exynos_cpu_info exynos_common_cpu_info = {
> + .cpu_boot_reg = exynos_common_cpu_boot_reg,
> + .cpu_power_down = exynos_common_cpu_power_down,
> + .cpu_power_up = exynos_common_cpu_power_up,
> +};
> +
> +static const struct soc_device_attribute exynos_soc_revision[] __initconst = {
> + {
> + .soc_id = "EXYNOS4210", .revision = "11",
Each attribute in new line please.
> + .data = &exynos4210_cpu_info
> + }, {
> + .soc_id = "EXYNOS4210", .revision = "10",
> + .data = &exynos_common_cpu_info
> + }
> +};
> +
> +static const struct of_device_id exynos_pmu_of_device_ids[] __initconst = {
> + {
> + .compatible = "samsung,exynos3250",
> + .data = &exynos3250_cpu_info
> + }, {
> + .compatible = "samsung,exynos4212",
> + .data = &exynos_common_cpu_info
> + }, {
> + .compatible = "samsung,exynos4412",
> + .data = &exynos4412_cpu_info
> + }, {
> + .compatible = "samsung,exynos5250",
> + .data = &exynos_common_cpu_info
> + }, {
> + .compatible = "samsung,exynos5260",
> + .data = &exynos_common_cpu_info
> + }, {
> + .compatible = "samsung,exynos5410",
> + .data = &exynos_common_cpu_info
> + }, {
> + .compatible = "samsung,exynos5420",
> + .data = &exynos5420_cpu_info
> + }, {
> + .compatible = "samsung,exynos5440",
> + .data = &exynos_common_cpu_info
> + }, {
> + .compatible = "samsung,exynos5800",
> + .data = &exynos5420_cpu_info
> + },
> + { /*sentinel*/ },
> +};
> +
> static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
> {
> unsigned long timeout;
> + const struct soc_device_attribute *match;
> u32 mpidr = cpu_logical_map(cpu);
> u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> int ret = -ENOSYS;
>
> + if (of_machine_is_compatible("samsung,exynos4210")) {
> + match = soc_device_match(exynos_soc_revision);
The 'exynos_soc_revision' is __initconst but this function is not
__init.
Best regards,
Krzysztof
> + if (match)
> + cpu_info = (const struct exynos_cpu_info *) match->data;
> + }
> +
> /*
> * Set synchronisation state between this boot processor
> * and the secondary one
> @@ -377,7 +543,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
>
> call_firmware_op(cpu_boot, core_id);
>
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> dsb_sev();
> else
> arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> @@ -403,6 +569,16 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
>
> static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
> {
> + const struct of_device_id *match;
> + struct device_node *np;
> +
> + np = of_find_matching_node_and_match(NULL,
> + exynos_pmu_of_device_ids, &match);
> + if (!np)
> + pr_err("failed to find supported CPU\n");
> + else
> + cpu_info = (const struct exynos_cpu_info *) match->data;
> +
> exynos_sysram_init();
>
> exynos_set_delayed_reset_assertion(true);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 8/8] ARM: EXYNOS: refactor of mach-exynos to use chipid information
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
` (6 preceding siblings ...)
2016-12-10 13:08 ` [PATCH v8 7/8] ARM: EXYNOS: refactor smp specific code and routines Pankaj Dubey
@ 2016-12-10 13:08 ` Pankaj Dubey
2016-12-17 19:03 ` Krzysztof Kozlowski
2016-12-16 13:11 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Marek Szyprowski
8 siblings, 1 reply; 31+ messages in thread
From: Pankaj Dubey @ 2016-12-10 13:08 UTC (permalink / raw)
To: linux-arm-kernel
Since now we have chipid driver in place and all dependencies of
soc_is_exynosMMMM macros have been address, lets remove all such
macros. Also remove static mapping of chipid SFR in exynos.c and
related helper functions.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/common.h | 92 ----------------------------
arch/arm/mach-exynos/exynos.c | 38 ------------
arch/arm/mach-exynos/include/mach/map.h | 21 -------
arch/arm/plat-samsung/cpu.c | 14 -----
arch/arm/plat-samsung/include/plat/cpu.h | 2 -
arch/arm/plat-samsung/include/plat/map-s5p.h | 2 -
6 files changed, 169 deletions(-)
delete mode 100644 arch/arm/mach-exynos/include/mach/map.h
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index cfd55ba..5886646 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -14,97 +14,6 @@
#include <linux/platform_data/cpuidle-exynos.h>
-#define EXYNOS3250_SOC_ID 0xE3472000
-#define EXYNOS3_SOC_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 EXYNOS5410_SOC_ID 0xE5410000
-#define EXYNOS5420_SOC_ID 0xE5420000
-#define EXYNOS5440_SOC_ID 0xE5440000
-#define EXYNOS5800_SOC_ID 0xE5422000
-#define EXYNOS5_SOC_MASK 0xFFFFF000
-
-extern unsigned long samsung_cpu_id;
-
-#define IS_SAMSUNG_CPU(name, id, mask) \
-static inline int is_samsung_##name(void) \
-{ \
- return ((samsung_cpu_id & mask) == (id & mask)); \
-}
-
-IS_SAMSUNG_CPU(exynos3250, EXYNOS3250_SOC_ID, EXYNOS3_SOC_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(exynos5410, EXYNOS5410_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)
-IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)
-
-#if defined(CONFIG_SOC_EXYNOS3250)
-# define soc_is_exynos3250() is_samsung_exynos3250()
-#else
-# define soc_is_exynos3250() 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_EXYNOS5410)
-# define soc_is_exynos5410() is_samsung_exynos5410()
-#else
-# define soc_is_exynos5410() 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
-
-#if defined(CONFIG_SOC_EXYNOS5800)
-# define soc_is_exynos5800() is_samsung_exynos5800()
-#else
-# define soc_is_exynos5800() 0
-#endif
-
extern u32 cp15_save_diag;
extern u32 cp15_save_power;
@@ -156,7 +65,6 @@ extern struct cpuidle_exynos_data cpuidle_coupled_exynos_data;
extern void exynos_set_delayed_reset_assertion(bool enable);
-extern unsigned int samsung_rev(void);
extern void exynos_core_restart(u32 core_id);
extern int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr);
extern int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr);
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 040ea66..66bd612 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -21,10 +21,6 @@
#include <asm/cacheflush.h>
#include <asm/hardware/cache-l2x0.h>
#include <asm/mach/arch.h>
-#include <asm/mach/map.h>
-
-#include <mach/map.h>
-#include <plat/cpu.h>
#include "common.h"
@@ -58,39 +54,6 @@ void __init exynos_sysram_init(void)
}
}
-static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
- int depth, void *data)
-{
- struct map_desc iodesc;
- const __be32 *reg;
- int 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 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);
-}
-
/*
* Set or clear the USE_DELAYED_RESET_ASSERTION option. Used by smp code
* and suspend.
@@ -203,7 +166,6 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
.l2c_aux_val = 0x3c400001,
.l2c_aux_mask = 0xc20fffff,
.smp = smp_ops(exynos_smp_ops),
- .map_io = exynos_init_io,
.init_early = exynos_firmware_init,
.init_irq = exynos_init_irq,
.init_machine = exynos_dt_machine_init,
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
deleted file mode 100644
index 0eef407..0000000
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
- * http://www.samsung.com/
- *
- * EXYNOS - Memory map definitions
- *
- * 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 __ASM_ARCH_MAP_H
-#define __ASM_ARCH_MAP_H __FILE__
-
-#include <plat/map-base.h>
-
-#include <plat/map-s5p.h>
-
-#define EXYNOS_PA_CHIPID 0x10000000
-
-#endif /* __ASM_ARCH_MAP_H */
diff --git a/arch/arm/plat-samsung/cpu.c b/arch/arm/plat-samsung/cpu.c
index a107b3a..e58f0f6 100644
--- a/arch/arm/plat-samsung/cpu.c
+++ b/arch/arm/plat-samsung/cpu.c
@@ -21,12 +21,6 @@
unsigned long samsung_cpu_id;
static unsigned int samsung_cpu_rev;
-unsigned int samsung_rev(void)
-{
- return samsung_cpu_rev;
-}
-EXPORT_SYMBOL(samsung_rev);
-
void __init s3c64xx_init_cpu(void)
{
samsung_cpu_id = readl_relaxed(S3C_VA_SYS + 0x118);
@@ -43,11 +37,3 @@ void __init s3c64xx_init_cpu(void)
pr_info("Samsung CPU ID: 0x%08lx\n", samsung_cpu_id);
}
-
-void __init s5p_init_cpu(const void __iomem *cpuid_addr)
-{
- samsung_cpu_id = readl_relaxed(cpuid_addr);
- samsung_cpu_rev = samsung_cpu_id & 0xFF;
-
- pr_info("Samsung CPU ID: 0x%08lx\n", samsung_cpu_id);
-}
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
index b7b702a..913c176 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -115,8 +115,6 @@ extern void s3c24xx_init_io(struct map_desc *mach_desc, int size);
extern void s3c64xx_init_cpu(void);
extern void s5p_init_cpu(const void __iomem *cpuid_addr);
-extern unsigned int samsung_rev(void);
-
extern void s3c24xx_init_uarts(struct s3c2410_uartcfg *cfg, int no);
extern void s3c24xx_init_clocks(int xtal);
diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
index 512ed1f..d6853f1 100644
--- a/arch/arm/plat-samsung/include/plat/map-s5p.h
+++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
@@ -13,8 +13,6 @@
#ifndef __ASM_PLAT_MAP_S5P_H
#define __ASM_PLAT_MAP_S5P_H __FILE__
-#define S5P_VA_CHIPID S3C_ADDR(0x02000000)
-
#define VA_VIC(x) (S3C_VA_IRQ + ((x) * 0x10000))
#define VA_VIC0 VA_VIC(0)
#define VA_VIC1 VA_VIC(1)
--
2.7.4
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v8 8/8] ARM: EXYNOS: refactor of mach-exynos to use chipid information
2016-12-10 13:08 ` [PATCH v8 8/8] ARM: EXYNOS: refactor of mach-exynos to use chipid information Pankaj Dubey
@ 2016-12-17 19:03 ` Krzysztof Kozlowski
0 siblings, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2016-12-17 19:03 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 10, 2016 at 06:38:43PM +0530, Pankaj Dubey wrote:
> Since now we have chipid driver in place and all dependencies of
> soc_is_exynosMMMM macros have been address, lets remove all such
> macros. Also remove static mapping of chipid SFR in exynos.c and
> related helper functions.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/common.h | 92 ----------------------------
> arch/arm/mach-exynos/exynos.c | 38 ------------
> arch/arm/mach-exynos/include/mach/map.h | 21 -------
> arch/arm/plat-samsung/cpu.c | 14 -----
> arch/arm/plat-samsung/include/plat/cpu.h | 2 -
> arch/arm/plat-samsung/include/plat/map-s5p.h | 2 -
> 6 files changed, 169 deletions(-)
> delete mode 100644 arch/arm/mach-exynos/include/mach/map.h
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index cfd55ba..5886646 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -14,97 +14,6 @@
>
> #include <linux/platform_data/cpuidle-exynos.h>
>
> -#define EXYNOS3250_SOC_ID 0xE3472000
> -#define EXYNOS3_SOC_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 EXYNOS5410_SOC_ID 0xE5410000
> -#define EXYNOS5420_SOC_ID 0xE5420000
> -#define EXYNOS5440_SOC_ID 0xE5440000
> -#define EXYNOS5800_SOC_ID 0xE5422000
> -#define EXYNOS5_SOC_MASK 0xFFFFF000
> -
> -extern unsigned long samsung_cpu_id;
> -
> -#define IS_SAMSUNG_CPU(name, id, mask) \
> -static inline int is_samsung_##name(void) \
> -{ \
> - return ((samsung_cpu_id & mask) == (id & mask)); \
> -}
> -
> -IS_SAMSUNG_CPU(exynos3250, EXYNOS3250_SOC_ID, EXYNOS3_SOC_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(exynos5410, EXYNOS5410_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)
> -IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)
> -
> -#if defined(CONFIG_SOC_EXYNOS3250)
> -# define soc_is_exynos3250() is_samsung_exynos3250()
> -#else
> -# define soc_is_exynos3250() 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_EXYNOS5410)
> -# define soc_is_exynos5410() is_samsung_exynos5410()
> -#else
> -# define soc_is_exynos5410() 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
> -
> -#if defined(CONFIG_SOC_EXYNOS5800)
> -# define soc_is_exynos5800() is_samsung_exynos5800()
> -#else
> -# define soc_is_exynos5800() 0
> -#endif
> -
> extern u32 cp15_save_diag;
> extern u32 cp15_save_power;
>
> @@ -156,7 +65,6 @@ extern struct cpuidle_exynos_data cpuidle_coupled_exynos_data;
>
> extern void exynos_set_delayed_reset_assertion(bool enable);
>
> -extern unsigned int samsung_rev(void);
> extern void exynos_core_restart(u32 core_id);
> extern int exynos_set_boot_addr(u32 core_id, unsigned long boot_addr);
> extern int exynos_get_boot_addr(u32 core_id, unsigned long *boot_addr);
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 040ea66..66bd612 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -21,10 +21,6 @@
> #include <asm/cacheflush.h>
> #include <asm/hardware/cache-l2x0.h>
> #include <asm/mach/arch.h>
> -#include <asm/mach/map.h>
> -
> -#include <mach/map.h>
> -#include <plat/cpu.h>
>
> #include "common.h"
>
> @@ -58,39 +54,6 @@ void __init exynos_sysram_init(void)
> }
> }
>
> -static int __init exynos_fdt_map_chipid(unsigned long node, const char *uname,
> - int depth, void *data)
> -{
> - struct map_desc iodesc;
> - const __be32 *reg;
> - int len;
> -
> - if (!of_flat_dt_is_compatible(node, "samsung,exynos4210-chipid") &&
> - !of_flat_dt_is_compatible(node, "samsung,exynos5440-clock"))
That's the problem with 5440. It comes without chipid binding. I think
with removal of this the SoC/board would becom broken?
Best regards,
Krzysztof
> - 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 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);
> -}
> -
> /*
> * Set or clear the USE_DELAYED_RESET_ASSERTION option. Used by smp code
> * and suspend.
> @@ -203,7 +166,6 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
> .l2c_aux_val = 0x3c400001,
> .l2c_aux_mask = 0xc20fffff,
> .smp = smp_ops(exynos_smp_ops),
> - .map_io = exynos_init_io,
> .init_early = exynos_firmware_init,
> .init_irq = exynos_init_irq,
> .init_machine = exynos_dt_machine_init,
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> deleted file mode 100644
> index 0eef407..0000000
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -/*
> - * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> - * http://www.samsung.com/
> - *
> - * EXYNOS - Memory map definitions
> - *
> - * 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 __ASM_ARCH_MAP_H
> -#define __ASM_ARCH_MAP_H __FILE__
> -
> -#include <plat/map-base.h>
> -
> -#include <plat/map-s5p.h>
> -
> -#define EXYNOS_PA_CHIPID 0x10000000
> -
> -#endif /* __ASM_ARCH_MAP_H */
> diff --git a/arch/arm/plat-samsung/cpu.c b/arch/arm/plat-samsung/cpu.c
> index a107b3a..e58f0f6 100644
> --- a/arch/arm/plat-samsung/cpu.c
> +++ b/arch/arm/plat-samsung/cpu.c
> @@ -21,12 +21,6 @@
> unsigned long samsung_cpu_id;
> static unsigned int samsung_cpu_rev;
>
> -unsigned int samsung_rev(void)
> -{
> - return samsung_cpu_rev;
> -}
> -EXPORT_SYMBOL(samsung_rev);
> -
> void __init s3c64xx_init_cpu(void)
> {
> samsung_cpu_id = readl_relaxed(S3C_VA_SYS + 0x118);
> @@ -43,11 +37,3 @@ void __init s3c64xx_init_cpu(void)
>
> pr_info("Samsung CPU ID: 0x%08lx\n", samsung_cpu_id);
> }
> -
> -void __init s5p_init_cpu(const void __iomem *cpuid_addr)
> -{
> - samsung_cpu_id = readl_relaxed(cpuid_addr);
> - samsung_cpu_rev = samsung_cpu_id & 0xFF;
> -
> - pr_info("Samsung CPU ID: 0x%08lx\n", samsung_cpu_id);
> -}
> diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
> index b7b702a..913c176 100644
> --- a/arch/arm/plat-samsung/include/plat/cpu.h
> +++ b/arch/arm/plat-samsung/include/plat/cpu.h
> @@ -115,8 +115,6 @@ extern void s3c24xx_init_io(struct map_desc *mach_desc, int size);
> extern void s3c64xx_init_cpu(void);
> extern void s5p_init_cpu(const void __iomem *cpuid_addr);
>
> -extern unsigned int samsung_rev(void);
> -
> extern void s3c24xx_init_uarts(struct s3c2410_uartcfg *cfg, int no);
>
> extern void s3c24xx_init_clocks(int xtal);
> diff --git a/arch/arm/plat-samsung/include/plat/map-s5p.h b/arch/arm/plat-samsung/include/plat/map-s5p.h
> index 512ed1f..d6853f1 100644
> --- a/arch/arm/plat-samsung/include/plat/map-s5p.h
> +++ b/arch/arm/plat-samsung/include/plat/map-s5p.h
> @@ -13,8 +13,6 @@
> #ifndef __ASM_PLAT_MAP_S5P_H
> #define __ASM_PLAT_MAP_S5P_H __FILE__
>
> -#define S5P_VA_CHIPID S3C_ADDR(0x02000000)
> -
> #define VA_VIC(x) (S3C_VA_IRQ + ((x) * 0x10000))
> #define VA_VIC0 VA_VIC(0)
> #define VA_VIC1 VA_VIC(1)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v8 0/8] Introducing Exynos ChipId driver
2016-12-10 13:08 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Pankaj Dubey
` (7 preceding siblings ...)
2016-12-10 13:08 ` [PATCH v8 8/8] ARM: EXYNOS: refactor of mach-exynos to use chipid information Pankaj Dubey
@ 2016-12-16 13:11 ` Marek Szyprowski
2016-12-19 13:25 ` pankaj.dubey
8 siblings, 1 reply; 31+ messages in thread
From: Marek Szyprowski @ 2016-12-16 13:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Pankaj
On 2016-12-10 14:08, Pankaj Dubey wrote:
> Each Exynos SoC has ChipID block which can give information about SoC's
> product Id and revision number.
>
> This patch series introduces Exynos Chipid SoC driver. At the same time
> it reduces dependency of mach-exynos files from plat-samsung, by removing
> soc_is_exynosMMMM and samsung_rev API. Instead of it now we can use
> soc_device_match API proposed by Arnd and getting discussed in thread [1].
>
> Until now we are using static mapping of Exynos Chipid and using this static
> mapping to know about SoC name and revision via soc_is_exynosMMMM macro. This
> is quite cumbersome and every time new ARMv7 based Exynos SoC supports lands in
> mainline a bunch of such new macros needs to be added. Quite long back during
> support of Exynos5260 it has been discussed to solve this problem.
>
> To solve this issue this patchset replaces use of soc_is_exynosMMMM by either
> of_machine_is_compatible or soc_device_match depending upon usecase.
>
> I have tested this patch series on Exynos4210 based Origen board for normal SMP
> boot.
>
>
> Although I submitted this series as a whole of 8 patchsets, following are dependency
> details among the patches.
>
> Patch 1/8 can be taken without any dependency on other patches.
> Patch 2/8 and 3/8 has dependency on 1/8 and can be taken along with 1/8.
> Patch 4/8 has no dependency and can be taken without chipid driver patch 1/8.
> Patch 5/8 has depency on 1/8
> Patch 6/8 has no dependency and can be taken without any other patches.
> Patch 7/8 has dependency on 6/8 and 1/8
> Patch 8/8 has dependency on rest of patches
Which kernel should I use as a base for applying those patches? I wanted
to test them,
but I got conflicts both for v4.9 and current linux-next (next-20161216).
> [1]: https://patchwork.kernel.org/patch/9361389/
>
> Revision 7 and it's discussion can be found here:
> - https://www.spinics.net/lists/arm-kernel/msg540790.html
>
> Revision 6 and it's discussion can be found here:
> - https://lkml.org/lkml/2016/5/25/101
>
> Revision 5 and it's discussion can be found here:
> - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/310046.html
>
> Revision 4 and it's discussion can be found here:
> - https://lkml.org/lkml/2014/12/3/115
>
> Changes since v7:
> - Added ARM64 based Exynos7 and Exynos5433 support in chipid driver.
> - Removed Exynos4415 support from chipid driver, as exynos4415 has been removed from tree
> - Added patch to enable chipid driver for ARM64 based Exynos platform
> - Addressed review comments from Arnd for firmware.c, platsmp.c and pm.c files/
> - Splitted changes in firmware.c, platsmp.c and pm.c in separate patches
> for better code review
> - Included suggested code improvement in exynos-chipid.c from Marek Szyprowski
>
> Chances since v6:
> - Removed platform driver from chipid, instead use early_init to register soc_device
> - Removed export functions from exynos chipid driver
> - Replace soc_is_exynosMMMM via either of_machine_is_compatible or soc_device_match in
> files in arm/mach-exynos folder
> - This patchset depends on the following patch series by Geert Uytterhoeven. This series
> includes patch which introduces soc_device_match.
> http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1261739.html
> - Rebased on latest krzk/for-next branch and retested.
>
> Change since v5:
> - Addressed Rob's review comments.
> - Rebased on latest krzk/for-next branch and retested.
>
> Changes since v4:
> - Removed custom sysfs entries as they were not providing any new information
> as pointed out by Arnd.
> - Removed functions exporting product_id and revision, instead we will export
> exynos_chipid_info structure. It will be helpfull when we need to provide more
> fields of chipid outside of chipid, as commented by Yadwinder
> - Converted all funcions as __init.
>
> Change since v3:
> - This patch set contains 5/6 and 6/6 patch from v3 series.
> - Made EXYNOS_CHIPID config option non-user selectable,
> as suggested by Tomasz Figa.
> - Made uniform macro for EXYNOS4/5_SOC_MASK as EXYNOS_SOC_MASK as
> suggested by Tomasz Figa.
> - Made local variables static in chipid driver.
> - Added existing SoC's product id's.
> - Added platform driver support.
>
> Changes since v2:
> - Reorganized patches as suggested by Tomasz Figa.
> - Addressed review comments of Tomasz Figa in i2c-s3c2410.c file.
>
> Pankaj Dubey (8):
> soc: samsung: add exynos chipid driver support
> ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
> ARM64: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
> ARM: EXYNOS: refactor firmware specific routines
> ARM: EXYNOS: refactor power management specific routines
> ARM: EXYNOS: remove secondary startup initialization from
> smp_prepare_cpus
> ARM: EXYNOS: refactor smp specific code and routines
> ARM: EXYNOS: refactor of mach-exynos to use chipid information
>
> arch/arm/mach-exynos/Kconfig | 1 +
> arch/arm/mach-exynos/common.h | 92 ---------
> arch/arm/mach-exynos/exynos.c | 38 ----
> arch/arm/mach-exynos/firmware.c | 100 +++++++---
> arch/arm/mach-exynos/include/mach/map.h | 21 ---
> arch/arm/mach-exynos/platsmp.c | 267 +++++++++++++++++++++------
> arch/arm/mach-exynos/pm.c | 185 ++++++++++++++++---
> arch/arm/plat-samsung/cpu.c | 14 --
> arch/arm/plat-samsung/include/plat/cpu.h | 2 -
> arch/arm/plat-samsung/include/plat/map-s5p.h | 2 -
> arch/arm64/Kconfig.platforms | 1 +
> drivers/soc/samsung/Kconfig | 5 +
> drivers/soc/samsung/Makefile | 1 +
> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++
> 14 files changed, 563 insertions(+), 282 deletions(-)
> delete mode 100644 arch/arm/mach-exynos/include/mach/map.h
> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH v8 0/8] Introducing Exynos ChipId driver
2016-12-16 13:11 ` [PATCH v8 0/8] Introducing Exynos ChipId driver Marek Szyprowski
@ 2016-12-19 13:25 ` pankaj.dubey
0 siblings, 0 replies; 31+ messages in thread
From: pankaj.dubey @ 2016-12-19 13:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marek,
On Friday 16 December 2016 06:41 PM, Marek Szyprowski wrote:
> Hi Pankaj
>
>
> On 2016-12-10 14:08, Pankaj Dubey wrote:
>> Each Exynos SoC has ChipID block which can give information about SoC's
>> product Id and revision number.
>>
>> This patch series introduces Exynos Chipid SoC driver. At the same time
>> it reduces dependency of mach-exynos files from plat-samsung, by removing
>> soc_is_exynosMMMM and samsung_rev API. Instead of it now we can use
>> soc_device_match API proposed by Arnd and getting discussed in thread
>> [1].
>>
>> Until now we are using static mapping of Exynos Chipid and using this
>> static
>> mapping to know about SoC name and revision via soc_is_exynosMMMM
>> macro. This
>> is quite cumbersome and every time new ARMv7 based Exynos SoC supports
>> lands in
>> mainline a bunch of such new macros needs to be added. Quite long back
>> during
>> support of Exynos5260 it has been discussed to solve this problem.
>>
>> To solve this issue this patchset replaces use of soc_is_exynosMMMM by
>> either
>> of_machine_is_compatible or soc_device_match depending upon usecase.
>>
>> I have tested this patch series on Exynos4210 based Origen board for
>> normal SMP
>> boot.
>>
>>
>> Although I submitted this series as a whole of 8 patchsets, following
>> are dependency
>> details among the patches.
>>
>> Patch 1/8 can be taken without any dependency on other patches.
>> Patch 2/8 and 3/8 has dependency on 1/8 and can be taken along with 1/8.
>> Patch 4/8 has no dependency and can be taken without chipid driver
>> patch 1/8.
>> Patch 5/8 has depency on 1/8
>> Patch 6/8 has no dependency and can be taken without any other patches.
>> Patch 7/8 has dependency on 6/8 and 1/8
>> Patch 8/8 has dependency on rest of patches
>
> Which kernel should I use as a base for applying those patches? I wanted
> to test them,
> but I got conflicts both for v4.9 and current linux-next (next-20161216).
>
Sorry for late reply.
Actually these patches were created on top of Krzysztof's for-next
dated: 05/11/2016, I can think for because of following patches [1] and
[2] you might be getting conflict in latest tree. So if possible please
apply following patches on top of Krzysztof's current for-next and then try
[1]: https://patchwork.kernel.org/patch/9421017/
[2]: https://patchwork.kernel.org/patch/9419099/
Hopefully all chipid patches will get applied on top of it. Otherwise I
will be posting v9 very soon after addressing all review comments of v8.
So please test on v9.
Thanks,
Pankaj Dubey
^ permalink raw reply [flat|nested] 31+ messages in thread