Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1730336 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config instead of malloc'ing it Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-09-11 15:10 +0200
    Re: [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config  instead of malloc'ing it Sean Wang <sean.wang@mediatek.com> - 2017-09-20 18:40 +0200
      Re: [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config instead  of malloc'ing it Masahiro Yamada <yamada.masahiro@socionext.com> - 2017-09-21 04:20 +0200
        Re: [PATCH 2/5] nvmem: mtk-efuse: use stack for nvmem_config  instead of malloc'ing it Sean Wang <sean.wang@mediatek.com> - 2017-09-22 06:10 +0200

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

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

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

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

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

[toc] | [next] | [standalone]

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

Hi, Masahiro

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

	Sean


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

[toc] | [prev] | [next] | [standalone]

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

Hi Sean,


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


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

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

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

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


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


-- 
Best Regards
Masahiro Yamada

[toc] | [prev] | [next] | [standalone]

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

Hi, Masahiro

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

thank you for your detailed explanation

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

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


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

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web