| Message ID | 20191003114606.7846-1-kamel.bouhara@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] soc: at91: Add Atmel SFR SN (Serial Number) support | expand |
Hi, Kamel, On 10/03/2019 02:46 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 nitpick probably it's better to print the SN in hex, so %08x instead of %d > 959527243 > 371539274 > > Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com> Is the SN unique for each device or it is unique per SoC? For example, for sama5d2, I get the following (in hex): root@sama5d2-xplained-sd:~# hexdump -n 8 -e '"%08x\n"' /sys/bus/nvmem/devices/atmel-sfr0/nvmem 4643524b 02010657 If this is unique for each device, then maybe is it worth to add the serial number in the entropy pool (with add_device_randomness()?) What was your use case, why do you need to print the SN? And you have a blank line at EOF, but probably the maintainers can remove it, if the patch will remain as is. Cheers, ta > --- > Changes in v3: > - Fixed typo: processor in Kconfig > - Renamed private struct sfr_priv to atmel_sfr_priv > - Dropped the drvdata structure as we have same size for both SN > registers in SAMA5D2/4, just hardcoded it for now. > - Cleaned up private struct from unused members > - Fixed misusage of devm_kzalloc > > drivers/soc/atmel/Kconfig | 11 +++++ > drivers/soc/atmel/Makefile | 1 + > drivers/soc/atmel/sfr.c | 93 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 105 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..e92050b4c09e > --- /dev/null > +++ b/drivers/soc/atmel/sfr.c > @@ -0,0 +1,93 @@ > +// 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/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; > + > + 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); > + } > + > + 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 > >
Hi Tudor, On 10/4/19 9:29 AM, Tudor.Ambarus@microchip.com wrote: > Hi, Kamel, > > On 10/03/2019 02:46 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 > nitpick probably it's better to print the SN in hex, so %08x instead of %d Of course, it is just an example of user space command, this is completely up to user to choose the relevant format. >> 959527243 >> 371539274 >> >> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com> > Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com> > > Is the SN unique for each device or it is unique per SoC? For example, for > sama5d2, I get the following (in hex): > > root@sama5d2-xplained-sd:~# hexdump -n 8 -e '"%08x\n"' > /sys/bus/nvmem/devices/atmel-sfr0/nvmem > 4643524b > 02010657 > > If this is unique for each device, then maybe is it worth to add the serial > number in the entropy pool (with add_device_randomness()?) Yes it is. Maybe it is not suited to throw a non random 8 bytes (only) number to the entropy pool ? By the way, some SoC drivers do it already. Shall I propose a v4 for this ? > > What was your use case, why do you need to print the SN? It can be used in a factory process to associate a mac address for instance... but I'm sure there is lot of use case using a unique identifier. > > And you have a blank line at EOF, but probably the maintainers can remove it, if > the patch will remain as is. OK, thanks for you review, If so, it will be fixed in v4. Kamel
On 10/04/2019 12:31 PM, Kamel Bouhara wrote: > External E-Mail > > > Hi Tudor, Hi, Kamel, > > On 10/4/19 9:29 AM, Tudor.Ambarus@microchip.com wrote: >> Hi, Kamel, >> >> On 10/03/2019 02:46 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 >> nitpick probably it's better to print the SN in hex, so %08x instead of %d > Of course, it is just an example of user space command, this is completely up to user to choose the relevant format. >>> 959527243 >>> 371539274 >>> >>> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com> >> Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> >> Is the SN unique for each device or it is unique per SoC? For example, for >> sama5d2, I get the following (in hex): >> >> root@sama5d2-xplained-sd:~# hexdump -n 8 -e '"%08x\n"' >> /sys/bus/nvmem/devices/atmel-sfr0/nvmem >> 4643524b >> 02010657 >> >> If this is unique for each device, then maybe is it worth to add the serial >> number in the entropy pool (with add_device_randomness()?) > > Yes it is. Maybe it is not suited to throw a non random 8 bytes (only) number to the entropy pool ? > > By the way, some SoC drivers do it already. > > Shall I propose a v4 for this ? I would. This is more like an improvement, it's not mandatory, it's your call. The code looks good. I have some doubts on the use cases though, but the at91 maintainers will probably decide on this. Let me know if you are sending v4, so I can know where to add my R-b tag :) > >> >> What was your use case, why do you need to print the SN? > It can be used in a factory process to associate a mac address for instance... but I'm sure there is lot of use case using a unique identifier. I don't know the strategy in regards to serial numbers. I guess there are some IPs that have fields for serial numbers in their registers, should we expose them all? Cheers, ta >> >> And you have a blank line at EOF, but probably the maintainers can remove it, if >> the patch will remain as is. > > OK, thanks for you review, If so, it will be fixed in v4. > > Kamel >
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..e92050b4c09e --- /dev/null +++ b/drivers/soc/atmel/sfr.c @@ -0,0 +1,93 @@ +// 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/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; + + 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); + } + + 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"); +
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 v3: - Fixed typo: processor in Kconfig - Renamed private struct sfr_priv to atmel_sfr_priv - Dropped the drvdata structure as we have same size for both SN registers in SAMA5D2/4, just hardcoded it for now. - Cleaned up private struct from unused members - Fixed misusage of devm_kzalloc drivers/soc/atmel/Kconfig | 11 +++++ drivers/soc/atmel/Makefile | 1 + drivers/soc/atmel/sfr.c | 93 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 drivers/soc/atmel/sfr.c -- 2.23.0