All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Quadros <rogerq@ti.com>
To: Rostislav Lisovy <lisovy@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	pekon <pekon@pek-sem.com>, Tony Lindgren <tony@atomide.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Cc: michal.vokac@comap.cz, Rostislav Lisovy <lisovy@merica.cz>,
	sojkam1@fel.cvut.cz
Subject: Re: [PATCH 1/2] mtd: nand: omap: Do not use global variables
Date: Thu, 2 Oct 2014 09:44:22 +0300	[thread overview]
Message-ID: <542CF446.6090608@ti.com> (raw)
In-Reply-To: <1412175554-19391-1-git-send-email-lisovy@merica.cz>

Hi,

On 10/01/2014 05:59 PM, Rostislav Lisovy wrote:
> Since the commit 97a288ba2cfa ("ARM: omap2+: gpmc-nand: Use
> dynamic platform_device_alloc()") gpmc-nand driver supports
> multiple NAND flash devices connected to the single controller.
> Remove global variable to make the code thread-safe.
> 
> Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
> ---
>  drivers/mtd/nand/omap2.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 5967b38..24d5c6a 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -146,8 +146,6 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
>  static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
>  #endif
>  
> -/* oob info generated runtime depending on ecc algorithm and layout selected */
> -static struct nand_ecclayout omap_oobinfo;
>  
>  struct omap_nand_info {
>  	struct nand_hw_control		controller;
> @@ -1794,7 +1792,7 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	/* populate MTD interface based on ECC scheme */
> -	ecclayout		= &omap_oobinfo;
> +	ecclayout		= kzalloc(sizeof(*ecclayout), GFP_KERNEL);

We should free this in failure path or omap_nand_remove(), or use devm_kzalloc().

How about making "struct nand_ecclayout" a part of omap_nand_info, so that it gets allocated
without a separate devm_kzalloc() call?

>  	switch (info->ecc_opt) {
>  	case OMAP_ECC_HAM1_CODE_SW:
>  		nand_chip->ecc.mode = NAND_ECC_SOFT;
> 

cheers,
-roger

WARNING: multiple messages have this Message-ID (diff)
From: Roger Quadros <rogerq@ti.com>
To: Rostislav Lisovy <lisovy@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	pekon <pekon@pek-sem.com>, Tony Lindgren <tony@atomide.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Cc: <michal.vokac@comap.cz>, <sojkam1@fel.cvut.cz>,
	Rostislav Lisovy <lisovy@merica.cz>
Subject: Re: [PATCH 1/2] mtd: nand: omap: Do not use global variables
Date: Thu, 2 Oct 2014 09:44:22 +0300	[thread overview]
Message-ID: <542CF446.6090608@ti.com> (raw)
In-Reply-To: <1412175554-19391-1-git-send-email-lisovy@merica.cz>

Hi,

On 10/01/2014 05:59 PM, Rostislav Lisovy wrote:
> Since the commit 97a288ba2cfa ("ARM: omap2+: gpmc-nand: Use
> dynamic platform_device_alloc()") gpmc-nand driver supports
> multiple NAND flash devices connected to the single controller.
> Remove global variable to make the code thread-safe.
> 
> Signed-off-by: Rostislav Lisovy <lisovy@merica.cz>
> ---
>  drivers/mtd/nand/omap2.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 5967b38..24d5c6a 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -146,8 +146,6 @@ static u_char bch8_vector[] = {0xf3, 0xdb, 0x14, 0x16, 0x8b, 0xd2, 0xbe, 0xcc,
>  static u_char bch4_vector[] = {0x00, 0x6b, 0x31, 0xdd, 0x41, 0xbc, 0x10};
>  #endif
>  
> -/* oob info generated runtime depending on ecc algorithm and layout selected */
> -static struct nand_ecclayout omap_oobinfo;
>  
>  struct omap_nand_info {
>  	struct nand_hw_control		controller;
> @@ -1794,7 +1792,7 @@ static int omap_nand_probe(struct platform_device *pdev)
>  	}
>  
>  	/* populate MTD interface based on ECC scheme */
> -	ecclayout		= &omap_oobinfo;
> +	ecclayout		= kzalloc(sizeof(*ecclayout), GFP_KERNEL);

We should free this in failure path or omap_nand_remove(), or use devm_kzalloc().

How about making "struct nand_ecclayout" a part of omap_nand_info, so that it gets allocated
without a separate devm_kzalloc() call?

>  	switch (info->ecc_opt) {
>  	case OMAP_ECC_HAM1_CODE_SW:
>  		nand_chip->ecc.mode = NAND_ECC_SOFT;
> 

cheers,
-roger

  parent reply	other threads:[~2014-10-02  6:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-01 14:59 [PATCH 1/2] mtd: nand: omap: Do not use global variables Rostislav Lisovy
2014-10-01 14:59 ` Rostislav Lisovy
2014-10-01 14:59 ` [PATCH 2/2] mtd: nand: omap: Synchronize access to the ECC engine Rostislav Lisovy
2014-10-01 14:59   ` Rostislav Lisovy
2014-10-02  7:12   ` Roger Quadros
2014-10-02  7:12     ` Roger Quadros
2014-10-02  6:44 ` Roger Quadros [this message]
2014-10-02  6:44   ` [PATCH 1/2] mtd: nand: omap: Do not use global variables Roger Quadros

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=542CF446.6090608@ti.com \
    --to=rogerq@ti.com \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=lisovy@gmail.com \
    --cc=lisovy@merica.cz \
    --cc=michal.vokac@comap.cz \
    --cc=pekon@pek-sem.com \
    --cc=sojkam1@fel.cvut.cz \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.