Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1440042 > unrolled thread

[PATCH v2 06/10] soc: Add SoC specific driver support for nuc900

Started byWan Zongshun <vw@iommu.org>
First post2016-07-10 09:30 +0200
Last post2016-07-12 12:00 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Wan Zongshun <vw@iommu.org> - 2016-07-10 09:30 +0200
    Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Arnd Bergmann <arnd@arndb.de> - 2016-07-11 10:10 +0200
      Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Wan Zongshun <vw@iommu.org> - 2016-07-11 11:10 +0200
        Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Arnd Bergmann <arnd@arndb.de> - 2016-07-11 12:30 +0200
          Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Wan ZongShun <mcuos.com@gmail.com> - 2016-07-11 12:30 +0200
            Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Arnd Bergmann <arnd@arndb.de> - 2016-07-11 12:40 +0200
      Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Wan Zongshun <vw@iommu.org> - 2016-07-12 11:10 +0200
        Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 Arnd Bergmann <arnd@arndb.de> - 2016-07-12 12:00 +0200

#1440042 — [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900

FromWan Zongshun <vw@iommu.org>
Date2016-07-10 09:30 +0200
Subject[PATCH v2 06/10] soc: Add SoC specific driver support for nuc900
Message-ID<rThlD-5X6-9@gated-at.bofh.it>
This patch is to add SoC specific driver for nuc970 SoC,
it is for getting nuc970 version id and chip id.

Signed-off-by: Wan Zongshun <mcuos.com@gmail.com>
---
 drivers/soc/Kconfig              |   1 +
 drivers/soc/Makefile             |   1 +
 drivers/soc/nuvoton/Kconfig      |  10 ++++
 drivers/soc/nuvoton/Makefile     |   1 +
 drivers/soc/nuvoton/soc-nuc900.c | 100 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 113 insertions(+)
 create mode 100644 drivers/soc/nuvoton/Kconfig
 create mode 100644 drivers/soc/nuvoton/Makefile
 create mode 100644 drivers/soc/nuvoton/soc-nuc900.c

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index cb58ef0..2119733 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -4,6 +4,7 @@ source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/brcmstb/Kconfig"
 source "drivers/soc/fsl/qe/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
+source "drivers/soc/nuvoton/Kconfig"
 source "drivers/soc/qcom/Kconfig"
 source "drivers/soc/rockchip/Kconfig"
 source "drivers/soc/samsung/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 380230f..bb1bfba 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
 obj-$(CONFIG_ARCH_MEDIATEK)	+= mediatek/
+obj-$(CONFIG_SOC_NUC900)	+= nuvoton/
 obj-$(CONFIG_ARCH_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_RENESAS)	+= renesas/
 obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip/
diff --git a/drivers/soc/nuvoton/Kconfig b/drivers/soc/nuvoton/Kconfig
new file mode 100644
index 0000000..53c106c
--- /dev/null
+++ b/drivers/soc/nuvoton/Kconfig
@@ -0,0 +1,10 @@
+#
+# ARM Versatile SoC drivers
+#
+config SOC_NUC900
+	bool "SoC bus device for the nuvoton NUC900 platforms"
+	depends on ARCH_W90X900
+	select SOC_BUS
+	help
+	  Include support for the SoC bus on the NUC900 platforms
+	  providing some sysfs information about the ASIC variant.
diff --git a/drivers/soc/nuvoton/Makefile b/drivers/soc/nuvoton/Makefile
new file mode 100644
index 0000000..88f9b7e
--- /dev/null
+++ b/drivers/soc/nuvoton/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SOC_NUC900)	+= soc-nuc900.o
diff --git a/drivers/soc/nuvoton/soc-nuc900.c b/drivers/soc/nuvoton/soc-nuc900.c
new file mode 100644
index 0000000..034ef94
--- /dev/null
+++ b/drivers/soc/nuvoton/soc-nuc900.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright 2016 Wan Zongshun <mcuos.com@gmail.com>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/of.h>
+
+/* System ID in syscon */
+#define GCR_CHIPID		0x00
+#define GCR_CHIPID_MASK		0x00ffffff
+
+static const struct of_device_id nuc900_soc_of_match[] = {
+	{ .compatible = "nuvoton,nuc900-soc",	},
+	{ }
+};
+
+static u32 nuc900_chipid;
+
+static ssize_t nuc900_get_chipid(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	return sprintf(buf, "0x%x\n", nuc900_chipid & GCR_CHIPID_MASK);
+}
+
+static struct device_attribute nuc900_chipid_attr =
+	__ATTR(manufacturer,  S_IRUGO, nuc900_get_chipid,  NULL);
+
+static ssize_t nuc900_get_versionid(struct device *dev,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	return sprintf(buf, "0x%x\n", (nuc900_chipid >> 24) & 0xff);
+}
+
+static struct device_attribute nuc900_version_attr =
+	__ATTR(board,  S_IRUGO, nuc900_get_versionid,  NULL);
+
+static int nuc900_soc_probe(struct platform_device *pdev)
+{
+	static struct regmap *syscon_regmap;
+	struct soc_device *soc_dev;
+	struct soc_device_attribute *soc_dev_attr;
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	syscon_regmap = syscon_regmap_lookup_by_phandle(np, "syscon");
+	if (IS_ERR(syscon_regmap))
+		return PTR_ERR(syscon_regmap);
+
+	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+	if (!soc_dev_attr)
+		return -ENOMEM;
+
+	ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
+	if (ret)
+		return -EINVAL;
+
+	soc_dev_attr->machine = "NUC900EVB";
+	soc_dev_attr->family = "NUC900";
+	soc_dev = soc_device_register(soc_dev_attr);
+	if (IS_ERR(soc_dev)) {
+		kfree(soc_dev_attr);
+		return -ENODEV;
+	}
+
+	ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
+	if (ret)
+		return -ENODEV;
+
+	device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
+	device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
+
+	dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
+		 nuc900_chipid & GCR_CHIPID_MASK,
+		 (nuc900_chipid >> 24) & 0xff);
+
+	return 0;
+}
+
+static struct platform_driver nuc900_soc_driver = {
+	.probe = nuc900_soc_probe,
+	.driver = {
+		.name = "nuc900-soc",
+		.of_match_table = nuc900_soc_of_match,
+	},
+};
+builtin_platform_driver(nuc900_soc_driver);
-- 
2.7.4

[toc] | [next] | [standalone]

#1440339

FromArnd Bergmann <arnd@arndb.de>
Date2016-07-11 10:10 +0200
Message-ID<rTErU-4nS-15@gated-at.bofh.it>
In reply to#1440042
On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
> +       ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
> +       if (ret)
>               return -EINVAL;
> +
> +       soc_dev_attr->machine = "NUC900EVB";
> +       soc_dev_attr->family = "NUC900";
> +       soc_dev = soc_device_register(soc_dev_attr);
> +       if (IS_ERR(soc_dev)) {
> +               kfree(soc_dev_attr);
> +               return -ENODEV;
> +       }
> +
> +       ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
> +       if (ret)
> +               return -ENODEV;
> +
> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
> +
> +       dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
> +                nuc900_chipid & GCR_CHIPID_MASK,
> +                (nuc900_chipid >> 24) & 0xff);

I'm still a bit unsure about the set of attributes here.

- The "soc_id" is read from the device tree from the field that contains
  the board name, I think for consistency you should try to map the
  GCR_CHIPID to the name of the SoC and assign that here

- The "machine" is hardcoded to "NUC900EVB", which in turn looks like
  a particular board but not the one you are running on. Maybe read
  that from the DT instead?

- The "revision" is not filled at all, I would suggest using something
  derived from the GCR_CHIPID register here

- you have two nonstandard attributes "chipid" and "version", which
  I'd hope to avoid -- the set of standard attributes is supposed to
  give enough information about the machine, and platform independent
  user space will never read those.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1440383

FromWan Zongshun <vw@iommu.org>
Date2016-07-11 11:10 +0200
Message-ID<rTFnY-4Z1-11@gated-at.bofh.it>
In reply to#1440339

On 2016年07月11日 16:03, Arnd Bergmann wrote:
> On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
>> +       ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
>> +       if (ret)
>>                return -EINVAL;
>> +
>> +       soc_dev_attr->machine = "NUC900EVB";
>> +       soc_dev_attr->family = "NUC900";
>> +       soc_dev = soc_device_register(soc_dev_attr);
>> +       if (IS_ERR(soc_dev)) {
>> +               kfree(soc_dev_attr);
>> +               return -ENODEV;
>> +       }
>> +
>> +       ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
>> +       if (ret)
>> +               return -ENODEV;
>> +
>> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
>> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
>> +
>> +       dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
>> +                nuc900_chipid & GCR_CHIPID_MASK,
>> +                (nuc900_chipid >> 24) & 0xff);
>
> I'm still a bit unsure about the set of attributes here.
>
> - The "soc_id" is read from the device tree from the field that contains
>    the board name, I think for consistency you should try to map the
>    GCR_CHIPID to the name of the SoC and assign that here

I will try to get chipid and map it to soc name like: “nuc970”, "nuc910".

And I will set this soc name to soc_id, ok?

>
> - The "machine" is hardcoded to "NUC900EVB", which in turn looks like
>    a particular board but not the one you are running on. Maybe read
>    that from the DT instead?

Should I read nuc970-evb.dts's "model" or "compatible" properties?

>
> - The "revision" is not filled at all, I would suggest using something
>    derived from the GCR_CHIPID register here
>
> - you have two nonstandard attributes "chipid" and "version", which
>    I'd hope to avoid -- the set of standard attributes is supposed to
>    give enough information about the machine, and platform independent
>    user space will never read those.
>
> 	Arnd
>
>

[toc] | [prev] | [next] | [standalone]

#1440444

FromArnd Bergmann <arnd@arndb.de>
Date2016-07-11 12:30 +0200
Message-ID<rTGDo-5Kn-7@gated-at.bofh.it>
In reply to#1440383
On Monday, July 11, 2016 5:07:01 PM CEST Wan Zongshun wrote:
> 
> On 2016年07月11日 16:03, Arnd Bergmann wrote:
> > On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
> >> +       ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
> >> +       if (ret)
> >>                return -EINVAL;
> >> +
> >> +       soc_dev_attr->machine = "NUC900EVB";
> >> +       soc_dev_attr->family = "NUC900";
> >> +       soc_dev = soc_device_register(soc_dev_attr);
> >> +       if (IS_ERR(soc_dev)) {
> >> +               kfree(soc_dev_attr);
> >> +               return -ENODEV;
> >> +       }
> >> +
> >> +       ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
> >> +       if (ret)
> >> +               return -ENODEV;
> >> +
> >> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
> >> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
> >> +
> >> +       dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
> >> +                nuc900_chipid & GCR_CHIPID_MASK,
> >> +                (nuc900_chipid >> 24) & 0xff);
> >
> > I'm still a bit unsure about the set of attributes here.
> >
> > - The "soc_id" is read from the device tree from the field that contains
> >    the board name, I think for consistency you should try to map the
> >    GCR_CHIPID to the name of the SoC and assign that here
> 
> I will try to get chipid and map it to soc name like: “nuc970”, "nuc910".
> 
> And I will set this soc name to soc_id, ok?

Ok.

> > - The "machine" is hardcoded to "NUC900EVB", which in turn looks like
> >    a particular board but not the one you are running on. Maybe read
> >    that from the DT instead?
> 
> Should I read nuc970-evb.dts's "model" or "compatible" properties?

I think "model" is best here, but see what the others do.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1440447

FromWan ZongShun <mcuos.com@gmail.com>
Date2016-07-11 12:30 +0200
Message-ID<rTGDo-5Kn-29@gated-at.bofh.it>
In reply to#1440444
2016-07-11 18:24 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> On Monday, July 11, 2016 5:07:01 PM CEST Wan Zongshun wrote:
>>
>> On 2016年07月11日 16:03, Arnd Bergmann wrote:
>> > On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
>> >> +       ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
>> >> +       if (ret)
>> >>                return -EINVAL;
>> >> +
>> >> +       soc_dev_attr->machine = "NUC900EVB";
>> >> +       soc_dev_attr->family = "NUC900";
>> >> +       soc_dev = soc_device_register(soc_dev_attr);
>> >> +       if (IS_ERR(soc_dev)) {
>> >> +               kfree(soc_dev_attr);
>> >> +               return -ENODEV;
>> >> +       }
>> >> +
>> >> +       ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
>> >> +       if (ret)
>> >> +               return -ENODEV;
>> >> +
>> >> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
>> >> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
>> >> +
>> >> +       dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
>> >> +                nuc900_chipid & GCR_CHIPID_MASK,
>> >> +                (nuc900_chipid >> 24) & 0xff);
>> >
>> > I'm still a bit unsure about the set of attributes here.
>> >
>> > - The "soc_id" is read from the device tree from the field that contains
>> >    the board name, I think for consistency you should try to map the
>> >    GCR_CHIPID to the name of the SoC and assign that here
>>
>> I will try to get chipid and map it to soc name like: “nuc970”, "nuc910".
>>
>> And I will set this soc name to soc_id, ok?
>
> Ok.

Maybe I also can set versionid as soc name partly, like
nuc970-version1,nuc970-version2? and then set the to soc_id, make
sense?


>
>> > - The "machine" is hardcoded to "NUC900EVB", which in turn looks like
>> >    a particular board but not the one you are running on. Maybe read
>> >    that from the DT instead?
>>
>> Should I read nuc970-evb.dts's "model" or "compatible" properties?
>
> I think "model" is best here, but see what the others do.
>
>         Arnd
>



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com

[toc] | [prev] | [next] | [standalone]

#1440461

FromArnd Bergmann <arnd@arndb.de>
Date2016-07-11 12:40 +0200
Message-ID<rTGN5-5On-43@gated-at.bofh.it>
In reply to#1440447
On Monday, July 11, 2016 6:28:57 PM CEST Wan ZongShun wrote:
> 2016-07-11 18:24 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
> > On Monday, July 11, 2016 5:07:01 PM CEST Wan Zongshun wrote:
> >>
> >> On 2016年07月11日 16:03, Arnd Bergmann wrote:
> >> > On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
> >> >> +       ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
> >> >> +       if (ret)
> >> >>                return -EINVAL;
> >> >> +
> >> >> +       soc_dev_attr->machine = "NUC900EVB";
> >> >> +       soc_dev_attr->family = "NUC900";
> >> >> +       soc_dev = soc_device_register(soc_dev_attr);
> >> >> +       if (IS_ERR(soc_dev)) {
> >> >> +               kfree(soc_dev_attr);
> >> >> +               return -ENODEV;
> >> >> +       }
> >> >> +
> >> >> +       ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
> >> >> +       if (ret)
> >> >> +               return -ENODEV;
> >> >> +
> >> >> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
> >> >> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
> >> >> +
> >> >> +       dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
> >> >> +                nuc900_chipid & GCR_CHIPID_MASK,
> >> >> +                (nuc900_chipid >> 24) & 0xff);
> >> >
> >> > I'm still a bit unsure about the set of attributes here.
> >> >
> >> > - The "soc_id" is read from the device tree from the field that contains
> >> >    the board name, I think for consistency you should try to map the
> >> >    GCR_CHIPID to the name of the SoC and assign that here
> >>
> >> I will try to get chipid and map it to soc name like: “nuc970”, "nuc910".
> >>
> >> And I will set this soc name to soc_id, ok?
> >
> > Ok.
> 
> Maybe I also can set versionid as soc name partly, like
> nuc970-version1,nuc970-version2? and then set the to soc_id, make
> sense?
> 

I didn't exactly understand what the suggestion is, maybe send that
as code so I see what you mean.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1441158

FromWan Zongshun <vw@iommu.org>
Date2016-07-12 11:10 +0200
Message-ID<rU1Rx-2SM-29@gated-at.bofh.it>
In reply to#1440339

On 2016年07月11日 16:03, Arnd Bergmann wrote:
> On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
>> +       ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id);
>> +       if (ret)
>>                return -EINVAL;
>> +
>> +       soc_dev_attr->machine = "NUC900EVB";
>> +       soc_dev_attr->family = "NUC900";
>> +       soc_dev = soc_device_register(soc_dev_attr);
>> +       if (IS_ERR(soc_dev)) {
>> +               kfree(soc_dev_attr);
>> +               return -ENODEV;
>> +       }
>> +
>> +       ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid);
>> +       if (ret)
>> +               return -ENODEV;
>> +
>> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
>> +       device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
>> +
>> +       dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n",
>> +                nuc900_chipid & GCR_CHIPID_MASK,
>> +                (nuc900_chipid >> 24) & 0xff);
>
> I'm still a bit unsure about the set of attributes here.
>
> - The "soc_id" is read from the device tree from the field that contains
>    the board name, I think for consistency you should try to map the
>    GCR_CHIPID to the name of the SoC and assign that here
>
> - The "machine" is hardcoded to "NUC900EVB", which in turn looks like
>    a particular board but not the one you are running on. Maybe read
>    that from the DT instead?
>
> - The "revision" is not filled at all, I would suggest using something
>    derived from the GCR_CHIPID register here
>
> - you have two nonstandard attributes "chipid" and "version", which
>    I'd hope to avoid -- the set of standard attributes is supposed to
>    give enough information about the machine, and platform independent
>    user space will never read those.

So, Maybe I can remove those two codes, no need push those information 
to user space?

device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);

>
> 	Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>

[toc] | [prev] | [next] | [standalone]

#1441190

FromArnd Bergmann <arnd@arndb.de>
Date2016-07-12 12:00 +0200
Message-ID<rU2DT-3b3-13@gated-at.bofh.it>
In reply to#1441158
On Tuesday, July 12, 2016 5:06:10 PM CEST Wan Zongshun wrote:
> On 2016年07月11日 16:03, Arnd Bergmann wrote:
> > On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
> > I'm still a bit unsure about the set of attributes here.
> >
> > - The "soc_id" is read from the device tree from the field that contains
> >    the board name, I think for consistency you should try to map the
> >    GCR_CHIPID to the name of the SoC and assign that here
> >
> > - The "machine" is hardcoded to "NUC900EVB", which in turn looks like
> >    a particular board but not the one you are running on. Maybe read
> >    that from the DT instead?
> >
> > - The "revision" is not filled at all, I would suggest using something
> >    derived from the GCR_CHIPID register here
> >
> > - you have two nonstandard attributes "chipid" and "version", which
> >    I'd hope to avoid -- the set of standard attributes is supposed to
> >    give enough information about the machine, and platform independent
> >    user space will never read those.
> 
> So, Maybe I can remove those two codes, no need push those information 
> to user space?
> 
> device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr);
> device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr);
> 

Yes, that would be good.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web