Hi Srinivas, Thansks for providing the comments.. Please find my repsonce inline... > -----Original Message----- > From: Srinivas Kandagatla [mailto:srinivas.kandagatla@xxxxxxxxxx] > Sent: Sunday, September 16, 2018 7:41 PM > To: Nava kishore Manne <navam@xxxxxxxxxx>; robh+dt@xxxxxxxxxx; > mark.rutland@xxxxxxx; Michal Simek <michals@xxxxxxxxxx>; Rajan Vaja > <RAJANV@xxxxxxxxxx>; Jolly Shah <JOLLYS@xxxxxxxxxx>; > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [RFC PATCH v2 3/3] nvmem: zynqmp: Added zynqmp nvmem > firmware driver > > Few minor Nits... > > I remember asking about this in the last review! > > On 04/09/18 10:52, Nava kishore Manne wrote: > > This patch adds zynqmp nvmem firmware driver to access the SoC > > revision information from the hardware register. > > > > Signed-off-by: Nava kishore Manne <nava.manne@xxxxxxxxxx> > > --- > > diff --git a/drivers/nvmem/zynqmp_nvmem.c > > b/drivers/nvmem/zynqmp_nvmem.c new file mode 100644 index > > 0000000..e377900 > > --- /dev/null > > +++ b/drivers/nvmem/zynqmp_nvmem.c > > @@ -0,0 +1,84 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2018 Xilinx, Inc. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/nvmem-provider.h> > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > +#include <linux/firmware/xlnx-zynqmp.h> > > + > > +#define SILICON_REVISION_MASK 0xF > > + > > +static int zynqmp_nvmem_read(void *context, unsigned int offset, > > + void *val, size_t bytes) > > +{ > > + int ret; > > + int idcode, version; > > + const struct zynqmp_eemi_ops *eemi_ops = > zynqmp_pm_get_eemi_ops(); > > + > > + if (!eemi_ops || !eemi_ops->get_chipid) > > + return -ENXIO; > > + > > + ret = eemi_ops->get_chipid(&idcode, &version); > > + if (ret < 0) > > + return ret; > > + > > + pr_debug("Read chipid val %x %x\n", idcode, version); > > + *(int *)val = version & SILICON_REVISION_MASK; > > Use dev_dbg here. Will fix in the next version.. > > + > > + return 0; > > +} > > + > > +static struct nvmem_config econfig = { > > + .name = "zynqmp-nvmem", > > + .owner = THIS_MODULE, > > + .word_size = 1, > > + .size = 1, > > + .read_only = true, > > +}; > > + > > +static const struct of_device_id zynqmp_nvmem_match[] = { > > + { .compatible = "xlnx,zynqmp-nvmem-fw", }, > > + { /* sentinel */ }, > > +}; > > +MODULE_DEVICE_TABLE(of, zynqmp_nvmem_match); > > + > > +static int zynqmp_nvmem_probe(struct platform_device *pdev) { > > + struct nvmem_device *nvmem; > > + > > + econfig.dev = &pdev->dev; > > + econfig.reg_read = zynqmp_nvmem_read; > > + > > + nvmem = nvmem_register(&econfig); > You can use devm_ variant and remove the zynqmp_nvmem_remove() totally > Will fix in the next version... Regards, Navakishore.
