[PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Kamel Bouhara]

Add support to read SFR's read-only registers providing the SoC
Serial Numbers (SN0+SN1) to userspace.

~ #  hexdump -n 8  -e'"%d\n"' /sys/bus/nvmem/devices/atmel-sfr0/nvmem
959527243
371539274

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
 Changes in v5:
 - Removed the blankline at EOF
 - Feeded the entropy pool with the SoC SN using add_device_randomness()
   and do it only once at probe().

 drivers/soc/atmel/Kconfig  | 11 +++++
 drivers/soc/atmel/Makefile |  1 +
 drivers/soc/atmel/sfr.c    | 99 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)
 create mode 100644 drivers/soc/atmel/sfr.c

diff --git a/drivers/soc/atmel/Kconfig b/drivers/soc/atmel/Kconfig
index 05528139b023..50caf6db9c0e 100644
--- a/drivers/soc/atmel/Kconfig
+++ b/drivers/soc/atmel/Kconfig
@@ -5,3 +5,14 @@ config AT91_SOC_ID
 	default ARCH_AT91
 	help
 	  Include support for the SoC bus on the Atmel ARM SoCs.
+
+config AT91_SOC_SFR
+	tristate "Special Function Registers support"
+	depends on ARCH_AT91 || COMPILE_TEST
+	help
+	  This is a driver for the Special Function Registers available on
+	  Atmel SAMA5Dx SoCs, providing access to specific aspects of the
+	  integrated memory, bridge implementations, processor etc.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called sfr.
diff --git a/drivers/soc/atmel/Makefile b/drivers/soc/atmel/Makefile
index 7ca355d10553..d849a897cd77 100644
--- a/drivers/soc/atmel/Makefile
+++ b/drivers/soc/atmel/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_AT91_SOC_ID) += soc.o
+obj-$(CONFIG_AT91_SOC_SFR) += sfr.o
diff --git a/drivers/soc/atmel/sfr.c b/drivers/soc/atmel/sfr.c
new file mode 100644
index 000000000000..145e9af63b4c
--- /dev/null
+++ b/drivers/soc/atmel/sfr.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * sfr.c - driver for special function registers
+ *
+ * Copyright (C) 2019 Bootlin.
+ *
+ */
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/random.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define SFR_SN0		0x4c
+#define SFR_SN_SIZE	8
+
+struct atmel_sfr_priv {
+	struct regmap			*regmap;
+};
+
+static int atmel_sfr_read(void *context, unsigned int offset,
+			  void *buf, size_t bytes)
+{
+	struct atmel_sfr_priv *priv = context;
+
+	return regmap_bulk_read(priv->regmap, SFR_SN0 + offset,
+				buf, bytes / 4);
+}
+
+static struct nvmem_config atmel_sfr_nvmem_config = {
+	.name = "atmel-sfr",
+	.read_only = true,
+	.word_size = 4,
+	.stride = 4,
+	.size = SFR_SN_SIZE,
+	.reg_read = atmel_sfr_read,
+};
+
+static int atmel_sfr_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct nvmem_device *nvmem;
+	struct atmel_sfr_priv *priv;
+	u8 sn[SFR_SN_SIZE];
+	int ret;
+
+	priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->regmap = syscon_node_to_regmap(np);
+	if (IS_ERR(priv->regmap)) {
+		dev_err(dev, "cannot get parent's regmap\n");
+		return PTR_ERR(priv->regmap);
+	}
+
+	atmel_sfr_nvmem_config.dev = dev;
+	atmel_sfr_nvmem_config.priv = priv;
+
+	nvmem = devm_nvmem_register(dev, &atmel_sfr_nvmem_config);
+	if (IS_ERR(nvmem)) {
+		dev_err(dev, "error registering nvmem config\n");
+		return PTR_ERR(nvmem);
+	}
+
+	ret = atmel_sfr_read(priv, 0, sn, SFR_SN_SIZE);
+	if (ret == 0)
+		add_device_randomness(sn, SFR_SN_SIZE);
+
+	return 0;
+}
+
+static const struct of_device_id atmel_sfr_dt_ids[] = {
+	{
+		.compatible = "atmel,sama5d2-sfr",
+	}, {
+		.compatible = "atmel,sama5d4-sfr",
+	}, {
+		/* sentinel */
+	},
+};
+MODULE_DEVICE_TABLE(of, atmel_sfr_dt_ids);
+
+static struct platform_driver atmel_sfr_driver = {
+	.probe = atmel_sfr_probe,
+	.driver = {
+		.name = "atmel-sfr",
+		.of_match_table = atmel_sfr_dt_ids,
+	},
+};
+module_platform_driver(atmel_sfr_driver);
+
+MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
+MODULE_DESCRIPTION("Atmel SFR SN driver for SAMA5D2/4 SoC family");
+MODULE_LICENSE("GPL v2");
--
2.23.0


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

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Tudor.Ambarus]

On 10/04/2019 05:12 PM, Kamel Bouhara wrote:
> External E-Mail
> 
> 
> Add support to read SFR's read-only registers providing the SoC
> Serial Numbers (SN0+SN1) to userspace.
> 
> ~ #  hexdump -n 8  -e'"%d\n"' /sys/bus/nvmem/devices/atmel-sfr0/nvmem
> 959527243
> 371539274
> 
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
>  Changes in v5:
>  - Removed the blankline at EOF
>  - Feeded the entropy pool with the SoC SN using add_device_randomness()
>    and do it only once at probe().
> 
>  drivers/soc/atmel/Kconfig  | 11 +++++
>  drivers/soc/atmel/Makefile |  1 +
>  drivers/soc/atmel/sfr.c    | 99 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+)
>  create mode 100644 drivers/soc/atmel/sfr.c
> 
> diff --git a/drivers/soc/atmel/Kconfig b/drivers/soc/atmel/Kconfig
> index 05528139b023..50caf6db9c0e 100644
> --- a/drivers/soc/atmel/Kconfig
> +++ b/drivers/soc/atmel/Kconfig
> @@ -5,3 +5,14 @@ config AT91_SOC_ID
>  	default ARCH_AT91
>  	help
>  	  Include support for the SoC bus on the Atmel ARM SoCs.
> +
> +config AT91_SOC_SFR
> +	tristate "Special Function Registers support"
> +	depends on ARCH_AT91 || COMPILE_TEST
> +	help
> +	  This is a driver for the Special Function Registers available on
> +	  Atmel SAMA5Dx SoCs, providing access to specific aspects of the
> +	  integrated memory, bridge implementations, processor etc.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called sfr.
> diff --git a/drivers/soc/atmel/Makefile b/drivers/soc/atmel/Makefile
> index 7ca355d10553..d849a897cd77 100644
> --- a/drivers/soc/atmel/Makefile
> +++ b/drivers/soc/atmel/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-$(CONFIG_AT91_SOC_ID) += soc.o
> +obj-$(CONFIG_AT91_SOC_SFR) += sfr.o
> diff --git a/drivers/soc/atmel/sfr.c b/drivers/soc/atmel/sfr.c
> new file mode 100644
> index 000000000000..145e9af63b4c
> --- /dev/null
> +++ b/drivers/soc/atmel/sfr.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * sfr.c - driver for special function registers
> + *
> + * Copyright (C) 2019 Bootlin.
> + *
> + */
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/random.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define SFR_SN0		0x4c
> +#define SFR_SN_SIZE	8
> +
> +struct atmel_sfr_priv {
> +	struct regmap			*regmap;
> +};
> +
> +static int atmel_sfr_read(void *context, unsigned int offset,
> +			  void *buf, size_t bytes)
> +{
> +	struct atmel_sfr_priv *priv = context;
> +
> +	return regmap_bulk_read(priv->regmap, SFR_SN0 + offset,
> +				buf, bytes / 4);
> +}
> +
> +static struct nvmem_config atmel_sfr_nvmem_config = {
> +	.name = "atmel-sfr",
> +	.read_only = true,
> +	.word_size = 4,
> +	.stride = 4,
> +	.size = SFR_SN_SIZE,
> +	.reg_read = atmel_sfr_read,
> +};
> +
> +static int atmel_sfr_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct nvmem_device *nvmem;
> +	struct atmel_sfr_priv *priv;
> +	u8 sn[SFR_SN_SIZE];
> +	int ret;
> +
> +	priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->regmap = syscon_node_to_regmap(np);
> +	if (IS_ERR(priv->regmap)) {
> +		dev_err(dev, "cannot get parent's regmap\n");
> +		return PTR_ERR(priv->regmap);
> +	}
> +
> +	atmel_sfr_nvmem_config.dev = dev;
> +	atmel_sfr_nvmem_config.priv = priv;
> +
> +	nvmem = devm_nvmem_register(dev, &atmel_sfr_nvmem_config);
> +	if (IS_ERR(nvmem)) {
> +		dev_err(dev, "error registering nvmem config\n");
> +		return PTR_ERR(nvmem);
> +	}
> +
> +	ret = atmel_sfr_read(priv, 0, sn, SFR_SN_SIZE);
> +	if (ret == 0)
> +		add_device_randomness(sn, SFR_SN_SIZE);
> +
> +	return 0;

	return ret;

to not miss a possible error from atmel_sfr_read().

The code looks good, with this fixed one can add:
Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

As I told in v3, I have some doubts on the use cases for this driver, but let's
see what the at91 folks think.

Cheers,
ta

> +}
> +
> +static const struct of_device_id atmel_sfr_dt_ids[] = {
> +	{
> +		.compatible = "atmel,sama5d2-sfr",
> +	}, {
> +		.compatible = "atmel,sama5d4-sfr",
> +	}, {
> +		/* sentinel */
> +	},
> +};
> +MODULE_DEVICE_TABLE(of, atmel_sfr_dt_ids);
> +
> +static struct platform_driver atmel_sfr_driver = {
> +	.probe = atmel_sfr_probe,
> +	.driver = {
> +		.name = "atmel-sfr",
> +		.of_match_table = atmel_sfr_dt_ids,
> +	},
> +};
> +module_platform_driver(atmel_sfr_driver);
> +
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("Atmel SFR SN driver for SAMA5D2/4 SoC family");
> +MODULE_LICENSE("GPL v2");
> --
> 2.23.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Alexandre Belloni]

On 04/10/2019 14:54:37+0000, Tudor.Ambarus@microchip.com wrote:
> 
> 
> On 10/04/2019 05:12 PM, Kamel Bouhara wrote:
> > External E-Mail
> > 
> > 
> > Add support to read SFR's read-only registers providing the SoC
> > Serial Numbers (SN0+SN1) to userspace.
> > 
> > ~ #  hexdump -n 8  -e'"%d\n"' /sys/bus/nvmem/devices/atmel-sfr0/nvmem
> > 959527243
> > 371539274
> > 
> > Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> > ---
> >  Changes in v5:
> >  - Removed the blankline at EOF
> >  - Feeded the entropy pool with the SoC SN using add_device_randomness()
> >    and do it only once at probe().
> > 
> >  drivers/soc/atmel/Kconfig  | 11 +++++
> >  drivers/soc/atmel/Makefile |  1 +
> >  drivers/soc/atmel/sfr.c    | 99 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 111 insertions(+)
> >  create mode 100644 drivers/soc/atmel/sfr.c
> > 
> > diff --git a/drivers/soc/atmel/Kconfig b/drivers/soc/atmel/Kconfig
> > index 05528139b023..50caf6db9c0e 100644
> > --- a/drivers/soc/atmel/Kconfig
> > +++ b/drivers/soc/atmel/Kconfig
> > @@ -5,3 +5,14 @@ config AT91_SOC_ID
> >  	default ARCH_AT91
> >  	help
> >  	  Include support for the SoC bus on the Atmel ARM SoCs.
> > +
> > +config AT91_SOC_SFR
> > +	tristate "Special Function Registers support"
> > +	depends on ARCH_AT91 || COMPILE_TEST
> > +	help
> > +	  This is a driver for the Special Function Registers available on
> > +	  Atmel SAMA5Dx SoCs, providing access to specific aspects of the
> > +	  integrated memory, bridge implementations, processor etc.
> > +
> > +	  This driver can also be built as a module. If so, the module
> > +	  will be called sfr.
> > diff --git a/drivers/soc/atmel/Makefile b/drivers/soc/atmel/Makefile
> > index 7ca355d10553..d849a897cd77 100644
> > --- a/drivers/soc/atmel/Makefile
> > +++ b/drivers/soc/atmel/Makefile
> > @@ -1,2 +1,3 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  obj-$(CONFIG_AT91_SOC_ID) += soc.o
> > +obj-$(CONFIG_AT91_SOC_SFR) += sfr.o
> > diff --git a/drivers/soc/atmel/sfr.c b/drivers/soc/atmel/sfr.c
> > new file mode 100644
> > index 000000000000..145e9af63b4c
> > --- /dev/null
> > +++ b/drivers/soc/atmel/sfr.c
> > @@ -0,0 +1,99 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * sfr.c - driver for special function registers
> > + *
> > + * Copyright (C) 2019 Bootlin.
> > + *
> > + */
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/nvmem-provider.h>
> > +#include <linux/random.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#define SFR_SN0		0x4c
> > +#define SFR_SN_SIZE	8
> > +
> > +struct atmel_sfr_priv {
> > +	struct regmap			*regmap;
> > +};
> > +
> > +static int atmel_sfr_read(void *context, unsigned int offset,
> > +			  void *buf, size_t bytes)
> > +{
> > +	struct atmel_sfr_priv *priv = context;
> > +
> > +	return regmap_bulk_read(priv->regmap, SFR_SN0 + offset,
> > +				buf, bytes / 4);
> > +}
> > +
> > +static struct nvmem_config atmel_sfr_nvmem_config = {
> > +	.name = "atmel-sfr",
> > +	.read_only = true,
> > +	.word_size = 4,
> > +	.stride = 4,
> > +	.size = SFR_SN_SIZE,
> > +	.reg_read = atmel_sfr_read,
> > +};
> > +
> > +static int atmel_sfr_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	struct nvmem_device *nvmem;
> > +	struct atmel_sfr_priv *priv;
> > +	u8 sn[SFR_SN_SIZE];
> > +	int ret;
> > +
> > +	priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->regmap = syscon_node_to_regmap(np);
> > +	if (IS_ERR(priv->regmap)) {
> > +		dev_err(dev, "cannot get parent's regmap\n");
> > +		return PTR_ERR(priv->regmap);
> > +	}
> > +
> > +	atmel_sfr_nvmem_config.dev = dev;
> > +	atmel_sfr_nvmem_config.priv = priv;
> > +
> > +	nvmem = devm_nvmem_register(dev, &atmel_sfr_nvmem_config);
> > +	if (IS_ERR(nvmem)) {
> > +		dev_err(dev, "error registering nvmem config\n");
> > +		return PTR_ERR(nvmem);
> > +	}
> > +
> > +	ret = atmel_sfr_read(priv, 0, sn, SFR_SN_SIZE);
> > +	if (ret == 0)
> > +		add_device_randomness(sn, SFR_SN_SIZE);
> > +
> > +	return 0;
> 
> 	return ret;
> 
> to not miss a possible error from atmel_sfr_read().
> 
> The code looks good, with this fixed one can add:
> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> As I told in v3, I have some doubts on the use cases for this driver, but let's
> see what the at91 folks think.
> 

There is already at least one microchip customer using it so I'm going
to apply it.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Tudor.Ambarus]

On 10/04/2019 06:00 PM, Alexandre Belloni wrote:
>> The code looks good, with this fixed one can add:
>> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> As I told in v3, I have some doubts on the use cases for this driver, but let's
>> see what the at91 folks think.
>>
> There is already at least one microchip customer using it so I'm going
> to apply it.

This is not an use case.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Alexandre Belloni]

On 04/10/2019 15:04:59+0000, Tudor.Ambarus@microchip.com wrote:
> 
> 
> On 10/04/2019 06:00 PM, Alexandre Belloni wrote:
> >> The code looks good, with this fixed one can add:
> >> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >>
> >> As I told in v3, I have some doubts on the use cases for this driver, but let's
> >> see what the at91 folks think.
> >>
> > There is already at least one microchip customer using it so I'm going
> > to apply it.
> 
> This is not an use case.

this avoids adding a ds2401 for example to get a unique serial number on
the board and reduces production cost because there is no eeprom to
write as the SN is already on the SoC.

If you are not convinced, maybe you should ask your hardware designers
why they added this feature (I guess the answer is customer request).

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Tudor.Ambarus]

On 10/04/2019 06:10 PM, Alexandre Belloni wrote:
> External E-Mail
> 
> 
> On 04/10/2019 15:04:59+0000, Tudor.Ambarus@microchip.com wrote:
>>
>>
>> On 10/04/2019 06:00 PM, Alexandre Belloni wrote:
>>>> The code looks good, with this fixed one can add:
>>>> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>
>>>> As I told in v3, I have some doubts on the use cases for this driver, but let's
>>>> see what the at91 folks think.
>>>>
>>> There is already at least one microchip customer using it so I'm going
>>> to apply it.
>>
>> This is not an use case.
> 
> this avoids adding a ds2401 for example to get a unique serial number on
> the board and reduces production cost because there is no eeprom to
> write as the SN is already on the SoC.
> 

I was not against it, I just wanted to know the reasons. Thanks for sharing this.

> If you are not convinced, maybe you should ask your hardware designers
> why they added this feature (I guess the answer is customer request).

Maybe a CRC should have been added, as in ds2401.

Take care,
ta
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Nicolas.Ferre]

On 04/10/2019 at 16:54, Tudor Ambarus - M18064 wrote:
> 
> 
> On 10/04/2019 05:12 PM, Kamel Bouhara wrote:
>> External E-Mail
>>
>>
>> Add support to read SFR's read-only registers providing the SoC
>> Serial Numbers (SN0+SN1) to userspace.
>>
>> ~ #  hexdump -n 8  -e'"%d\n"' /sys/bus/nvmem/devices/atmel-sfr0/nvmem
>> 959527243
>> 371539274
>>
>> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> ---
>>   Changes in v5:
>>   - Removed the blankline at EOF
>>   - Feeded the entropy pool with the SoC SN using add_device_randomness()
>>     and do it only once at probe().
>>
>>   drivers/soc/atmel/Kconfig  | 11 +++++
>>   drivers/soc/atmel/Makefile |  1 +
>>   drivers/soc/atmel/sfr.c    | 99 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 111 insertions(+)
>>   create mode 100644 drivers/soc/atmel/sfr.c
>>
>> diff --git a/drivers/soc/atmel/Kconfig b/drivers/soc/atmel/Kconfig
>> index 05528139b023..50caf6db9c0e 100644
>> --- a/drivers/soc/atmel/Kconfig
>> +++ b/drivers/soc/atmel/Kconfig
>> @@ -5,3 +5,14 @@ config AT91_SOC_ID
>>   	default ARCH_AT91
>>   	help
>>   	  Include support for the SoC bus on the Atmel ARM SoCs.
>> +
>> +config AT91_SOC_SFR
>> +	tristate "Special Function Registers support"
>> +	depends on ARCH_AT91 || COMPILE_TEST
>> +	help
>> +	  This is a driver for the Special Function Registers available on
>> +	  Atmel SAMA5Dx SoCs, providing access to specific aspects of the
>> +	  integrated memory, bridge implementations, processor etc.
>> +
>> +	  This driver can also be built as a module. If so, the module
>> +	  will be called sfr.
>> diff --git a/drivers/soc/atmel/Makefile b/drivers/soc/atmel/Makefile
>> index 7ca355d10553..d849a897cd77 100644
>> --- a/drivers/soc/atmel/Makefile
>> +++ b/drivers/soc/atmel/Makefile
>> @@ -1,2 +1,3 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   obj-$(CONFIG_AT91_SOC_ID) += soc.o
>> +obj-$(CONFIG_AT91_SOC_SFR) += sfr.o
>> diff --git a/drivers/soc/atmel/sfr.c b/drivers/soc/atmel/sfr.c
>> new file mode 100644
>> index 000000000000..145e9af63b4c
>> --- /dev/null
>> +++ b/drivers/soc/atmel/sfr.c
>> @@ -0,0 +1,99 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * sfr.c - driver for special function registers
>> + *
>> + * Copyright (C) 2019 Bootlin.
>> + *
>> + */
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/random.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#define SFR_SN0		0x4c
>> +#define SFR_SN_SIZE	8
>> +
>> +struct atmel_sfr_priv {
>> +	struct regmap			*regmap;
>> +};
>> +
>> +static int atmel_sfr_read(void *context, unsigned int offset,
>> +			  void *buf, size_t bytes)
>> +{
>> +	struct atmel_sfr_priv *priv = context;
>> +
>> +	return regmap_bulk_read(priv->regmap, SFR_SN0 + offset,
>> +				buf, bytes / 4);
>> +}
>> +
>> +static struct nvmem_config atmel_sfr_nvmem_config = {
>> +	.name = "atmel-sfr",
>> +	.read_only = true,
>> +	.word_size = 4,
>> +	.stride = 4,
>> +	.size = SFR_SN_SIZE,
>> +	.reg_read = atmel_sfr_read,
>> +};
>> +
>> +static int atmel_sfr_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np = dev->of_node;
>> +	struct nvmem_device *nvmem;
>> +	struct atmel_sfr_priv *priv;
>> +	u8 sn[SFR_SN_SIZE];
>> +	int ret;
>> +
>> +	priv = devm_kmalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->regmap = syscon_node_to_regmap(np);
>> +	if (IS_ERR(priv->regmap)) {
>> +		dev_err(dev, "cannot get parent's regmap\n");
>> +		return PTR_ERR(priv->regmap);
>> +	}
>> +
>> +	atmel_sfr_nvmem_config.dev = dev;
>> +	atmel_sfr_nvmem_config.priv = priv;
>> +
>> +	nvmem = devm_nvmem_register(dev, &atmel_sfr_nvmem_config);
>> +	if (IS_ERR(nvmem)) {
>> +		dev_err(dev, "error registering nvmem config\n");
>> +		return PTR_ERR(nvmem);
>> +	}
>> +
>> +	ret = atmel_sfr_read(priv, 0, sn, SFR_SN_SIZE);
>> +	if (ret == 0)
>> +		add_device_randomness(sn, SFR_SN_SIZE);
>> +
>> +	return 0;
> 
> 	return ret;
> 
> to not miss a possible error from atmel_sfr_read().
> 
> The code looks good, with this fixed one can add:
> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> As I told in v3, I have some doubts on the use cases for this driver, but let's
> see what the at91 folks think.

Well, I'm sure to not know all the applications of this feature but 
surely unique serial number (per single chip) is very useful while 
wanting to identify your system precisely: attributing a MAC address, 
pairing with one service or user software, generating unique keys, etc.

What I don't know is if there is special API in the kernel to use it as 
several vendors seem to expose it differently (/proc/cpuinfo being one 
other option). This one in nvmem /sys file is surely a valid one.

Best regards,
   Nicolas

>> +static const struct of_device_id atmel_sfr_dt_ids[] = {
>> +	{
>> +		.compatible = "atmel,sama5d2-sfr",
>> +	}, {
>> +		.compatible = "atmel,sama5d4-sfr",
>> +	}, {
>> +		/* sentinel */
>> +	},
>> +};
>> +MODULE_DEVICE_TABLE(of, atmel_sfr_dt_ids);
>> +
>> +static struct platform_driver atmel_sfr_driver = {
>> +	.probe = atmel_sfr_probe,
>> +	.driver = {
>> +		.name = "atmel-sfr",
>> +		.of_match_table = atmel_sfr_dt_ids,
>> +	},
>> +};
>> +module_platform_driver(atmel_sfr_driver);
>> +
>> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
>> +MODULE_DESCRIPTION("Atmel SFR SN driver for SAMA5D2/4 SoC family");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.23.0
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>


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

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Alexander Dahl]

[-- Attachment #1.1: Type: text/plain, Size: 1754 bytes --]

Hei hei,

I know this was already applied, but let me add some comments.

On Fri, Oct 04, 2019 at 03:25:27PM +0000, Nicolas.Ferre@microchip.com wrote:
> Well, I'm sure to not know all the applications of this feature but 
> surely unique serial number (per single chip) is very useful while 
> wanting to identify your system precisely: attributing a MAC address, 
> pairing with one service or user software, generating unique keys, etc.

+1

> What I don't know is if there is special API in the kernel to use it as 
> several vendors seem to expose it differently (/proc/cpuinfo being one 
> other option). This one in nvmem /sys file is surely a valid one.

With commit 9aebf4de2203 ("base: soc: Add serial_number attribute to soc")
there was added a member serial_number to struct soc_device_attribute.
As far as I can see this is part of mainline since v5.4-rc1.

I saw some patches for i.MX on this list recently, which also got
applied, and which use that mechanism. So there seem to be at least
two different ways to access this now.

FWIW: I was working on a patch for exposing the sama5d2 SN through
that interface. If anyone is interested: 

https://github.com/LeSpocky/linux/tree/wip/sama5d2-sn-squash

One thing I noticed on the original patch: there's already a header
file for register offsets in SFR block: soc/at91/atmel-sfr.h – IMHO
this should have been used instead of defining those in .c file.

Greets
Alex

-- 
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN     | speech censured, the first thought forbidden, the
 X  AGAINST      | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL    | (Jean-Luc Picard, quoting Judge Aaron Satie)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH v5] soc: at91: Add Atmel SFR SN (Serial Number) support

[Alexandre Belloni]

On 01/11/2019 08:13:49+0100, Alexander Dahl wrote:
> Hei hei,
> 
> I know this was already applied, but let me add some comments.
> 
> On Fri, Oct 04, 2019 at 03:25:27PM +0000, Nicolas.Ferre@microchip.com wrote:
> > Well, I'm sure to not know all the applications of this feature but 
> > surely unique serial number (per single chip) is very useful while 
> > wanting to identify your system precisely: attributing a MAC address, 
> > pairing with one service or user software, generating unique keys, etc.
> 
> +1
> 
> > What I don't know is if there is special API in the kernel to use it as 
> > several vendors seem to expose it differently (/proc/cpuinfo being one 
> > other option). This one in nvmem /sys file is surely a valid one.
> 
> With commit 9aebf4de2203 ("base: soc: Add serial_number attribute to soc")
> there was added a member serial_number to struct soc_device_attribute.
> As far as I can see this is part of mainline since v5.4-rc1.
> 
> I saw some patches for i.MX on this list recently, which also got
> applied, and which use that mechanism. So there seem to be at least
> two different ways to access this now.
> 
> FWIW: I was working on a patch for exposing the sama5d2 SN through
> that interface. If anyone is interested: 
> 
> https://github.com/LeSpocky/linux/tree/wip/sama5d2-sn-squash
> 

The main issue with that interface is that there is no way to consume
the SN from the kernel while nvmem has both in-kernel and userspace
APIs.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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