[PATCH 0/5] nvmem: some cleanups and sparse warning fixes

[PATCH 0/5] nvmem: some cleanups and sparse warning fixes

[Masahiro Yamada]

Socionext is trying to add a new nvmem driver.

Before adding a new one, I'd like to be sure about the preferred
coding style in this subsystem, and build-up cleaner code base.

Please pick up what you like.  Please feel free to throw away
what you do not like.

1/5, 2/5: convert kzalloc to stack because nvmem_config
is small one-time use data.
This is what we agreed in the discussion.

3/5, 4/5: fix sparse warning
For mtk-efuse, qfprom, the base address is the only private data
but, assigning (void __iomem *) to ->priv directly causes sparse
warnings.

5/5: clean-up of owner settings - if the owner field matches
nvmem->dev->driver->onwer, we need not set the onwer field
explicitly.



Masahiro Yamada (5):
  nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it
  nvmem: mtk-efuse: use stack for nvmem_config instead of malloc'ing it
  nvmem: mtk-efuse: fix different address space warnings of sparse
  nvmem: qfprom: fix different address space warnings of sparse
  nvmem: set nvmem->owner to nvmem->dev->driver->owner if unset

 drivers/nvmem/bcm-ocotp.c      |  1 -
 drivers/nvmem/core.c           |  2 ++
 drivers/nvmem/imx-iim.c        | 24 +++++++++------------
 drivers/nvmem/imx-ocotp.c      |  1 -
 drivers/nvmem/lpc18xx_eeprom.c |  1 -
 drivers/nvmem/lpc18xx_otp.c    |  1 -
 drivers/nvmem/meson-efuse.c    |  1 -
 drivers/nvmem/mtk-efuse.c      | 47 ++++++++++++++++++++++--------------------
 drivers/nvmem/mxs-ocotp.c      |  1 -
 drivers/nvmem/qfprom.c         | 27 +++++++++++++++---------
 drivers/nvmem/rockchip-efuse.c |  1 -
 drivers/nvmem/sunxi_sid.c      |  1 -
 drivers/nvmem/vf610-ocotp.c    |  1 -
 13 files changed, 54 insertions(+), 55 deletions(-)

-- 
2.7.4

[PATCH 1/5] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

nvmem_register() copies all the members of nvmem_config to
nvmem_device.  So, nvmem_config is one-time use data during
probing.  There is no point to keep it until the driver detach.
Using stack should be no problem because nvmem_config is pretty
small.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/nvmem/imx-iim.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/nvmem/imx-iim.c b/drivers/nvmem/imx-iim.c
index 52ff65e..a599260 100644
--- a/drivers/nvmem/imx-iim.c
+++ b/drivers/nvmem/imx-iim.c
@@ -34,7 +34,6 @@ struct imx_iim_drvdata {
 struct iim_priv {
 	void __iomem *base;
 	struct clk *clk;
-	struct nvmem_config nvmem;
 };
 
 static int imx_iim_read(void *context, unsigned int offset,
@@ -108,7 +107,7 @@ static int imx_iim_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct iim_priv *iim;
 	struct nvmem_device *nvmem;
-	struct nvmem_config *cfg;
+	struct nvmem_config cfg = {};
 	const struct imx_iim_drvdata *drvdata = NULL;
 
 	iim = devm_kzalloc(dev, sizeof(*iim), GFP_KERNEL);
@@ -130,19 +129,17 @@ static int imx_iim_probe(struct platform_device *pdev)
 	if (IS_ERR(iim->clk))
 		return PTR_ERR(iim->clk);
 
-	cfg = &iim->nvmem;
-
-	cfg->name = "imx-iim",
-	cfg->read_only = true,
-	cfg->word_size = 1,
-	cfg->stride = 1,
-	cfg->owner = THIS_MODULE,
-	cfg->reg_read = imx_iim_read,
-	cfg->dev = dev;
-	cfg->size = drvdata->nregs;
-	cfg->priv = iim;
-
-	nvmem = nvmem_register(cfg);
+	cfg.name = "imx-iim",
+	cfg.read_only = true,
+	cfg.word_size = 1,
+	cfg.stride = 1,
+	cfg.owner = THIS_MODULE,
+	cfg.reg_read = imx_iim_read,
+	cfg.dev = dev;
+	cfg.size = drvdata->nregs;
+	cfg.priv = iim;
+
+	nvmem = nvmem_register(&cfg);
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
-- 
2.7.4

[PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

nvmem_register() copies all the members of nvmem_config to
nvmem_device.  So, nvmem_config is one-time use data during
probing.  There is no point to keep it until the driver detach.
Using stack should be no problem because nvmem_config is pretty
small.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/nvmem/mtk-efuse.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
index 32fd572..fa7a0f6 100644
--- a/drivers/nvmem/mtk-efuse.c
+++ b/drivers/nvmem/mtk-efuse.c
@@ -49,7 +49,7 @@ static int mtk_efuse_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct nvmem_device *nvmem;
-	struct nvmem_config *econfig;
+	struct nvmem_config econfig = {};
 	void __iomem *base;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -57,19 +57,15 @@ static int mtk_efuse_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
-	if (!econfig)
-		return -ENOMEM;
-
-	econfig->stride = 4;
-	econfig->word_size = 4;
-	econfig->reg_read = mtk_reg_read;
-	econfig->reg_write = mtk_reg_write;
-	econfig->size = resource_size(res);
-	econfig->priv = base;
-	econfig->dev = dev;
-	econfig->owner = THIS_MODULE;
-	nvmem = nvmem_register(econfig);
+	econfig.stride = 4;
+	econfig.word_size = 4;
+	econfig.reg_read = mtk_reg_read;
+	econfig.reg_write = mtk_reg_write;
+	econfig.size = resource_size(res);
+	econfig.priv = base;
+	econfig.dev = dev;
+	econfig.owner = THIS_MODULE;
+	nvmem = nvmem_register(&econfig);
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
-- 
2.7.4

[PATCH 3/5] nvmem: mtk-efuse: fix different address space warnings of sparse

[Masahiro Yamada]

Fix the following sparse warnings:

drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)
drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base
drivers/nvmem/mtk-efuse.c:24:30:    got void *context
drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)
drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base
drivers/nvmem/mtk-efuse.c:37:30:    got void *context
drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)
drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv
drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base

The type of nvmem_config->priv is (void *), so sparse complains
about assignment of the base address with (void __iomem *) type.

Even if we cast it out, sparse still warns:
warning: cast removes address space of expression

Of course, we can shut up the sparse by marking __force, but a more
correct way is to put the base address into driver private data.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/nvmem/mtk-efuse.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
index fa7a0f6..c4058b5 100644
--- a/drivers/nvmem/mtk-efuse.c
+++ b/drivers/nvmem/mtk-efuse.c
@@ -18,15 +18,19 @@
 #include <linux/nvmem-provider.h>
 #include <linux/platform_device.h>
 
+struct mtk_efuse_priv {
+	void __iomem *base;
+};
+
 static int mtk_reg_read(void *context,
 			unsigned int reg, void *_val, size_t bytes)
 {
-	void __iomem *base = context;
+	struct mtk_efuse_priv *priv = context;
 	u32 *val = _val;
 	int i = 0, words = bytes / 4;
 
 	while (words--)
-		*val++ = readl(base + reg + (i++ * 4));
+		*val++ = readl(priv->base + reg + (i++ * 4));
 
 	return 0;
 }
@@ -34,12 +38,12 @@ static int mtk_reg_read(void *context,
 static int mtk_reg_write(void *context,
 			 unsigned int reg, void *_val, size_t bytes)
 {
-	void __iomem *base = context;
+	struct mtk_efuse_priv *priv = context;
 	u32 *val = _val;
 	int i = 0, words = bytes / 4;
 
 	while (words--)
-		writel(*val++, base + reg + (i++ * 4));
+		writel(*val++, priv->base + reg + (i++ * 4));
 
 	return 0;
 }
@@ -50,19 +54,23 @@ static int mtk_efuse_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct nvmem_device *nvmem;
 	struct nvmem_config econfig = {};
-	void __iomem *base;
+	struct mtk_efuse_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
 
 	econfig.stride = 4;
 	econfig.word_size = 4;
 	econfig.reg_read = mtk_reg_read;
 	econfig.reg_write = mtk_reg_write;
 	econfig.size = resource_size(res);
-	econfig.priv = base;
+	econfig.priv = priv;
 	econfig.dev = dev;
 	econfig.owner = THIS_MODULE;
 	nvmem = nvmem_register(&econfig);
-- 
2.7.4

[PATCH 4/5] nvmem: qfprom: fix different address space warnings of sparse

[Masahiro Yamada]

Fix the following sparse warnings:

drivers/nvmem/qfprom.c:23:30: warning: incorrect type in initializer (different address spaces)
drivers/nvmem/qfprom.c:23:30:    expected void [noderef] <asn:2>*base
drivers/nvmem/qfprom.c:23:30:    got void *context
drivers/nvmem/qfprom.c:36:30: warning: incorrect type in initializer (different address spaces)
drivers/nvmem/qfprom.c:36:30:    expected void [noderef] <asn:2>*base
drivers/nvmem/qfprom.c:36:30:    got void *context
drivers/nvmem/qfprom.c:76:22: warning: incorrect type in assignment (different address spaces)
drivers/nvmem/qfprom.c:76:22:    expected void *static [toplevel] [assigned] priv
drivers/nvmem/qfprom.c:76:22:    got void [noderef] <asn:2>*[assigned] base

The type of nvmem_config->priv is (void *), so sparse complains
about assignment of the base address with (void __iomem *) type.

Even if we cast it out, sparse still warns:
warning: cast removes address space of expression

Of course, we can shut up the sparse by marking __force, but a more
correct way is to put the base address into driver private data.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/nvmem/qfprom.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index 2bdb6c3..b96730e 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -17,15 +17,19 @@
 #include <linux/nvmem-provider.h>
 #include <linux/platform_device.h>
 
+struct qfprom_priv {
+	void __iomem *base;
+};
+
 static int qfprom_reg_read(void *context,
 			unsigned int reg, void *_val, size_t bytes)
 {
-	void __iomem *base = context;
+	struct qfprom_priv *priv = context;
 	u8 *val = _val;
 	int i = 0, words = bytes;
 
 	while (words--)
-		*val++ = readb(base + reg + i++);
+		*val++ = readb(priv->base + reg + i++);
 
 	return 0;
 }
@@ -33,12 +37,12 @@ static int qfprom_reg_read(void *context,
 static int qfprom_reg_write(void *context,
 			 unsigned int reg, void *_val, size_t bytes)
 {
-	void __iomem *base = context;
+	struct qfprom_priv *priv = context;
 	u8 *val = _val;
 	int i = 0, words = bytes;
 
 	while (words--)
-		writeb(*val++, base + reg + i++);
+		writeb(*val++, priv->base + reg + i++);
 
 	return 0;
 }
@@ -64,16 +68,20 @@ static int qfprom_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct nvmem_device *nvmem;
-	void __iomem *base;
+	struct qfprom_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
 
 	econfig.size = resource_size(res);
 	econfig.dev = dev;
-	econfig.priv = base;
+	econfig.priv = priv;
 
 	nvmem = nvmem_register(&econfig);
 	if (IS_ERR(nvmem))
-- 
2.7.4

[PATCH 5/5] nvmem: set nvmem->owner to nvmem->dev->driver->owner if unset

[Masahiro Yamada]

All nvmem drivers are supposed to set the owner field of struct
nvmem_config, but this matches nvmem->dev->driver->owner.

As far as I see in drivers/nvmem/ directory, all the drivers are
the case.  So, make nvmem_register() set the nvmem's owner to the
associated driver's owner unless nvmem_config sets otherwise.

Remove .owner settings in the drivers that are now redundant.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/nvmem/bcm-ocotp.c      | 1 -
 drivers/nvmem/core.c           | 2 ++
 drivers/nvmem/imx-iim.c        | 1 -
 drivers/nvmem/imx-ocotp.c      | 1 -
 drivers/nvmem/lpc18xx_eeprom.c | 1 -
 drivers/nvmem/lpc18xx_otp.c    | 1 -
 drivers/nvmem/meson-efuse.c    | 1 -
 drivers/nvmem/mtk-efuse.c      | 1 -
 drivers/nvmem/mxs-ocotp.c      | 1 -
 drivers/nvmem/qfprom.c         | 1 -
 drivers/nvmem/rockchip-efuse.c | 1 -
 drivers/nvmem/sunxi_sid.c      | 1 -
 drivers/nvmem/vf610-ocotp.c    | 1 -
 13 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/nvmem/bcm-ocotp.c b/drivers/nvmem/bcm-ocotp.c
index 3c56e3b..5e9e324 100644
--- a/drivers/nvmem/bcm-ocotp.c
+++ b/drivers/nvmem/bcm-ocotp.c
@@ -232,7 +232,6 @@ static struct nvmem_config bcm_otpc_nvmem_config = {
 	.read_only = false,
 	.word_size = 4,
 	.stride = 4,
-	.owner = THIS_MODULE,
 	.reg_read = bcm_otpc_read,
 	.reg_write = bcm_otpc_write,
 };
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 2ba5589..0d5e893 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -462,6 +462,8 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 
 	nvmem->id = rval;
 	nvmem->owner = config->owner;
+	if (!nvmem->owner && config->dev->driver)
+		nvmem->owner = config->dev->driver->owner;
 	nvmem->stride = config->stride;
 	nvmem->word_size = config->word_size;
 	nvmem->size = config->size;
diff --git a/drivers/nvmem/imx-iim.c b/drivers/nvmem/imx-iim.c
index a599260..52cfe91d 100644
--- a/drivers/nvmem/imx-iim.c
+++ b/drivers/nvmem/imx-iim.c
@@ -133,7 +133,6 @@ static int imx_iim_probe(struct platform_device *pdev)
 	cfg.read_only = true,
 	cfg.word_size = 1,
 	cfg.stride = 1,
-	cfg.owner = THIS_MODULE,
 	cfg.reg_read = imx_iim_read,
 	cfg.dev = dev;
 	cfg.size = drvdata->nregs;
diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c
index 193ca8f..e57e2a5 100644
--- a/drivers/nvmem/imx-ocotp.c
+++ b/drivers/nvmem/imx-ocotp.c
@@ -303,7 +303,6 @@ static struct nvmem_config imx_ocotp_nvmem_config = {
 	.read_only = false,
 	.word_size = 4,
 	.stride = 4,
-	.owner = THIS_MODULE,
 	.reg_read = imx_ocotp_read,
 	.reg_write = imx_ocotp_write,
 };
diff --git a/drivers/nvmem/lpc18xx_eeprom.c b/drivers/nvmem/lpc18xx_eeprom.c
index 6c7e2c4..b1af966 100644
--- a/drivers/nvmem/lpc18xx_eeprom.c
+++ b/drivers/nvmem/lpc18xx_eeprom.c
@@ -159,7 +159,6 @@ static struct nvmem_config lpc18xx_nvmem_config = {
 	.word_size = 4,
 	.reg_read = lpc18xx_eeprom_read,
 	.reg_write = lpc18xx_eeprom_gather_write,
-	.owner = THIS_MODULE,
 };
 
 static int lpc18xx_eeprom_probe(struct platform_device *pdev)
diff --git a/drivers/nvmem/lpc18xx_otp.c b/drivers/nvmem/lpc18xx_otp.c
index be8d074..95268db 100644
--- a/drivers/nvmem/lpc18xx_otp.c
+++ b/drivers/nvmem/lpc18xx_otp.c
@@ -64,7 +64,6 @@ static struct nvmem_config lpc18xx_otp_nvmem_config = {
 	.read_only = true,
 	.word_size = LPC18XX_OTP_WORD_SIZE,
 	.stride = LPC18XX_OTP_WORD_SIZE,
-	.owner = THIS_MODULE,
 	.reg_read = lpc18xx_otp_read,
 };
 
diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
index 70bfc98..dbedfce 100644
--- a/drivers/nvmem/meson-efuse.c
+++ b/drivers/nvmem/meson-efuse.c
@@ -37,7 +37,6 @@ static int meson_efuse_read(void *context, unsigned int offset,
 
 static struct nvmem_config econfig = {
 	.name = "meson-efuse",
-	.owner = THIS_MODULE,
 	.stride = 1,
 	.word_size = 1,
 	.read_only = true,
diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
index c4058b5..9ee3479 100644
--- a/drivers/nvmem/mtk-efuse.c
+++ b/drivers/nvmem/mtk-efuse.c
@@ -72,7 +72,6 @@ static int mtk_efuse_probe(struct platform_device *pdev)
 	econfig.size = resource_size(res);
 	econfig.priv = priv;
 	econfig.dev = dev;
-	econfig.owner = THIS_MODULE;
 	nvmem = nvmem_register(&econfig);
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
diff --git a/drivers/nvmem/mxs-ocotp.c b/drivers/nvmem/mxs-ocotp.c
index d26dd03..7018e2e 100644
--- a/drivers/nvmem/mxs-ocotp.c
+++ b/drivers/nvmem/mxs-ocotp.c
@@ -118,7 +118,6 @@ static struct nvmem_config ocotp_config = {
 	.name = "mxs-ocotp",
 	.stride = 16,
 	.word_size = 4,
-	.owner = THIS_MODULE,
 	.reg_read = mxs_ocotp_read,
 };
 
diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
index b96730e..cb3b48b 100644
--- a/drivers/nvmem/qfprom.c
+++ b/drivers/nvmem/qfprom.c
@@ -56,7 +56,6 @@ static int qfprom_remove(struct platform_device *pdev)
 
 static struct nvmem_config econfig = {
 	.name = "qfprom",
-	.owner = THIS_MODULE,
 	.stride = 1,
 	.word_size = 1,
 	.reg_read = qfprom_reg_read,
diff --git a/drivers/nvmem/rockchip-efuse.c b/drivers/nvmem/rockchip-efuse.c
index 63e3eb5..e292bbe 100644
--- a/drivers/nvmem/rockchip-efuse.c
+++ b/drivers/nvmem/rockchip-efuse.c
@@ -149,7 +149,6 @@ static int rockchip_rk3399_efuse_read(void *context, unsigned int offset,
 
 static struct nvmem_config econfig = {
 	.name = "rockchip-efuse",
-	.owner = THIS_MODULE,
 	.stride = 1,
 	.word_size = 1,
 	.read_only = true,
diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index 0d6648b..1c3b5cf 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -40,7 +40,6 @@ static struct nvmem_config econfig = {
 	.read_only = true,
 	.stride = 4,
 	.word_size = 1,
-	.owner = THIS_MODULE,
 };
 
 struct sunxi_sid_cfg {
diff --git a/drivers/nvmem/vf610-ocotp.c b/drivers/nvmem/vf610-ocotp.c
index 72e4faa..5ae9e00 100644
--- a/drivers/nvmem/vf610-ocotp.c
+++ b/drivers/nvmem/vf610-ocotp.c
@@ -206,7 +206,6 @@ static int vf610_ocotp_read(void *context, unsigned int offset,
 
 static struct nvmem_config ocotp_config = {
 	.name = "ocotp",
-	.owner = THIS_MODULE,
 	.stride = 4,
 	.word_size = 4,
 	.reg_read = vf610_ocotp_read,
-- 
2.7.4

Re: [PATCH 0/5] nvmem: some cleanups and sparse warning fixes

[Masahiro Yamada]

Hi Srinivas,


2017-09-11 22:00 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>
> Socionext is trying to add a new nvmem driver.
>
> Before adding a new one, I'd like to be sure about the preferred
> coding style in this subsystem, and build-up cleaner code base.
>
> Please pick up what you like.  Please feel free to throw away
> what you do not like.
>
> 1/5, 2/5: convert kzalloc to stack because nvmem_config
> is small one-time use data.
> This is what we agreed in the discussion.
>
> 3/5, 4/5: fix sparse warning
> For mtk-efuse, qfprom, the base address is the only private data
> but, assigning (void __iomem *) to ->priv directly causes sparse
> warnings.
>
> 5/5: clean-up of owner settings - if the owner field matches
> nvmem->dev->driver->onwer, we need not set the onwer field
> explicitly.


Does this series look OK?

Thanks.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/5] nvmem: mtk-efuse: fix different address space warnings of sparse

[Sean Wang]

On Mon, 2017-09-11 at 22:01 +0900, Masahiro Yamada wrote:
> Fix the following sparse warnings:
> 
> drivers/nvmem/mtk-efuse.c:24:30: warning: incorrect type in initializer (different address spaces)
> drivers/nvmem/mtk-efuse.c:24:30:    expected void [noderef] <asn:2>*base
> drivers/nvmem/mtk-efuse.c:24:30:    got void *context
> drivers/nvmem/mtk-efuse.c:37:30: warning: incorrect type in initializer (different address spaces)
> drivers/nvmem/mtk-efuse.c:37:30:    expected void [noderef] <asn:2>*base
> drivers/nvmem/mtk-efuse.c:37:30:    got void *context
> drivers/nvmem/mtk-efuse.c:69:23: warning: incorrect type in assignment (different address spaces)
> drivers/nvmem/mtk-efuse.c:69:23:    expected void *priv
> drivers/nvmem/mtk-efuse.c:69:23:    got void [noderef] <asn:2>*[assigned] base
> 
> The type of nvmem_config->priv is (void *), so sparse complains
> about assignment of the base address with (void __iomem *) type.
> 
> Even if we cast it out, sparse still warns:
> warning: cast removes address space of expression
> 
> Of course, we can shut up the sparse by marking __force, but a more
> correct way is to put the base address into driver private data.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Acked-by: Sean Wang <sean.wang@mediatek.com>

> ---
> 
>  drivers/nvmem/mtk-efuse.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> index fa7a0f6..c4058b5 100644
> --- a/drivers/nvmem/mtk-efuse.c
> +++ b/drivers/nvmem/mtk-efuse.c
> @@ -18,15 +18,19 @@
>  #include <linux/nvmem-provider.h>
>  #include <linux/platform_device.h>
>  
> +struct mtk_efuse_priv {
> +	void __iomem *base;
> +};
> +
>  static int mtk_reg_read(void *context,
>  			unsigned int reg, void *_val, size_t bytes)
>  {
> -	void __iomem *base = context;
> +	struct mtk_efuse_priv *priv = context;
>  	u32 *val = _val;
>  	int i = 0, words = bytes / 4;
>  
>  	while (words--)
> -		*val++ = readl(base + reg + (i++ * 4));
> +		*val++ = readl(priv->base + reg + (i++ * 4));
>  
>  	return 0;
>  }
> @@ -34,12 +38,12 @@ static int mtk_reg_read(void *context,
>  static int mtk_reg_write(void *context,
>  			 unsigned int reg, void *_val, size_t bytes)
>  {
> -	void __iomem *base = context;
> +	struct mtk_efuse_priv *priv = context;
>  	u32 *val = _val;
>  	int i = 0, words = bytes / 4;
>  
>  	while (words--)
> -		writel(*val++, base + reg + (i++ * 4));
> +		writel(*val++, priv->base + reg + (i++ * 4));
>  
>  	return 0;
>  }
> @@ -50,19 +54,23 @@ static int mtk_efuse_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct nvmem_device *nvmem;
>  	struct nvmem_config econfig = {};
> -	void __iomem *base;
> +	struct mtk_efuse_priv *priv;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> +	priv->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
>  
>  	econfig.stride = 4;
>  	econfig.word_size = 4;
>  	econfig.reg_read = mtk_reg_read;
>  	econfig.reg_write = mtk_reg_write;
>  	econfig.size = resource_size(res);
> -	econfig.priv = base;
> +	econfig.priv = priv;
>  	econfig.dev = dev;
>  	econfig.owner = THIS_MODULE;
>  	nvmem = nvmem_register(&econfig);

Re: [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config instead of malloc'ing it

[Sean Wang]

Hi, Masahiro

For maintainability, I felt it's better if we use the same way to
register nvmem as that most drivers does under nvmem usually using
static structure. Otherwise, they should also be changed to use the
one-time data in stack to avoid extra bytes to keep them.

	Sean


On Mon, 2017-09-11 at 22:01 +0900, Masahiro Yamada wrote:
> nvmem_register() copies all the members of nvmem_config to
> nvmem_device.  So, nvmem_config is one-time use data during
> probing.  There is no point to keep it until the driver detach.
> Using stack should be no problem because nvmem_config is pretty
> small.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/nvmem/mtk-efuse.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> index 32fd572..fa7a0f6 100644
> --- a/drivers/nvmem/mtk-efuse.c
> +++ b/drivers/nvmem/mtk-efuse.c
> @@ -49,7 +49,7 @@ static int mtk_efuse_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	struct nvmem_device *nvmem;
> -	struct nvmem_config *econfig;
> +	struct nvmem_config econfig = {};
>  	void __iomem *base;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -57,19 +57,15 @@ static int mtk_efuse_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> -	econfig = devm_kzalloc(dev, sizeof(*econfig), GFP_KERNEL);
> -	if (!econfig)
> -		return -ENOMEM;
> -
> -	econfig->stride = 4;
> -	econfig->word_size = 4;
> -	econfig->reg_read = mtk_reg_read;
> -	econfig->reg_write = mtk_reg_write;
> -	econfig->size = resource_size(res);
> -	econfig->priv = base;
> -	econfig->dev = dev;
> -	econfig->owner = THIS_MODULE;
> -	nvmem = nvmem_register(econfig);
> +	econfig.stride = 4;
> +	econfig.word_size = 4;
> +	econfig.reg_read = mtk_reg_read;
> +	econfig.reg_write = mtk_reg_write;
> +	econfig.size = resource_size(res);
> +	econfig.priv = base;
> +	econfig.dev = dev;
> +	econfig.owner = THIS_MODULE;
> +	nvmem = nvmem_register(&econfig);
>  	if (IS_ERR(nvmem))
>  		return PTR_ERR(nvmem);
>  

Re: [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

Hi Sean,


2017-09-21 1:32 GMT+09:00 Sean Wang <sean.wang@mediatek.com>:
> Hi, Masahiro
>
> For maintainability, I felt it's better if we use the same way to
> register nvmem as that most drivers does under nvmem usually using
> static structure. Otherwise, they should also be changed to use the
> one-time data in stack to avoid extra bytes to keep them.
>
>         Sean


Srinivas and I discussed the best practice for allocating nvmem_config.
https://lkml.org/lkml/2017/9/11/4

>From the discussion, static structure is possible only when
the system has one instance of the device.

If you know this is the case for mediatek,
yes, you can turn it into static,
but it is not always possible.
For example, Socionext SoCs have
two banks of efuse devices.

So, if we want to align the coding style for consistency,
nvmem in stack is safe and efficient, I think.


Moving one-time data into stack slightly
reduces the kernel image size.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config instead of malloc'ing it

[Sean Wang]

Hi, Masahiro

On Thu, 2017-09-21 at 11:09 +0900, Masahiro Yamada wrote:
> Hi Sean,
> 
> 
> 2017-09-21 1:32 GMT+09:00 Sean Wang <sean.wang@mediatek.com>:
> > Hi, Masahiro
> >
> > For maintainability, I felt it's better if we use the same way to
> > register nvmem as that most drivers does under nvmem usually using
> > static structure. Otherwise, they should also be changed to use the
> > one-time data in stack to avoid extra bytes to keep them.
> >
> >         Sean
> 
> 
> Srinivas and I discussed the best practice for allocating nvmem_config.
> https://lkml.org/lkml/2017/9/11/4
> 
> From the discussion, static structure is possible only when
> the system has one instance of the device.
> 

thank you for your detailed explanation

> If you know this is the case for mediatek,
> yes, you can turn it into static,
> but it is not always possible.
> For example, Socionext SoCs have
> two banks of efuse devices.
> 

there should be only one instance of nvmem for mediatek soc, currently


> So, if we want to align the coding style for consistency,
> nvmem in stack is safe and efficient, I think.
> 
> 
> Moving one-time data into stack slightly
> reduces the kernel image size.
> 
> 

agreed on those statements, indeed bigger struct uart_8250_port is still
used on stack for configuring in a lot 8250 driver, so 

Acked-by: Sean Wang <sean.wang@mediatek.com>