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

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

2016-07-12 Thread Arnd Bergmann

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), _chipid_attr);
> device_create_file(soc_device_to_device(soc_dev), _version_attr);
> 

Yes, that would be good.

Arnd

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

2016-07-12 Thread Arnd Bergmann

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), _chipid_attr);
> device_create_file(soc_device_to_device(soc_dev), _version_attr);
> 

Yes, that would be good.

Arnd

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

2016-07-12 Thread Wan Zongshun




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", _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, _chipid);
+   if (ret)
+   return -ENODEV;
+
+   device_create_file(soc_device_to_device(soc_dev), _chipid_attr);
+   device_create_file(soc_device_to_device(soc_dev), _version_attr);
+
+   dev_info(>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), _chipid_attr);
device_create_file(soc_device_to_device(soc_dev), _version_attr);



Arnd

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

2016-07-12 Thread Wan Zongshun




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", _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, _chipid);
+   if (ret)
+   return -ENODEV;
+
+   device_create_file(soc_device_to_device(soc_dev), _chipid_attr);
+   device_create_file(soc_device_to_device(soc_dev), _version_attr);
+
+   dev_info(>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), _chipid_attr);
device_create_file(soc_device_to_device(soc_dev), _version_attr);



Arnd

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

2016-07-11 Thread Arnd Bergmann

On Monday, July 11, 2016 6:28:57 PM CEST Wan ZongShun wrote:
> 2016-07-11 18:24 GMT+08:00 Arnd Bergmann :
> > 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", 
> >> >> _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, _chipid);
> >> >> +   if (ret)
> >> >> +   return -ENODEV;
> >> >> +
> >> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> >> _chipid_attr);
> >> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> >> _version_attr);
> >> >> +
> >> >> +   dev_info(>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

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

2016-07-11 Thread Arnd Bergmann

On Monday, July 11, 2016 6:28:57 PM CEST Wan ZongShun wrote:
> 2016-07-11 18:24 GMT+08:00 Arnd Bergmann :
> > 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", 
> >> >> _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, _chipid);
> >> >> +   if (ret)
> >> >> +   return -ENODEV;
> >> >> +
> >> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> >> _chipid_attr);
> >> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> >> _version_attr);
> >> >> +
> >> >> +   dev_info(>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

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

2016-07-11 Thread Wan ZongShun

2016-07-11 18:24 GMT+08:00 Arnd Bergmann :
> 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", 
>> >> _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, _chipid);
>> >> +   if (ret)
>> >> +   return -ENODEV;
>> >> +
>> >> +   device_create_file(soc_device_to_device(soc_dev), 
>> >> _chipid_attr);
>> >> +   device_create_file(soc_device_to_device(soc_dev), 
>> >> _version_attr);
>> >> +
>> >> +   dev_info(>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

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

2016-07-11 Thread Wan ZongShun

2016-07-11 18:24 GMT+08:00 Arnd Bergmann :
> 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", 
>> >> _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, _chipid);
>> >> +   if (ret)
>> >> +   return -ENODEV;
>> >> +
>> >> +   device_create_file(soc_device_to_device(soc_dev), 
>> >> _chipid_attr);
>> >> +   device_create_file(soc_device_to_device(soc_dev), 
>> >> _version_attr);
>> >> +
>> >> +   dev_info(>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

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

2016-07-11 Thread Arnd Bergmann

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", 
> >> _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, _chipid);
> >> +   if (ret)
> >> +   return -ENODEV;
> >> +
> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> _chipid_attr);
> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> _version_attr);
> >> +
> >> +   dev_info(>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

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

2016-07-11 Thread Arnd Bergmann

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", 
> >> _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, _chipid);
> >> +   if (ret)
> >> +   return -ENODEV;
> >> +
> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> _chipid_attr);
> >> +   device_create_file(soc_device_to_device(soc_dev), 
> >> _version_attr);
> >> +
> >> +   dev_info(>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

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

2016-07-11 Thread Wan Zongshun




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", _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, _chipid);
+   if (ret)
+   return -ENODEV;
+
+   device_create_file(soc_device_to_device(soc_dev), _chipid_attr);
+   device_create_file(soc_device_to_device(soc_dev), _version_attr);
+
+   dev_info(>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

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

2016-07-11 Thread Wan Zongshun




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", _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, _chipid);
+   if (ret)
+   return -ENODEV;
+
+   device_create_file(soc_device_to_device(soc_dev), _chipid_attr);
+   device_create_file(soc_device_to_device(soc_dev), _version_attr);
+
+   dev_info(>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

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

2016-07-11 Thread Arnd Bergmann

On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
> +   ret = of_property_read_string(np, "compatible", 
> _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, _chipid);
> +   if (ret)
> +   return -ENODEV;
> +
> +   device_create_file(soc_device_to_device(soc_dev), 
> _chipid_attr);
> +   device_create_file(soc_device_to_device(soc_dev), 
> _version_attr);
> +
> +   dev_info(>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

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

2016-07-11 Thread Arnd Bergmann

On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote:
> +   ret = of_property_read_string(np, "compatible", 
> _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, _chipid);
> +   if (ret)
> +   return -ENODEV;
> +
> +   device_create_file(soc_device_to_device(soc_dev), 
> _chipid_attr);
> +   device_create_file(soc_device_to_device(soc_dev), 
> _version_attr);
> +
> +   dev_info(>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

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

2016-07-10 Thread Wan Zongshun

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 
---
 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 000..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 000..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 000..034ef94
--- /dev/null
+++ b/drivers/soc/nuvoton/soc-nuc900.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright 2016 Wan Zongshun 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* System ID in syscon */
+#define GCR_CHIPID 0x00
+#define GCR_CHIPID_MASK0x00ff
+
+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", _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, _chipid);
+   if (ret)
+   return -ENODEV;
+
+

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

2016-07-10 Thread Wan Zongshun

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 
---
 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 000..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 000..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 000..034ef94
--- /dev/null
+++ b/drivers/soc/nuvoton/soc-nuc900.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright 2016 Wan Zongshun 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* System ID in syscon */
+#define GCR_CHIPID 0x00
+#define GCR_CHIPID_MASK0x00ff
+
+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", _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, _chipid);
+   if (ret)
+   return -ENODEV;
+
+   device_create_file(soc_device_to_device(soc_dev), _chipid_attr);
+

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

16 matches

Site Navigation

Mail list logo

Footer information