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

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

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