[PATCH 00/12] nvmem: patches set-1 for v4.15

[PATCH 00/12] nvmem: patches set-1 for v4.15

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Hi Greg,

Here are some nvmem patches which include few new provider drivers
(amlogic, uniphier), some trivial cleanups and few enhanements.

Can you consider these for v4.15.

I will send second set of patches soon, am waiting for Acks from
original authors.

Thanks,
srini


Icenowy Zheng (1):
  nvmem: sunxi-sid: add support for A64/H5's SID controller

Keiji Hayashibara (2):
  dt-bindings: nvmem: add description for UniPhier eFuse
  nvmem: uniphier: add UniPhier eFuse driver

Martin Blumenstingl (3):
  dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse
  nvmem: meson-efuse: indicate that this driver is only for Meson GX
    SoCs
  nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs

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

Romain Perier (1):
  nvmem: rockchip: add support for RK3368

 .../bindings/nvmem/allwinner,sunxi-sid.txt         |   1 +
 .../devicetree/bindings/nvmem/amlogic-efuse.txt    |   2 +-
 .../bindings/nvmem/amlogic-meson-mx-efuse.txt      |  22 ++
 .../devicetree/bindings/nvmem/rockchip-efuse.txt   |   1 +
 .../devicetree/bindings/nvmem/uniphier-efuse.txt   |  49 ++++
 drivers/nvmem/Kconfig                              |  25 +-
 drivers/nvmem/Makefile                             |   4 +
 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                        |   5 +-
 drivers/nvmem/meson-mx-efuse.c                     | 265 +++++++++++++++++++++
 drivers/nvmem/mtk-efuse.c                          |  47 ++--
 drivers/nvmem/mxs-ocotp.c                          |   1 -
 drivers/nvmem/qfprom.c                             |  27 ++-
 drivers/nvmem/rockchip-efuse.c                     |   5 +-
 drivers/nvmem/sunxi_sid.c                          |   7 +-
 drivers/nvmem/uniphier-efuse.c                     |  97 ++++++++
 drivers/nvmem/vf610-ocotp.c                        |   1 -
 22 files changed, 529 insertions(+), 60 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt
 create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
 create mode 100644 drivers/nvmem/meson-mx-efuse.c
 create mode 100644 drivers/nvmem/uniphier-efuse.c

-- 
2.11.0

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

[srinivas.kandagatla]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

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>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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 52ff65e0673f..a5992602709a 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.11.0

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 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>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  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 52ff65e0673f..a5992602709a 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 = {};

You do realize you are now not zeroing out this structure, and have to
explicitly initialize all of the fields, right?

What is the real problem with doing a dynamic allocation for this?
Putting structures on the stack is a "bad idea" for all of the obvious
reasons (small stack in the kernel, initialized data, lower layers
expect it to be dma-able, etc.)

thanks,

greg k-h

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

Hi Greg,

2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
>> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>>
>> 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>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>  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 52ff65e0673f..a5992602709a 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 = {};
>
> You do realize you are now not zeroing out this structure, and have to
> explicitly initialize all of the fields, right?

Why?

I am surely zeroing out the structure.

Did you miss "= {};" in my code?



> What is the real problem with doing a dynamic allocation for this?
> Putting structures on the stack is a "bad idea" for all of the obvious
> reasons (small stack in the kernel, initialized data, lower layers
> expect it to be dma-able, etc.)


Why is this a problem?

Did you really understand this patch?

 - This structure is very small.
   struct uart_8250_port is five times bigger
   and it is allocated in the stack and it is fine.

 - All data are initialized.

 - Why DMA?
   Please do not exaggerate things by introducing unrelated topic.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Greg KH]

On Fri, Oct 20, 2017 at 10:47:16PM +0900, Masahiro Yamada wrote:
> Hi Greg,
> 
> 2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> > On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>
> >> 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>
> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >> ---
> >>  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 52ff65e0673f..a5992602709a 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 = {};
> >
> > You do realize you are now not zeroing out this structure, and have to
> > explicitly initialize all of the fields, right?
> 
> Why?
> 
> I am surely zeroing out the structure.
> 
> Did you miss "= {};" in my code?

Are you sure that does zero it out?  I know we have had issues with this
in the past...

> > What is the real problem with doing a dynamic allocation for this?
> > Putting structures on the stack is a "bad idea" for all of the obvious
> > reasons (small stack in the kernel, initialized data, lower layers
> > expect it to be dma-able, etc.)
> 
> 
> Why is this a problem?
> 
> Did you really understand this patch?
> 
>  - This structure is very small.
>    struct uart_8250_port is five times bigger
>    and it is allocated in the stack and it is fine.
> 
>  - All data are initialized.
> 
>  - Why DMA?
>    Please do not exaggerate things by introducing unrelated topic.

I just want you to realize the change, the initialized is the big thing.

And keeping structures off of the stack is a good thing, if this is not
a performance issue, I suggest keeping it as-is, right?

thanks,

greg k-h

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Fri, Oct 20, 2017 at 10:47:16PM +0900, Masahiro Yamada wrote:
>> Hi Greg,
>>
>> 2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> > On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
>> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>> >>
>> >> 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>
>> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> >> ---
>> >>  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 52ff65e0673f..a5992602709a 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 = {};
>> >
>> > You do realize you are now not zeroing out this structure, and have to
>> > explicitly initialize all of the fields, right?
>>
>> Why?
>>
>> I am surely zeroing out the structure.
>>
>> Did you miss "= {};" in my code?
>
> Are you sure that does zero it out?  I know we have had issues with this
> in the past...

Do you have a reference for that?

All members that are not specified in the initializer
are set to 0 (or NULL).

"git show c7836d1593b87cb813c58cf64e08b052ebbe2a78"
and do you agree that this is correct?


>> > What is the real problem with doing a dynamic allocation for this?
>> > Putting structures on the stack is a "bad idea" for all of the obvious
>> > reasons (small stack in the kernel, initialized data, lower layers
>> > expect it to be dma-able, etc.)
>>
>>
>> Why is this a problem?
>>
>> Did you really understand this patch?
>>
>>  - This structure is very small.
>>    struct uart_8250_port is five times bigger
>>    and it is allocated in the stack and it is fine.
>>
>>  - All data are initialized.
>>
>>  - Why DMA?
>>    Please do not exaggerate things by introducing unrelated topic.
>
> I just want you to realize the change, the initialized is the big thing.
>
> And keeping structures off of the stack is a good thing, if this is not
> a performance issue, I suggest keeping it as-is, right?
>

I do not see logical explanation in your comment.

The structure is initialized.
Other subsystem use stack for such a small structure.
Why is (devm_)kzalloc necessary?




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Greg KH]

On Fri, Oct 20, 2017 at 11:26:16PM +0900, Masahiro Yamada wrote:
> 2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> > On Fri, Oct 20, 2017 at 10:47:16PM +0900, Masahiro Yamada wrote:
> >> Hi Greg,
> >>
> >> 2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> >> > On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
> >> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> >>
> >> >> 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>
> >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >> >> ---
> >> >>  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 52ff65e0673f..a5992602709a 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 = {};
> >> >
> >> > You do realize you are now not zeroing out this structure, and have to
> >> > explicitly initialize all of the fields, right?
> >>
> >> Why?
> >>
> >> I am surely zeroing out the structure.
> >>
> >> Did you miss "= {};" in my code?
> >
> > Are you sure that does zero it out?  I know we have had issues with this
> > in the past...
> 
> Do you have a reference for that?
> 
> All members that are not specified in the initializer
> are set to 0 (or NULL).
> 
> "git show c7836d1593b87cb813c58cf64e08b052ebbe2a78"
> and do you agree that this is correct?

Ugh, you are right, that's what I get for reviewing 250+ patches at a
time, my fault, sorry for the noise.

Can you resend these?

thanks,

greg k-h

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

2017-10-20 23:47 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Fri, Oct 20, 2017 at 11:26:16PM +0900, Masahiro Yamada wrote:
>> 2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> > On Fri, Oct 20, 2017 at 10:47:16PM +0900, Masahiro Yamada wrote:
>> >> Hi Greg,
>> >>
>> >> 2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> >> > On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
>> >> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>> >> >>
>> >> >> 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>
>> >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> >> >> ---
>> >> >>  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 52ff65e0673f..a5992602709a 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 = {};
>> >> >
>> >> > You do realize you are now not zeroing out this structure, and have to
>> >> > explicitly initialize all of the fields, right?
>> >>
>> >> Why?
>> >>
>> >> I am surely zeroing out the structure.
>> >>
>> >> Did you miss "= {};" in my code?
>> >
>> > Are you sure that does zero it out?  I know we have had issues with this
>> > in the past...
>>
>> Do you have a reference for that?
>>
>> All members that are not specified in the initializer
>> are set to 0 (or NULL).
>>
>> "git show c7836d1593b87cb813c58cf64e08b052ebbe2a78"
>> and do you agree that this is correct?
>
> Ugh, you are right, that's what I get for reviewing 250+ patches at a
> time, my fault, sorry for the noise.
>
> Can you resend these?
>
> thanks,
>
> greg k-h


For what?

Srinivas said he based the series on char-misc,
and this is the first patch of the series.

It should be applied cleanly to char-misc.
(and 05/12 too, if you obey the patch order)


If you are in trouble, please let me know
the base commit it should be rebased on.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Greg KH]

On Sat, Oct 21, 2017 at 01:18:14AM +0900, Masahiro Yamada wrote:
> 2017-10-20 23:47 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> > On Fri, Oct 20, 2017 at 11:26:16PM +0900, Masahiro Yamada wrote:
> >> 2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> >> > On Fri, Oct 20, 2017 at 10:47:16PM +0900, Masahiro Yamada wrote:
> >> >> Hi Greg,
> >> >>
> >> >> 2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> >> >> > On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
> >> >> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> >> >>
> >> >> >> 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>
> >> >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >> >> >> ---
> >> >> >>  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 52ff65e0673f..a5992602709a 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 = {};
> >> >> >
> >> >> > You do realize you are now not zeroing out this structure, and have to
> >> >> > explicitly initialize all of the fields, right?
> >> >>
> >> >> Why?
> >> >>
> >> >> I am surely zeroing out the structure.
> >> >>
> >> >> Did you miss "= {};" in my code?
> >> >
> >> > Are you sure that does zero it out?  I know we have had issues with this
> >> > in the past...
> >>
> >> Do you have a reference for that?
> >>
> >> All members that are not specified in the initializer
> >> are set to 0 (or NULL).
> >>
> >> "git show c7836d1593b87cb813c58cf64e08b052ebbe2a78"
> >> and do you agree that this is correct?
> >
> > Ugh, you are right, that's what I get for reviewing 250+ patches at a
> > time, my fault, sorry for the noise.
> >
> > Can you resend these?
> >
> > thanks,
> >
> > greg k-h
> 
> 
> For what?
> 
> Srinivas said he based the series on char-misc,
> and this is the first patch of the series.
> 
> It should be applied cleanly to char-misc.
> (and 05/12 too, if you obey the patch order)
> 
> 
> If you are in trouble, please let me know
> the base commit it should be rebased on.

Please resend the patches that I didn't apply, rebased against the
char-misc-testing branch of the char-misc.git tree.

thanks,

greg k-h

Re: [PATCH 01/12] nvmem: imx-iim: use stack for nvmem_config instead of malloc'ing it

[Masahiro Yamada]

Hi Greg,


2017-10-21 1:26 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Sat, Oct 21, 2017 at 01:18:14AM +0900, Masahiro Yamada wrote:
>> 2017-10-20 23:47 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> > On Fri, Oct 20, 2017 at 11:26:16PM +0900, Masahiro Yamada wrote:
>> >> 2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> >> > On Fri, Oct 20, 2017 at 10:47:16PM +0900, Masahiro Yamada wrote:
>> >> >> Hi Greg,
>> >> >>
>> >> >> 2017-10-20 22:32 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> >> >> > On Mon, Oct 09, 2017 at 03:26:30PM +0200, srinivas.kandagatla@linaro.org wrote:
>> >> >> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>> >> >> >>
>> >> >> >> 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>
>> >> >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> >> >> >> ---
>> >> >> >>  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 52ff65e0673f..a5992602709a 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 = {};
>> >> >> >
>> >> >> > You do realize you are now not zeroing out this structure, and have to
>> >> >> > explicitly initialize all of the fields, right?
>> >> >>
>> >> >> Why?
>> >> >>
>> >> >> I am surely zeroing out the structure.
>> >> >>
>> >> >> Did you miss "= {};" in my code?
>> >> >
>> >> > Are you sure that does zero it out?  I know we have had issues with this
>> >> > in the past...
>> >>
>> >> Do you have a reference for that?
>> >>
>> >> All members that are not specified in the initializer
>> >> are set to 0 (or NULL).
>> >>
>> >> "git show c7836d1593b87cb813c58cf64e08b052ebbe2a78"
>> >> and do you agree that this is correct?
>> >
>> > Ugh, you are right, that's what I get for reviewing 250+ patches at a
>> > time, my fault, sorry for the noise.
>> >
>> > Can you resend these?
>> >
>> > thanks,
>> >
>> > greg k-h
>>
>>
>> For what?
>>
>> Srinivas said he based the series on char-misc,
>> and this is the first patch of the series.
>>
>> It should be applied cleanly to char-misc.
>> (and 05/12 too, if you obey the patch order)
>>
>>
>> If you are in trouble, please let me know
>> the base commit it should be rebased on.
>
> Please resend the patches that I didn't apply, rebased against the
> char-misc-testing branch of the char-misc.git tree.
>
> thanks,
>
> greg k-h


I rebased and resent my patches as requested.

If there is something wrong, please let me know.

Thanks!



-- 
Best Regards
Masahiro Yamada

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

[srinivas.kandagatla]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

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>
Acked-by: Sean Wang <sean.wang@mediatek.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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 32fd572e18c5..fa7a0f66b37e 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.11.0

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

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:31PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 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.

Same objection as previous patch, what is wrong with it as-is?

thanks,

greg k-h

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

[Masahiro Yamada]

2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Mon, Oct 09, 2017 at 03:26:31PM +0200, srinivas.kandagatla@linaro.org wrote:
>> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>>
>> 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.
>
> Same objection as previous patch, what is wrong with it as-is?
>

It is wasting memory.


-- 
Best Regards
Masahiro Yamada

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

[Greg KH]

On Fri, Oct 20, 2017 at 10:47:28PM +0900, Masahiro Yamada wrote:
> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> > On Mon, Oct 09, 2017 at 03:26:31PM +0200, srinivas.kandagatla@linaro.org wrote:
> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>
> >> 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.
> >
> > Same objection as previous patch, what is wrong with it as-is?
> >
> 
> It is wasting memory.

The memory is freed again, after the call, right?

thanks,

greg k-h

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

[Masahiro Yamada]

2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Fri, Oct 20, 2017 at 10:47:28PM +0900, Masahiro Yamada wrote:
>> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> > On Mon, Oct 09, 2017 at 03:26:31PM +0200, srinivas.kandagatla@linaro.org wrote:
>> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>> >>
>> >> 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.
>> >
>> > Same objection as previous patch, what is wrong with it as-is?
>> >
>>
>> It is wasting memory.
>
> The memory is freed again, after the call, right?
>

I assume your "as-is" meant devm_kzalloc().

The memory is freed when the driver is detached.

It is generally quite long time
that this struct occupies the memory for no good reason.



-- 
Best Regards
Masahiro Yamada

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

[Greg KH]

On Fri, Oct 20, 2017 at 10:59:45PM +0900, Masahiro Yamada wrote:
> 2017-10-20 22:54 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> > On Fri, Oct 20, 2017 at 10:47:28PM +0900, Masahiro Yamada wrote:
> >> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> >> > On Mon, Oct 09, 2017 at 03:26:31PM +0200, srinivas.kandagatla@linaro.org wrote:
> >> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> >>
> >> >> 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.
> >> >
> >> > Same objection as previous patch, what is wrong with it as-is?
> >> >
> >>
> >> It is wasting memory.
> >
> > The memory is freed again, after the call, right?
> >
> 
> I assume your "as-is" meant devm_kzalloc().
> 
> The memory is freed when the driver is detached.
> 
> It is generally quite long time
> that this struct occupies the memory for no good reason.

Ah, ok, a simple kmalloc() might be better, but ok, if you are _sure_
it's ok taking that much stack space up... :)

thanks,

greg k-h

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

[srinivas.kandagatla]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

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>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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 fa7a0f66b37e..c4058b598703 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.11.0

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

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 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.

Ick, no, really?

That's horrid.  Do the __force cast as that is what you are really doing
here, and it is good to see the bad use of it (hint, perhaps the api
needs to be fixed up so you do not have to do that???)



> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  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 fa7a0f66b37e..c4058b598703 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,


Make the read call take a __iomem *, not a random void *.  That should
solve this issue, right?

thanks,

greg k-h

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

[Masahiro Yamada]

2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:
>> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>>
>> 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.
>
> Ick, no, really?
>
> That's horrid.  Do the __force cast as that is what you are really doing
> here, and it is good to see the bad use of it (hint, perhaps the api
> needs to be fixed up so you do not have to do that???)


Other drivers take private data from the "void *context".

In this driver, the priv just happens to have one member.
What's the matter?




>> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
>> index fa7a0f66b37e..c4058b598703 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,
>
>
> Make the read call take a __iomem *, not a random void *.  That should
> solve this issue, right?
>

NACK.

See drivers/nvmem/imx-iim.c

The ->reg_read hook needs more private data than __iomem *.



-- 
Best Regards
Masahiro Yamada

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

[Greg KH]

On Fri, Oct 20, 2017 at 10:55:04PM +0900, Masahiro Yamada wrote:
> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> > On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:
> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>
> >> 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.
> >
> > Ick, no, really?
> >
> > That's horrid.  Do the __force cast as that is what you are really doing
> > here, and it is good to see the bad use of it (hint, perhaps the api
> > needs to be fixed up so you do not have to do that???)
> 
> 
> Other drivers take private data from the "void *context".
> 
> In this driver, the priv just happens to have one member.
> What's the matter?
> 
> 
> 
> 
> >> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> >> index fa7a0f66b37e..c4058b598703 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,
> >
> >
> > Make the read call take a __iomem *, not a random void *.  That should
> > solve this issue, right?
> >
> 
> NACK.
> 
> See drivers/nvmem/imx-iim.c
> 
> The ->reg_read hook needs more private data than __iomem *.

Ok, that makes a bit more sense, but it still feels a bit of a hack,
don't you agree?

thanks,

greg k-h

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

[Masahiro Yamada]

2017-10-20 23:49 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
> On Fri, Oct 20, 2017 at 10:55:04PM +0900, Masahiro Yamada wrote:
>> 2017-10-20 22:34 GMT+09:00 Greg KH <gregkh@linuxfoundation.org>:
>> > On Mon, Oct 09, 2017 at 03:26:32PM +0200, srinivas.kandagatla@linaro.org wrote:
>> >> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>> >>
>> >> 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.
>> >
>> > Ick, no, really?
>> >
>> > That's horrid.  Do the __force cast as that is what you are really doing
>> > here, and it is good to see the bad use of it (hint, perhaps the api
>> > needs to be fixed up so you do not have to do that???)
>>
>>
>> Other drivers take private data from the "void *context".
>>
>> In this driver, the priv just happens to have one member.
>> What's the matter?
>>
>>
>>
>>
>> >> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
>> >> index fa7a0f66b37e..c4058b598703 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,
>> >
>> >
>> > Make the read call take a __iomem *, not a random void *.  That should
>> > solve this issue, right?
>> >
>>
>> NACK.
>>
>> See drivers/nvmem/imx-iim.c
>>
>> The ->reg_read hook needs more private data than __iomem *.
>
> Ok, that makes a bit more sense, but it still feels a bit of a hack,
> don't you agree?


But, I have no choice but __force.

I disagree with using __force to hide things.

In my opinion, __force should be used only where it is really legitimate.

This driver is not the case, as we can fix it
by writing correct code.


-- 
Best Regards
Masahiro Yamada

[PATCH 04/12] nvmem: qfprom: fix different address space warnings of sparse

[srinivas.kandagatla]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

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>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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 2bdb6c389328..b96730e99580 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.11.0

Re: [PATCH 04/12] nvmem: qfprom: fix different address space warnings of sparse

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:33PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 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.

Same objection here.  Don't hide things like this.

greg k-h

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

[srinivas.kandagatla]

From: Masahiro Yamada <yamada.masahiro@socionext.com>

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>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 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 3c56e3b2bd65..5e9e324427f9 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 0a3464f647a4..e5f5df783898 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 a5992602709a..52cfe91d9762 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 193ca8fd350a..e57e2a57aa3f 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 6c7e2c424a4e..b1af966206a6 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 be8d07403ffc..95268db155e9 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 70bfc9839bb2..dbedfcea4e05 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 c4058b598703..9ee3479cfc7b 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 d26dd03cec80..7018e2ef5714 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 b96730e99580..cb3b48b47d64 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 63e3eb55f3ac..e292bbe6bb68 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 0d6648be93b8..1c3b5cf89212 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 72e4faabce29..5ae9e002f195 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.11.0

Re: [PATCH 05/12] nvmem: set nvmem->owner to nvmem->dev->driver->owner if unset

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:34PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> 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>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Patch does not apply :(

Re: [PATCH 05/12] nvmem: set nvmem->owner to nvmem->dev->driver->owner if unset

[Srinivas Kandagatla]

On 20/10/17 14:38, Greg KH wrote:
> On Mon, Oct 09, 2017 at 03:26:34PM +0200, srinivas.kandagatla@linaro.org wrote:
>> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>>
>> 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>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> Patch does not apply :(
> 
Hmm.. I did base it on char-misc last time when I sent it..

Will rebase it and resend it!

thanks,
srini

Re: [PATCH 05/12] nvmem: set nvmem->owner to nvmem->dev->driver->owner if unset

[Greg KH]

On Fri, Oct 20, 2017 at 02:44:51PM +0100, Srinivas Kandagatla wrote:
> 
> 
> On 20/10/17 14:38, Greg KH wrote:
> > On Mon, Oct 09, 2017 at 03:26:34PM +0200, srinivas.kandagatla@linaro.org wrote:
> > > From: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > 
> > > 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>
> > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > 
> > Patch does not apply :(
> > 
> Hmm.. I did base it on char-misc last time when I sent it..
> 
> Will rebase it and resend it!

Probably because I didn't take the previous patches in this series...

[PATCH 06/12] nvmem: sunxi-sid: add support for A64/H5's SID controller

[srinivas.kandagatla]

From: Icenowy Zheng <icenowy@aosc.io>

Allwinner A64/H5 SoCs come with a SID controller like the one in H3, but
without the silicon bug that makes the initial value at 0x200 wrong, so
the value at 0x200 can be directly read.

Add support for this kind of SID controller.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt | 1 +
 drivers/nvmem/sunxi_sid.c                                       | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
index ef06d061913c..6ea0836939ee 100644
--- a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
+++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
@@ -5,6 +5,7 @@ Required properties:
   "allwinner,sun4i-a10-sid"
   "allwinner,sun7i-a20-sid"
   "allwinner,sun8i-h3-sid"
+  "allwinner,sun50i-a64-sid"
 
 - reg: Should contain registers location and length
 
diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index 1c3b5cf89212..99bd54d85fcb 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -198,10 +198,16 @@ static const struct sunxi_sid_cfg sun8i_h3_cfg = {
 	.need_register_readout = true,
 };
 
+static const struct sunxi_sid_cfg sun50i_a64_cfg = {
+	.value_offset = 0x200,
+	.size = 0x100,
+};
+
 static const struct of_device_id sunxi_sid_of_match[] = {
 	{ .compatible = "allwinner,sun4i-a10-sid", .data = &sun4i_a10_cfg },
 	{ .compatible = "allwinner,sun7i-a20-sid", .data = &sun7i_a20_cfg },
 	{ .compatible = "allwinner,sun8i-h3-sid", .data = &sun8i_h3_cfg },
+	{ .compatible = "allwinner,sun50i-a64-sid", .data = &sun50i_a64_cfg },
 	{/* sentinel */},
 };
 MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
-- 
2.11.0

Re: [PATCH 06/12] nvmem: sunxi-sid: add support for A64/H5's SID controller

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:35PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Icenowy Zheng <icenowy@aosc.io>
> 
> Allwinner A64/H5 SoCs come with a SID controller like the one in H3, but
> without the silicon bug that makes the initial value at 0x200 wrong, so
> the value at 0x200 can be directly read.
> 
> Add support for this kind of SID controller.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt | 1 +
>  drivers/nvmem/sunxi_sid.c                                       | 6 ++++++
>  2 files changed, 7 insertions(+)

I need a DT maintainer's ack for this, right?

thanks,

greg k-h

[PATCH 07/12] dt-bindings: nvmem: add description for UniPhier eFuse

[srinivas.kandagatla]

From: Keiji Hayashibara <hayashibara.keiji@socionext.com>

Add uniphier-efuse dt-bindings documentation.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

diff --git a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
new file mode 100644
index 000000000000..1a394e5f71dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
@@ -0,0 +1,49 @@
+= UniPhier eFuse device tree bindings =
+
+This UniPhier eFuse must be under soc-glue.
+
+Required properties:
+- compatible: should be "socionext,uniphier-efuse"
+- reg: should contain the register location and length
+
+= Data cells =
+Are child nodes of efuse, bindings of which as described in
+bindings/nvmem/nvmem.txt
+
+Example:
+
+	soc-glue@5f900000 {
+		compatible = "socionext,uniphier-ld20-soc-glue-debug",
+			     "simple-mfd";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x5f900000 0x2000>;
+
+		efuse@100 {
+			compatible = "socionext,uniphier-efuse";
+			reg = <0x100 0x28>;
+		};
+
+		efuse@200 {
+			compatible = "socionext,uniphier-efuse";
+			reg = <0x200 0x68>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* Data cells */
+			usb_mon: usb_mon {
+				reg = <0x54 0xc>;
+			};
+		};
+	};
+
+= Data consumers =
+Are device nodes which consume nvmem data cells.
+
+Example:
+
+	usb {
+		...
+		nvmem-cells = <&usb_mon>;
+		nvmem-cell-names = "usb_mon";
+	}
-- 
2.11.0

Re: [PATCH 07/12] dt-bindings: nvmem: add description for UniPhier eFuse

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:36PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> 
> Add uniphier-efuse dt-bindings documentation.
> 
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

Same here, I need a DT maintainer's ack...

[PATCH 08/12] nvmem: uniphier: add UniPhier eFuse driver

[srinivas.kandagatla]

From: Keiji Hayashibara <hayashibara.keiji@socionext.com>

Add eFuse driver for Socionext UniPhier series SoC.
Note that eFuse device is under soc-glue and this register
implements as read only.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/nvmem/Kconfig          | 11 +++++
 drivers/nvmem/Makefile         |  2 +
 drivers/nvmem/uniphier-efuse.c | 97 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 drivers/nvmem/uniphier-efuse.c

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index ea3044c5d6ee..2755df8f6e9b 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -123,6 +123,17 @@ config NVMEM_SUNXI_SID
 	  This driver can also be built as a module. If so, the module
 	  will be called nvmem_sunxi_sid.
 
+config UNIPHIER_EFUSE
+	tristate "UniPhier SoCs eFuse support"
+	depends on ARCH_UNIPHIER || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  This is a simple driver to dump specified values of UniPhier SoC
+	  from eFuse.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called nvmem-uniphier-efuse.
+
 config NVMEM_VF610_OCOTP
 	tristate "VF610 SoC OCOTP support"
 	depends on SOC_VF610 || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 4c589184acee..11825353f729 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_ROCKCHIP_EFUSE)	+= nvmem_rockchip_efuse.o
 nvmem_rockchip_efuse-y		:= rockchip-efuse.o
 obj-$(CONFIG_NVMEM_SUNXI_SID)	+= nvmem_sunxi_sid.o
 nvmem_sunxi_sid-y		:= sunxi_sid.o
+obj-$(CONFIG_UNIPHIER_EFUSE)	+= nvmem-uniphier-efuse.o
+nvmem-uniphier-efuse-y		:= uniphier-efuse.o
 obj-$(CONFIG_NVMEM_VF610_OCOTP)	+= nvmem-vf610-ocotp.o
 nvmem-vf610-ocotp-y		:= vf610-ocotp.o
 obj-$(CONFIG_MESON_EFUSE)	+= nvmem_meson_efuse.o
diff --git a/drivers/nvmem/uniphier-efuse.c b/drivers/nvmem/uniphier-efuse.c
new file mode 100644
index 000000000000..2bb45c42fd6b
--- /dev/null
+++ b/drivers/nvmem/uniphier-efuse.c
@@ -0,0 +1,97 @@
+/*
+ * UniPhier eFuse driver
+ *
+ * Copyright (C) 2017 Socionext Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/platform_device.h>
+
+struct uniphier_efuse_priv {
+	void __iomem *base;
+};
+
+static int uniphier_reg_read(void *context,
+			     unsigned int reg, void *_val, size_t bytes)
+{
+	struct uniphier_efuse_priv *priv = context;
+	u32 *val = _val;
+	int offs;
+
+	for (offs = 0; offs < bytes; offs += sizeof(u32))
+		*val++ = readl(priv->base + reg + offs);
+
+	return 0;
+}
+
+static int uniphier_efuse_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct nvmem_device *nvmem;
+	struct nvmem_config econfig ={};
+	struct uniphier_efuse_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	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.read_only = true;
+	econfig.reg_read = uniphier_reg_read;
+	econfig.size = resource_size(res);
+	econfig.priv = priv;
+	econfig.dev = dev;
+	nvmem = nvmem_register(&econfig);
+	if (IS_ERR(nvmem))
+		return PTR_ERR(nvmem);
+
+	platform_set_drvdata(pdev, nvmem);
+
+	return 0;
+}
+
+static int uniphier_efuse_remove(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+	return nvmem_unregister(nvmem);
+}
+
+static const struct of_device_id uniphier_efuse_of_match[] = {
+	{ .compatible = "socionext,uniphier-efuse",},
+	{/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, uniphier_efuse_of_match);
+
+static struct platform_driver uniphier_efuse_driver = {
+	.probe = uniphier_efuse_probe,
+	.remove = uniphier_efuse_remove,
+	.driver = {
+		.name = "uniphier-efuse",
+		.of_match_table = uniphier_efuse_of_match,
+	},
+};
+module_platform_driver(uniphier_efuse_driver);
+
+MODULE_AUTHOR("Keiji Hayashibara <hayashibara.keiji@socionext.com>");
+MODULE_DESCRIPTION("UniPhier eFuse driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

Re: [PATCH 08/12] nvmem: uniphier: add UniPhier eFuse driver

[Greg KH]

On Mon, Oct 09, 2017 at 03:26:37PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> 
> Add eFuse driver for Socionext UniPhier series SoC.
> Note that eFuse device is under soc-glue and this register
> implements as read only.
> 
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/nvmem/Kconfig          | 11 +++++
>  drivers/nvmem/Makefile         |  2 +
>  drivers/nvmem/uniphier-efuse.c | 97 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 110 insertions(+)
>  create mode 100644 drivers/nvmem/uniphier-efuse.c
> 
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index ea3044c5d6ee..2755df8f6e9b 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -123,6 +123,17 @@ config NVMEM_SUNXI_SID
>  	  This driver can also be built as a module. If so, the module
>  	  will be called nvmem_sunxi_sid.
>  
> +config UNIPHIER_EFUSE
> +	tristate "UniPhier SoCs eFuse support"
> +	depends on ARCH_UNIPHIER || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	help
> +	  This is a simple driver to dump specified values of UniPhier SoC
> +	  from eFuse.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called nvmem-uniphier-efuse.
> +
>  config NVMEM_VF610_OCOTP
>  	tristate "VF610 SoC OCOTP support"
>  	depends on SOC_VF610 || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 4c589184acee..11825353f729 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_ROCKCHIP_EFUSE)	+= nvmem_rockchip_efuse.o
>  nvmem_rockchip_efuse-y		:= rockchip-efuse.o
>  obj-$(CONFIG_NVMEM_SUNXI_SID)	+= nvmem_sunxi_sid.o
>  nvmem_sunxi_sid-y		:= sunxi_sid.o
> +obj-$(CONFIG_UNIPHIER_EFUSE)	+= nvmem-uniphier-efuse.o
> +nvmem-uniphier-efuse-y		:= uniphier-efuse.o
>  obj-$(CONFIG_NVMEM_VF610_OCOTP)	+= nvmem-vf610-ocotp.o
>  nvmem-vf610-ocotp-y		:= vf610-ocotp.o
>  obj-$(CONFIG_MESON_EFUSE)	+= nvmem_meson_efuse.o
> diff --git a/drivers/nvmem/uniphier-efuse.c b/drivers/nvmem/uniphier-efuse.c
> new file mode 100644
> index 000000000000..2bb45c42fd6b
> --- /dev/null
> +++ b/drivers/nvmem/uniphier-efuse.c
> @@ -0,0 +1,97 @@
> +/*
> + * UniPhier eFuse driver
> + *
> + * Copyright (C) 2017 Socionext Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/platform_device.h>
> +
> +struct uniphier_efuse_priv {
> +	void __iomem *base;
> +};

Don't do this private mess here :(

[PATCH 09/12] nvmem: rockchip: add support for RK3368

[srinivas.kandagatla]

From: Romain Perier <romain.perier@collabora.com>

This adds the necessary function for handling support on RK3368 SoCs

Signed-off-by: Romain Perier <romain.perier@collabora.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 1 +
 drivers/nvmem/rockchip-efuse.c                             | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt b/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
index 1ff02afdc55a..60bec4782806 100644
--- a/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
@@ -6,6 +6,7 @@ Required properties:
   - "rockchip,rk3188-efuse" - for RK3188 SoCs.
   - "rockchip,rk3228-efuse" - for RK3228 SoCs.
   - "rockchip,rk3288-efuse" - for RK3288 SoCs.
+  - "rockchip,rk3368-efuse" - for RK3368 SoCs.
   - "rockchip,rk3399-efuse" - for RK3399 SoCs.
 - reg: Should contain the registers location and exact eFuse size
 - clocks: Should be the clock id of eFuse
diff --git a/drivers/nvmem/rockchip-efuse.c b/drivers/nvmem/rockchip-efuse.c
index e292bbe6bb68..123de77ca5d6 100644
--- a/drivers/nvmem/rockchip-efuse.c
+++ b/drivers/nvmem/rockchip-efuse.c
@@ -177,6 +177,10 @@ static const struct of_device_id rockchip_efuse_match[] = {
 		.data = (void *)&rockchip_rk3288_efuse_read,
 	},
 	{
+		.compatible = "rockchip,rk3368-efuse",
+		.data = (void *)&rockchip_rk3288_efuse_read,
+	},
+	{
 		.compatible = "rockchip,rk3399-efuse",
 		.data = (void *)&rockchip_rk3399_efuse_read,
 	},
-- 
2.11.0

[PATCH 10/12] dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse

[srinivas.kandagatla]

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Amlogic Meson6, Meson8 and Meson8b SoCs have an efuse which contains
calibration data from the factory (for the internal temperature sensor
and some CVBS connector settings). Some manufacturers also store the MAC
address or serial number in the efuse.

This documents the devicetree bindings for the efuse on these SoCs.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../bindings/nvmem/amlogic-meson-mx-efuse.txt      | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt

diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt
new file mode 100644
index 000000000000..a3c63954a1a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt
@@ -0,0 +1,22 @@
+Amlogic Meson6/Meson8/Meson8b efuse
+
+Required Properties:
+- compatible: depending on the SoC this should be one of:
+	- "amlogic,meson6-efuse"
+	- "amlogic,meson8-efuse"
+	- "amlogic,meson8b-efuse"
+- reg: base address and size of the efuse registers
+- clocks: a reference to the efuse core gate clock
+- clock-names: must be "core"
+
+All properties and sub-nodes as well as the consumer bindings
+defined in nvmem.txt in this directory are also supported.
+
+
+Example:
+	efuse: nvmem@0 {
+		compatible = "amlogic,meson8-efuse";
+		reg = <0x0 0x2000>;
+		clocks = <&clkc CLKID_EFUSE>;
+		clock-names = "core";
+	};
-- 
2.11.0

Re: [PATCH 10/12] dt-bindings: nvmem: Describe the Amlogic Meson6/Meson8/Meson8b efuse

[Martin Blumenstingl]

On Mon, Oct 9, 2017 at 3:26 PM,  <srinivas.kandagatla@linaro.org> wrote:
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> Amlogic Meson6, Meson8 and Meson8b SoCs have an efuse which contains
> calibration data from the factory (for the internal temperature sensor
> and some CVBS connector settings). Some manufacturers also store the MAC
> address or serial number in the efuse.
>
> This documents the devicetree bindings for the efuse on these SoCs.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
I also got Rob's ACK for this patch, see [0]

> ---
>  .../bindings/nvmem/amlogic-meson-mx-efuse.txt      | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt
>
> diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt
> new file mode 100644
> index 000000000000..a3c63954a1a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/amlogic-meson-mx-efuse.txt
> @@ -0,0 +1,22 @@
> +Amlogic Meson6/Meson8/Meson8b efuse
> +
> +Required Properties:
> +- compatible: depending on the SoC this should be one of:
> +       - "amlogic,meson6-efuse"
> +       - "amlogic,meson8-efuse"
> +       - "amlogic,meson8b-efuse"
> +- reg: base address and size of the efuse registers
> +- clocks: a reference to the efuse core gate clock
> +- clock-names: must be "core"
> +
> +All properties and sub-nodes as well as the consumer bindings
> +defined in nvmem.txt in this directory are also supported.
> +
> +
> +Example:
> +       efuse: nvmem@0 {
> +               compatible = "amlogic,meson8-efuse";
> +               reg = <0x0 0x2000>;
> +               clocks = <&clkc CLKID_EFUSE>;
> +               clock-names = "core";
> +       };
> --
> 2.11.0
>

[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-October/004943.html

[PATCH 11/12] nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs

[srinivas.kandagatla]

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

The current Amlogic Meson eFuse driver only supports the 64-bit SoCs
(GXBB and newer). Older SoCs cannot be supported by the same driver
because they do not use the meson secure monitor firmware to access the
hardware.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt | 2 +-
 drivers/nvmem/Kconfig                                     | 4 ++--
 drivers/nvmem/meson-efuse.c                               | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
index fafd85bd67a6..e3298e18de26 100644
--- a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
@@ -1,4 +1,4 @@
-= Amlogic eFuse device tree bindings =
+= Amlogic Meson GX eFuse device tree bindings =
 
 Required properties:
 - compatible: should be "amlogic,meson-gxbb-efuse"
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 2755df8f6e9b..e364c925e10c 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -146,11 +146,11 @@ config NVMEM_VF610_OCOTP
 	  be called nvmem-vf610-ocotp.
 
 config MESON_EFUSE
-	tristate "Amlogic eFuse Support"
+	tristate "Amlogic Meson GX eFuse Support"
 	depends on (ARCH_MESON || COMPILE_TEST) && MESON_SM
 	help
 	  This is a driver to retrieve specific values from the eFuse found on
-	  the Amlogic Meson SoCs.
+	  the Amlogic Meson GX SoCs.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called nvmem_meson_efuse.
diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
index dbedfcea4e05..a43c68f90937 100644
--- a/drivers/nvmem/meson-efuse.c
+++ b/drivers/nvmem/meson-efuse.c
@@ -1,5 +1,5 @@
 /*
- * Amlogic eFuse Driver
+ * Amlogic Meson GX eFuse Driver
  *
  * Copyright (c) 2016 Endless Computers, Inc.
  * Author: Carlo Caione <carlo@endlessm.com>
@@ -88,5 +88,5 @@ static struct platform_driver meson_efuse_driver = {
 module_platform_driver(meson_efuse_driver);
 
 MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
-MODULE_DESCRIPTION("Amlogic Meson NVMEM driver");
+MODULE_DESCRIPTION("Amlogic Meson GX NVMEM driver");
 MODULE_LICENSE("GPL v2");
-- 
2.11.0

Re: [PATCH 11/12] nvmem: meson-efuse: indicate that this driver is only for Meson GX SoCs

[Martin Blumenstingl]

On Mon, Oct 9, 2017 at 3:26 PM,  <srinivas.kandagatla@linaro.org> wrote:
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> The current Amlogic Meson eFuse driver only supports the 64-bit SoCs
> (GXBB and newer). Older SoCs cannot be supported by the same driver
> because they do not use the meson secure monitor firmware to access the
> hardware.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
I also got Rob's ACK for this patch, see [0]

> ---
>  Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt | 2 +-
>  drivers/nvmem/Kconfig                                     | 4 ++--
>  drivers/nvmem/meson-efuse.c                               | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
> index fafd85bd67a6..e3298e18de26 100644
> --- a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
> +++ b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
> @@ -1,4 +1,4 @@
> -= Amlogic eFuse device tree bindings =
> += Amlogic Meson GX eFuse device tree bindings =
>
>  Required properties:
>  - compatible: should be "amlogic,meson-gxbb-efuse"
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 2755df8f6e9b..e364c925e10c 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -146,11 +146,11 @@ config NVMEM_VF610_OCOTP
>           be called nvmem-vf610-ocotp.
>
>  config MESON_EFUSE
> -       tristate "Amlogic eFuse Support"
> +       tristate "Amlogic Meson GX eFuse Support"
>         depends on (ARCH_MESON || COMPILE_TEST) && MESON_SM
>         help
>           This is a driver to retrieve specific values from the eFuse found on
> -         the Amlogic Meson SoCs.
> +         the Amlogic Meson GX SoCs.
>
>           This driver can also be built as a module. If so, the module
>           will be called nvmem_meson_efuse.
> diff --git a/drivers/nvmem/meson-efuse.c b/drivers/nvmem/meson-efuse.c
> index dbedfcea4e05..a43c68f90937 100644
> --- a/drivers/nvmem/meson-efuse.c
> +++ b/drivers/nvmem/meson-efuse.c
> @@ -1,5 +1,5 @@
>  /*
> - * Amlogic eFuse Driver
> + * Amlogic Meson GX eFuse Driver
>   *
>   * Copyright (c) 2016 Endless Computers, Inc.
>   * Author: Carlo Caione <carlo@endlessm.com>
> @@ -88,5 +88,5 @@ static struct platform_driver meson_efuse_driver = {
>  module_platform_driver(meson_efuse_driver);
>
>  MODULE_AUTHOR("Carlo Caione <carlo@endlessm.com>");
> -MODULE_DESCRIPTION("Amlogic Meson NVMEM driver");
> +MODULE_DESCRIPTION("Amlogic Meson GX NVMEM driver");
>  MODULE_LICENSE("GPL v2");
> --
> 2.11.0
>


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-October/004944.html

[PATCH 12/12] nvmem: add a driver for the Amlogic Meson6/Meson8/Meson8b SoCs

[srinivas.kandagatla]

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

This adds a driver to access the efuse on Amlogic Meson6, Meson8 and
Meson8b SoCs.
These SoCs are accessing the efuse IP block directly through the
registers in the "secbus" region. This makes it different from the Meson
GX efuse driver which uses the "secure monitor" firmware to access the
efuse.

The efuse on Meson6 can only read one byte at a time, while the efuse on
Meson8 and Meson8b always reads 4 bytes at a time. The new driver
supports both, but due to lack of hardware Meson6 support was not tested.

The hardware also supports writing. However, this is currently not
supported by the driver.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/nvmem/Kconfig          |  10 ++
 drivers/nvmem/Makefile         |   2 +
 drivers/nvmem/meson-mx-efuse.c | 265 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/nvmem/meson-mx-efuse.c

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index e364c925e10c..ff505af064ba 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -155,6 +155,16 @@ config MESON_EFUSE
 	  This driver can also be built as a module. If so, the module
 	  will be called nvmem_meson_efuse.
 
+config MESON_MX_EFUSE
+	tristate "Amlogic Meson6/Meson8/Meson8b eFuse Support"
+	depends on ARCH_MESON || COMPILE_TEST
+	help
+	  This is a driver to retrieve specific values from the eFuse found on
+	  the Amlogic Meson6, Meson8 and Meson8b SoCs.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called nvmem_meson_mx_efuse.
+
 config NVMEM_SNVS_LPGPR
 	tristate "Support for Low Power General Purpose Register"
 	depends on SOC_IMX6 || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 11825353f729..64849e946d50 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -32,5 +32,7 @@ obj-$(CONFIG_NVMEM_VF610_OCOTP)	+= nvmem-vf610-ocotp.o
 nvmem-vf610-ocotp-y		:= vf610-ocotp.o
 obj-$(CONFIG_MESON_EFUSE)	+= nvmem_meson_efuse.o
 nvmem_meson_efuse-y		:= meson-efuse.o
+obj-$(CONFIG_MESON_MX_EFUSE)	+= nvmem_meson_mx_efuse.o
+nvmem_meson_mx_efuse-y		:= meson-mx-efuse.o
 obj-$(CONFIG_NVMEM_SNVS_LPGPR)	+= nvmem_snvs_lpgpr.o
 nvmem_snvs_lpgpr-y		:= snvs_lpgpr.o
diff --git a/drivers/nvmem/meson-mx-efuse.c b/drivers/nvmem/meson-mx-efuse.c
new file mode 100644
index 000000000000..a346b4923550
--- /dev/null
+++ b/drivers/nvmem/meson-mx-efuse.c
@@ -0,0 +1,265 @@
+/*
+ * Amlogic Meson6, Meson8 and Meson8b eFuse Driver
+ *
+ * Copyright (c) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/iopoll.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/sizes.h>
+#include <linux/slab.h>
+
+#define MESON_MX_EFUSE_CNTL1					0x04
+#define MESON_MX_EFUSE_CNTL1_PD_ENABLE				BIT(27)
+#define MESON_MX_EFUSE_CNTL1_AUTO_RD_BUSY			BIT(26)
+#define MESON_MX_EFUSE_CNTL1_AUTO_RD_START			BIT(25)
+#define MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE			BIT(24)
+#define MESON_MX_EFUSE_CNTL1_BYTE_WR_DATA			GENMASK(23, 16)
+#define MESON_MX_EFUSE_CNTL1_AUTO_WR_BUSY			BIT(14)
+#define MESON_MX_EFUSE_CNTL1_AUTO_WR_START			BIT(13)
+#define MESON_MX_EFUSE_CNTL1_AUTO_WR_ENABLE			BIT(12)
+#define MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET			BIT(11)
+#define MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK			GENMASK(10, 0)
+
+#define MESON_MX_EFUSE_CNTL2					0x08
+
+#define MESON_MX_EFUSE_CNTL4					0x10
+#define MESON_MX_EFUSE_CNTL4_ENCRYPT_ENABLE			BIT(10)
+
+struct meson_mx_efuse_platform_data {
+	const char *name;
+	unsigned int word_size;
+};
+
+struct meson_mx_efuse {
+	void __iomem *base;
+	struct clk *core_clk;
+	struct nvmem_device *nvmem;
+	struct nvmem_config config;
+};
+
+static void meson_mx_efuse_mask_bits(struct meson_mx_efuse *efuse, u32 reg,
+				     u32 mask, u32 set)
+{
+	u32 data;
+
+	data = readl(efuse->base + reg);
+	data &= ~mask;
+	data |= (set & mask);
+
+	writel(data, efuse->base + reg);
+}
+
+static int meson_mx_efuse_hw_enable(struct meson_mx_efuse *efuse)
+{
+	int err;
+
+	err = clk_prepare_enable(efuse->core_clk);
+	if (err)
+		return err;
+
+	/* power up the efuse */
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_PD_ENABLE, 0);
+
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL4,
+				 MESON_MX_EFUSE_CNTL4_ENCRYPT_ENABLE, 0);
+
+	return 0;
+}
+
+static void meson_mx_efuse_hw_disable(struct meson_mx_efuse *efuse)
+{
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_PD_ENABLE,
+				 MESON_MX_EFUSE_CNTL1_PD_ENABLE);
+
+	clk_disable_unprepare(efuse->core_clk);
+}
+
+static int meson_mx_efuse_read_addr(struct meson_mx_efuse *efuse,
+				    unsigned int addr, u32 *value)
+{
+	int err;
+	u32 regval;
+
+	/* write the address to read */
+	regval = FIELD_PREP(MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, addr);
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_BYTE_ADDR_MASK, regval);
+
+	/* inform the hardware that we changed the address */
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET,
+				 MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET);
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_BYTE_ADDR_SET, 0);
+
+	/* start the read process */
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_AUTO_RD_START,
+				 MESON_MX_EFUSE_CNTL1_AUTO_RD_START);
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_AUTO_RD_START, 0);
+
+	/*
+	 * perform a dummy read to ensure that the HW has the RD_BUSY bit set
+	 * when polling for the status below.
+	 */
+	readl(efuse->base + MESON_MX_EFUSE_CNTL1);
+
+	err = readl_poll_timeout_atomic(efuse->base + MESON_MX_EFUSE_CNTL1,
+			regval,
+			(!(regval & MESON_MX_EFUSE_CNTL1_AUTO_RD_BUSY)),
+			1, 1000);
+	if (err) {
+		dev_err(efuse->config.dev,
+			"Timeout while reading efuse address %u\n", addr);
+		return err;
+	}
+
+	*value = readl(efuse->base + MESON_MX_EFUSE_CNTL2);
+
+	return 0;
+}
+
+static int meson_mx_efuse_read(void *context, unsigned int offset,
+			       void *buf, size_t bytes)
+{
+	struct meson_mx_efuse *efuse = context;
+	u32 tmp;
+	int err, i, addr;
+
+	err = meson_mx_efuse_hw_enable(efuse);
+	if (err)
+		return err;
+
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE,
+				 MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE);
+
+	for (i = offset; i < offset + bytes; i += efuse->config.word_size) {
+		addr = i / efuse->config.word_size;
+
+		err = meson_mx_efuse_read_addr(efuse, addr, &tmp);
+		if (err)
+			break;
+
+		memcpy(buf + i, &tmp, efuse->config.word_size);
+	}
+
+	meson_mx_efuse_mask_bits(efuse, MESON_MX_EFUSE_CNTL1,
+				 MESON_MX_EFUSE_CNTL1_AUTO_RD_ENABLE, 0);
+
+	meson_mx_efuse_hw_disable(efuse);
+
+	return err;
+}
+
+static const struct meson_mx_efuse_platform_data meson6_efuse_data = {
+	.name = "meson6-efuse",
+	.word_size = 1,
+};
+
+static const struct meson_mx_efuse_platform_data meson8_efuse_data = {
+	.name = "meson8-efuse",
+	.word_size = 4,
+};
+
+static const struct meson_mx_efuse_platform_data meson8b_efuse_data = {
+	.name = "meson8b-efuse",
+	.word_size = 4,
+};
+
+static const struct of_device_id meson_mx_efuse_match[] = {
+	{ .compatible = "amlogic,meson6-efuse", .data = &meson6_efuse_data },
+	{ .compatible = "amlogic,meson8-efuse", .data = &meson8_efuse_data },
+	{ .compatible = "amlogic,meson8b-efuse", .data = &meson8b_efuse_data },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, meson_mx_efuse_match);
+
+static int meson_mx_efuse_probe(struct platform_device *pdev)
+{
+	const struct meson_mx_efuse_platform_data *drvdata;
+	struct meson_mx_efuse *efuse;
+	struct resource *res;
+
+	drvdata = of_device_get_match_data(&pdev->dev);
+	if (!drvdata)
+		return -EINVAL;
+
+	efuse = devm_kzalloc(&pdev->dev, sizeof(*efuse), GFP_KERNEL);
+	if (!efuse)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	efuse->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(efuse->base))
+		return PTR_ERR(efuse->base);
+
+	efuse->config.name = devm_kstrdup(&pdev->dev, drvdata->name,
+					  GFP_KERNEL);
+	efuse->config.owner = THIS_MODULE;
+	efuse->config.dev = &pdev->dev;
+	efuse->config.priv = efuse;
+	efuse->config.stride = drvdata->word_size;
+	efuse->config.word_size = drvdata->word_size;
+	efuse->config.size = SZ_512;
+	efuse->config.read_only = true;
+	efuse->config.reg_read = meson_mx_efuse_read;
+
+	efuse->core_clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(efuse->core_clk)) {
+		dev_err(&pdev->dev, "Failed to get core clock\n");
+		return PTR_ERR(efuse->core_clk);
+	}
+
+	efuse->nvmem = nvmem_register(&efuse->config);
+	if (IS_ERR(efuse->nvmem))
+		return PTR_ERR(efuse->nvmem);
+
+	platform_set_drvdata(pdev, efuse);
+
+	return 0;
+}
+
+static int meson_mx_efuse_remove(struct platform_device *pdev)
+{
+	struct meson_mx_efuse *efuse = platform_get_drvdata(pdev);
+
+	return nvmem_unregister(efuse->nvmem);
+}
+
+static struct platform_driver meson_mx_efuse_driver = {
+	.probe = meson_mx_efuse_probe,
+	.remove = meson_mx_efuse_remove,
+	.driver = {
+		.name = "meson-mx-efuse",
+		.of_match_table = meson_mx_efuse_match,
+	},
+};
+
+module_platform_driver(meson_mx_efuse_driver);
+
+MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
+MODULE_DESCRIPTION("Amlogic Meson MX eFuse NVMEM driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0