diff mbox series

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

Message ID 20171009132641.27169-3-srinivas.kandagatla@linaro.org
State New
Headers show
Series nvmem: patches set-1 for v4.15 | expand

Commit Message

Srinivas Kandagatla Oct. 9, 2017, 1:26 p.m. UTC
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(-)

-- 
2.11.0

Comments

Greg KH Oct. 20, 2017, 1:34 p.m. UTC | #1
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
Masahiro Yamada Oct. 20, 2017, 1:47 p.m. UTC | #2
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
Greg KH Oct. 20, 2017, 1:54 p.m. UTC | #3
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
Masahiro Yamada Oct. 20, 2017, 1:59 p.m. UTC | #4
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
Greg KH Oct. 20, 2017, 2:48 p.m. UTC | #5
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
diff mbox series

Patch

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);