diff mbox

[v9,04/12] soc: samsung: add exynos chipid driver support

Message ID 1490879826-16754-5-git-send-email-pankaj.dubey@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pankaj Dubey March 30, 2017, 1:16 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 | 109 ++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+)
 create mode 100644 drivers/soc/samsung/exynos-chipid.c

Comments

Arnd Bergmann March 30, 2017, 1:50 p.m. UTC | #1
On Thu, Mar 30, 2017 at 3:16 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
>
> +config EXYNOS_CHIPID
> +       bool "Exynos Chipid controller driver" if COMPILE_TEST
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       select SOC_BUS

This lets you disable the driver when COMPILE_TEST is set on exynos,
which is probably not what you wanted.

I'd do

         bool "Exynos Chipid controller driver" if COMPILE_TEST && !ARCHEXYNOS
         default ARCH_EXYNOS

     Arnd
Pankaj Dubey March 31, 2017, 5:36 a.m. UTC | #2
Hi Arnd,

On Thursday 30 March 2017 07:20 PM, Arnd Bergmann wrote:
> On Thu, Mar 30, 2017 at 3:16 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
>>
>> +config EXYNOS_CHIPID
>> +       bool "Exynos Chipid controller driver" if COMPILE_TEST
>> +       depends on ARCH_EXYNOS || COMPILE_TEST
>> +       select SOC_BUS
> 
> This lets you disable the driver when COMPILE_TEST is set on exynos,
> which is probably not what you wanted.
> 

Sorry I could not get this point. EXYNOS_CHIPID is invisible menu
option, when I enabled COMPILE_TEST, option is visible in make
menuconfig but it does not allow to disable this driver. I have adopted
this config same as EXYNOS_PMU in the same file.

> I'd do
> 
>          bool "Exynos Chipid controller driver" if COMPILE_TEST && !ARCHEXYNOS
>          default ARCH_EXYNOS

I can adopt, default ARCH_EXYNOS, which will allow me to drop patch 5/12
and 6/12.

Thanks for review.
Pankaj Dubey

> 
>      Arnd
> 
> 
>
Arnd Bergmann March 31, 2017, 8:09 a.m. UTC | #3
On Fri, Mar 31, 2017 at 7:36 AM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
> Hi Arnd,
>
> On Thursday 30 March 2017 07:20 PM, Arnd Bergmann wrote:
>> On Thu, Mar 30, 2017 at 3:16 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
>>>
>>> +config EXYNOS_CHIPID
>>> +       bool "Exynos Chipid controller driver" if COMPILE_TEST
>>> +       depends on ARCH_EXYNOS || COMPILE_TEST
>>> +       select SOC_BUS
>>
>> This lets you disable the driver when COMPILE_TEST is set on exynos,
>> which is probably not what you wanted.
>>
>
> Sorry I could not get this point. EXYNOS_CHIPID is invisible menu
> option, when I enabled COMPILE_TEST, option is visible in make
> menuconfig but it does not allow to disable this driver. I have adopted
> this config same as EXYNOS_PMU in the same file.

I wrote my comment before I saw the 'select' statement in the later
patch. With that select, it is not a problem.

>>          bool "Exynos Chipid controller driver" if COMPILE_TEST && !ARCHEXYNOS
>>          default ARCH_EXYNOS
>
> I can adopt, default ARCH_EXYNOS, which will allow me to drop patch 5/12
> and 6/12.

Either way (select or default) is fine, just do the same thing for both chipid
and pmu. If you keep the 'select' and the 'if COMPILE_TEST', you can
drop the line 'depends on ARCH_EXYNOS || COMPILE_TEST'.

      Arnd
Marek Szyprowski April 3, 2017, 7:57 a.m. UTC | #4
Hi Pankaj

On 2017-03-30 15:16, 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 | 109 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 115 insertions(+)
>   create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 8b25bd5..a90a4ad 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -6,6 +6,11 @@ menuconfig SOC_SAMSUNG
>   
>   if SOC_SAMSUNG
>   
> +config EXYNOS_CHIPID
> +	bool "Exynos Chipid controller driver" if COMPILE_TEST
> +	depends on ARCH_EXYNOS || COMPILE_TEST
> +	select SOC_BUS
> +
>   config EXYNOS_PMU
>   	bool "Exynos PMU controller driver" if COMPILE_TEST
>   	depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST)
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 4d7694a..be3b6bb 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
>   obj-$(CONFIG_EXYNOS_PMU)	+= exynos-pmu.o
>   
>   obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)	+= exynos3250-pmu.o exynos4-pmu.o \
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..1e9fb6b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright (c) 2017 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", 0x43210000, 0xFFFFF000 },

You have once again changed the mask for Exynos4 SoCs, so please add
following line to the above array:

{ "EXYNOS4210", 0x43200000, 0xFFFFF000 },       /* EVT0 revision */

Otherwise Exynos C210 (4210 EVT0) is not properly detected:

soc soc0: Exynos: CPU[UNKNOWN] PRO_ID[0x43200200] REV[0x0] Detected

> +	{ "EXYNOS4212", 0x43220000, 0xFFFFF000 },
> +	{ "EXYNOS4412", 0xE4412000, 0xFFFFF000 },
> +	{ "EXYNOS5250", 0x43520000, 0xFFFFF000 },
> +	{ "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
> +	{ "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
> +	{ "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
> +	{ "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
> +	{ "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
> +	{ "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
> +	{ "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
> +};

Now the mask is same for all revisions, so you can remove it from the above
array and directly use some kind of define in the code.

> +
> +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";
> +}
> +
> +int __init exynos_chipid_early_init(void)
> +{
> +	struct soc_device_attribute *soc_dev_attr;
> +	void __iomem *exynos_chipid_base;
> +	struct soc_device *soc_dev;
> +	struct device_node *root;
> +	struct device_node *np;
> +	struct device *dev;
> +	u32 product_id;
> +	u32 revision;
> +
> +	/* look up for chipid node */
> +	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
> +	if (!np)
> +		return -ENODEV;
> +
> +	exynos_chipid_base = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!exynos_chipid_base) {
> +		pr_err("%s: failed to map chipid\n", np->name);
> +		return -ENOMEM;
> +	}
> +
> +	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);
> +
> +	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);
> +	}
> +	dev = soc_device_to_device(soc_dev);
> +
> +	dev_info(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
> +			soc_dev_attr->soc_id, product_id, revision);
> +
> +	return 0;
> +}
> +early_initcall(exynos_chipid_early_init);

Best regards
Pankaj Dubey April 3, 2017, 9:21 a.m. UTC | #5
On Friday 31 March 2017 01:39 PM, Arnd Bergmann wrote:
> On Fri, Mar 31, 2017 at 7:36 AM, pankaj.dubey <pankaj.dubey@samsung.com> wrote:
>> Hi Arnd,
>>
>> On Thursday 30 March 2017 07:20 PM, Arnd Bergmann wrote:
>>> On Thu, Mar 30, 2017 at 3:16 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
>>>>
>>>> +config EXYNOS_CHIPID
>>>> +       bool "Exynos Chipid controller driver" if COMPILE_TEST
>>>> +       depends on ARCH_EXYNOS || COMPILE_TEST
>>>> +       select SOC_BUS
>>>
>>> This lets you disable the driver when COMPILE_TEST is set on exynos,
>>> which is probably not what you wanted.
>>>
>>
>> Sorry I could not get this point. EXYNOS_CHIPID is invisible menu
>> option, when I enabled COMPILE_TEST, option is visible in make
>> menuconfig but it does not allow to disable this driver. I have adopted
>> this config same as EXYNOS_PMU in the same file.
> 
> I wrote my comment before I saw the 'select' statement in the later
> patch. With that select, it is not a problem.
> 
>>>          bool "Exynos Chipid controller driver" if COMPILE_TEST && !ARCHEXYNOS
>>>          default ARCH_EXYNOS
>>
>> I can adopt, default ARCH_EXYNOS, which will allow me to drop patch 5/12
>> and 6/12.
> 
> Either way (select or default) is fine, just do the same thing for both chipid
> and pmu. If you keep the 'select' and the 'if COMPILE_TEST', you can
> drop the line 'depends on ARCH_EXYNOS || COMPILE_TEST'.
> 

OK, I understood your point. Thanks.

Pankaj Dubey

>       Arnd
> 
> 
>
Pankaj Dubey April 3, 2017, 9:35 a.m. UTC | #6
Hi Marek,

On Monday 03 April 2017 01:27 PM, Marek Szyprowski wrote:
> Hi Pankaj
> 
> On 2017-03-30 15:16, 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.

snip

>> +
>> +static const struct exynos_soc_id {
>> +    const char *name;
>> +    unsigned int id;
>> +    unsigned int mask;
>> +} soc_ids[] = {
>> +    { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
>> +    { "EXYNOS4210", 0x43210000, 0xFFFFF000 },
> 
> You have once again changed the mask for Exynos4 SoCs, so please add
> following line to the above array:

Yes, this time I cross-verified from the UM of respective SoC's and
added it.

> 
> { "EXYNOS4210", 0x43200000, 0xFFFFF000 },       /* EVT0 revision */
> 
> Otherwise Exynos C210 (4210 EVT0) is not properly detected:
> 
> soc soc0: Exynos: CPU[UNKNOWN] PRO_ID[0x43200200] REV[0x0] Detected

Thanks for testing, I will add support for this.

> 
>> +    { "EXYNOS4212", 0x43220000, 0xFFFFF000 },
>> +    { "EXYNOS4412", 0xE4412000, 0xFFFFF000 },
>> +    { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
>> +    { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
>> +    { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
>> +    { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
>> +    { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
>> +    { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
>> +    { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
>> +    { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
>> +};
> 
> Now the mask is same for all revisions, so you can remove it from the above
> array and directly use some kind of define in the code.

Yes, I also observed, but somehow I missed to update this part. I will
change this in next version.

> 
>> +
>> +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";
>> +}
>> +
>> +int __init exynos_chipid_early_init(void)
>> +{
>> +    struct soc_device_attribute *soc_dev_attr;
>> +    void __iomem *exynos_chipid_base;
>> +    struct soc_device *soc_dev;
>> +    struct device_node *root;
>> +    struct device_node *np;
>> +    struct device *dev;
>> +    u32 product_id;
>> +    u32 revision;
>> +
>> +    /* look up for chipid node */
>> +    np = of_find_compatible_node(NULL, NULL,
>> "samsung,exynos4210-chipid");
>> +    if (!np)
>> +        return -ENODEV;
>> +
>> +    exynos_chipid_base = of_iomap(np, 0);
>> +    of_node_put(np);
>> +
>> +    if (!exynos_chipid_base) {
>> +        pr_err("%s: failed to map chipid\n", np->name);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    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);
>> +
>> +    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);
>> +    }
>> +    dev = soc_device_to_device(soc_dev);
>> +
>> +    dev_info(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
>> +            soc_dev_attr->soc_id, product_id, revision);
>> +
>> +    return 0;
>> +}
>> +early_initcall(exynos_chipid_early_init);
> 
> Best regards


Thanks,
Pankaj Dubey
Krzysztof Kozlowski April 7, 2017, 9:13 a.m. UTC | #7
On Thu, Mar 30, 2017 at 3:16 PM, Pankaj Dubey <pankaj.dubey@samsung.com> 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 | 109 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 115 insertions(+)
>  create mode 100644 drivers/soc/samsung/exynos-chipid.c
>
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 8b25bd5..a90a4ad 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -6,6 +6,11 @@ menuconfig SOC_SAMSUNG
>
>  if SOC_SAMSUNG
>
> +config EXYNOS_CHIPID
> +       bool "Exynos Chipid controller driver" if COMPILE_TEST
> +       depends on ARCH_EXYNOS || COMPILE_TEST
> +       select SOC_BUS
> +
>  config EXYNOS_PMU
>         bool "Exynos PMU controller driver" if COMPILE_TEST
>         depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST)
> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> index 4d7694a..be3b6bb 100644
> --- a/drivers/soc/samsung/Makefile
> +++ b/drivers/soc/samsung/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_EXYNOS_CHIPID)    += exynos-chipid.o
>  obj-$(CONFIG_EXYNOS_PMU)       += exynos-pmu.o
>
>  obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)   += exynos3250-pmu.o exynos4-pmu.o \
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 0000000..1e9fb6b
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright (c) 2017 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", 0x43210000, 0xFFFFF000 },
> +       { "EXYNOS4212", 0x43220000, 0xFFFFF000 },
> +       { "EXYNOS4412", 0xE4412000, 0xFFFFF000 },
> +       { "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";
> +}
> +
> +int __init exynos_chipid_early_init(void)
> +{
> +       struct soc_device_attribute *soc_dev_attr;
> +       void __iomem *exynos_chipid_base;
> +       struct soc_device *soc_dev;
> +       struct device_node *root;
> +       struct device_node *np;
> +       struct device *dev;
> +       u32 product_id;
> +       u32 revision;
> +
> +       /* look up for chipid node */
> +       np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
> +       if (!np)
> +               return -ENODEV;
> +
> +       exynos_chipid_base = of_iomap(np, 0);
> +       of_node_put(np);
> +
> +       if (!exynos_chipid_base) {
> +               pr_err("%s: failed to map chipid\n", np->name);
> +               return -ENOMEM;
> +       }
> +
> +       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;

This should be -ENOMEM

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

This was not allocated with kstrdup_const() so it should not be freed.

Best regards,
Krzysztof
Pankaj Dubey April 7, 2017, 12:18 p.m. UTC | #8
On Friday 07 April 2017 02:43 PM, Krzysztof Kozlowski wrote:
> On Thu, Mar 30, 2017 at 3:16 PM, Pankaj Dubey <pankaj.dubey@samsung.com> 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 | 109 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 115 insertions(+)
>>  create mode 100644 drivers/soc/samsung/exynos-chipid.c
>>
>> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
>> index 8b25bd5..a90a4ad 100644
>> --- a/drivers/soc/samsung/Kconfig
>> +++ b/drivers/soc/samsung/Kconfig
>> @@ -6,6 +6,11 @@ menuconfig SOC_SAMSUNG
>>
>>  if SOC_SAMSUNG
>>
>> +config EXYNOS_CHIPID
>> +       bool "Exynos Chipid controller driver" if COMPILE_TEST
>> +       depends on ARCH_EXYNOS || COMPILE_TEST
>> +       select SOC_BUS
>> +
>>  config EXYNOS_PMU
>>         bool "Exynos PMU controller driver" if COMPILE_TEST
>>         depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST)
>> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
>> index 4d7694a..be3b6bb 100644
>> --- a/drivers/soc/samsung/Makefile
>> +++ b/drivers/soc/samsung/Makefile
>> @@ -1,3 +1,4 @@
>> +obj-$(CONFIG_EXYNOS_CHIPID)    += exynos-chipid.o
>>  obj-$(CONFIG_EXYNOS_PMU)       += exynos-pmu.o
>>
>>  obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)   += exynos3250-pmu.o exynos4-pmu.o \
>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>> new file mode 100644
>> index 0000000..1e9fb6b
>> --- /dev/null
>> +++ b/drivers/soc/samsung/exynos-chipid.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Copyright (c) 2017 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", 0x43210000, 0xFFFFF000 },
>> +       { "EXYNOS4212", 0x43220000, 0xFFFFF000 },
>> +       { "EXYNOS4412", 0xE4412000, 0xFFFFF000 },
>> +       { "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";
>> +}
>> +
>> +int __init exynos_chipid_early_init(void)
>> +{
>> +       struct soc_device_attribute *soc_dev_attr;
>> +       void __iomem *exynos_chipid_base;
>> +       struct soc_device *soc_dev;
>> +       struct device_node *root;
>> +       struct device_node *np;
>> +       struct device *dev;
>> +       u32 product_id;
>> +       u32 revision;
>> +
>> +       /* look up for chipid node */
>> +       np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
>> +       if (!np)
>> +               return -ENODEV;
>> +
>> +       exynos_chipid_base = of_iomap(np, 0);
>> +       of_node_put(np);
>> +
>> +       if (!exynos_chipid_base) {
>> +               pr_err("%s: failed to map chipid\n", np->name);
>> +               return -ENOMEM;
>> +       }
>> +
>> +       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;
> 
> This should be -ENOMEM
> 
OK. Will take care in next patchset.

>> +
>> +       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);
>> +
>> +       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);
> 
> This was not allocated with kstrdup_const() so it should not be freed.
> 

Sorry, I missed to take care of this, you pointed out this in V8 as well.

Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
> 
> 
>
diff mbox

Patch

diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
index 8b25bd5..a90a4ad 100644
--- a/drivers/soc/samsung/Kconfig
+++ b/drivers/soc/samsung/Kconfig
@@ -6,6 +6,11 @@  menuconfig SOC_SAMSUNG
 
 if SOC_SAMSUNG
 
+config EXYNOS_CHIPID
+	bool "Exynos Chipid controller driver" if COMPILE_TEST
+	depends on ARCH_EXYNOS || COMPILE_TEST
+	select SOC_BUS
+
 config EXYNOS_PMU
 	bool "Exynos PMU controller driver" if COMPILE_TEST
 	depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST)
diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
index 4d7694a..be3b6bb 100644
--- a/drivers/soc/samsung/Makefile
+++ b/drivers/soc/samsung/Makefile
@@ -1,3 +1,4 @@ 
+obj-$(CONFIG_EXYNOS_CHIPID)	+= exynos-chipid.o
 obj-$(CONFIG_EXYNOS_PMU)	+= exynos-pmu.o
 
 obj-$(CONFIG_EXYNOS_PMU_ARM_DRIVERS)	+= exynos3250-pmu.o exynos4-pmu.o \
diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
new file mode 100644
index 0000000..1e9fb6b
--- /dev/null
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -0,0 +1,109 @@ 
+/*
+ * Copyright (c) 2017 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", 0x43210000, 0xFFFFF000 },
+	{ "EXYNOS4212", 0x43220000, 0xFFFFF000 },
+	{ "EXYNOS4412", 0xE4412000, 0xFFFFF000 },
+	{ "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";
+}
+
+int __init exynos_chipid_early_init(void)
+{
+	struct soc_device_attribute *soc_dev_attr;
+	void __iomem *exynos_chipid_base;
+	struct soc_device *soc_dev;
+	struct device_node *root;
+	struct device_node *np;
+	struct device *dev;
+	u32 product_id;
+	u32 revision;
+
+	/* look up for chipid node */
+	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
+	if (!np)
+		return -ENODEV;
+
+	exynos_chipid_base = of_iomap(np, 0);
+	of_node_put(np);
+
+	if (!exynos_chipid_base) {
+		pr_err("%s: failed to map chipid\n", np->name);
+		return -ENOMEM;
+	}
+
+	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);
+
+	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);
+	}
+	dev = soc_device_to_device(soc_dev);
+
+	dev_info(dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
+			soc_dev_attr->soc_id, product_id, revision);
+
+	return 0;
+}
+early_initcall(exynos_chipid_early_init);