diff mbox

[v8,1/8] soc: samsung: add exynos chipid driver support

Message ID 1481375323-29724-2-git-send-email-pankaj.dubey@samsung.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Pankaj Dubey Dec. 10, 2016, 1:08 p.m. UTC
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

Comments

Krzysztof Kozlowski Dec. 16, 2016, 5:37 p.m. UTC | #1
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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Dec. 17, 2016, 4:06 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Dec. 17, 2016, 12:03 p.m. UTC | #3
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

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Markus Reichl Dec. 19, 2016, 11:59 a.m. UTC | #4
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);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Dec. 19, 2016, 1:29 p.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski Dec. 19, 2016, 6:03 p.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Dec. 21, 2016, 7:52 a.m. UTC | #7
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
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrzej Hajda Dec. 21, 2016, 2:08 p.m. UTC | #8
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);


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz Dec. 27, 2016, 2:02 p.m. UTC | #9
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

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pankaj Dubey Dec. 28, 2016, 2:38 a.m. UTC | #10
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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);