From mboxrd@z Thu Jan  1 00:00:00 1970
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Mon, 29 Oct 2018 18:58:36 +0200
Subject: [U-Boot] [PATCH] mtd: nand: lpc32xx mlc: predefine number of
 NAND chips to support
In-Reply-To: <20181029102935.35d6710d@xps13>
References: <20181019002105.4620-1-vz@mleia.com>
 <20181029102935.35d6710d@xps13>
Message-ID: <203c14c9-c9f9-2498-1f77-6d265f2b85f8@mleia.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Miquel,

On 10/29/2018 11:29 AM, Miquel Raynal wrote:
> Hi Vladimir,
> 
> Vladimir Zapolskiy <vz@mleia.com> wrote on Fri, 19 Oct 2018 03:21:05
> +0300:
> 
>> Build option CONFIG_SYS_MAX_NAND_CHIPS is used by NXP LPC32xx NAND MLC
>> driver only, as a preparation for potential removal or replacement of
>> the option the change predefines CONFIG_SYS_MAX_NAND_CHIPS to 1, same
>> value is used by the single user Work Microwave Work 92105 board, thus
>> it will be safe now to remove the option as a board specific one.
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---
>>  drivers/mtd/nand/raw/lpc32xx_nand_mlc.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/raw/lpc32xx_nand_mlc.c b/drivers/mtd/nand/raw/lpc32xx_nand_mlc.c
>> index 5d4ffea608d9..79d1489dc72c 100644
>> --- a/drivers/mtd/nand/raw/lpc32xx_nand_mlc.c
>> +++ b/drivers/mtd/nand/raw/lpc32xx_nand_mlc.c
>> @@ -82,6 +82,10 @@ struct lpc32xx_nand_mlc_registers {
>>  static struct lpc32xx_nand_mlc_registers __iomem *lpc32xx_nand_mlc_registers
>>  	= (struct lpc32xx_nand_mlc_registers __iomem *)MLC_NAND_BASE;
>>  
>> +#if !defined(CONFIG_SYS_MAX_NAND_CHIPS)
>> +#define CONFIG_SYS_MAX_NAND_CHIPS	1
>> +#endif
>> +
>>  #define clkdiv(v, w, o) (((1+(clk/v)) & w) << o)
>>  
>>  /**
> 
> 
> If it is safe now, can you also remote the CONFIG_ option now?

At the moment I plan to remove the CONFIG_SYS_MAX_NAND_CHIPS option
from include/configs/work_92105.h as a side activity (concentrate all
LPC32xx raw NAND options in arch/arm/include/asm/arch-lpc32xx/config.h,
the change is in my queue), and regarding total removal of the option,
it shall be considered separately.

For instance you can find it is mentioned in doc/README.nand :

        if (nand_scan_ident(mtd, CONFIG_SYS_MAX_NAND_CHIPS, NULL))
                error out

and commonly it would be expected to provide it as an argument to
nand_scan_ident(), however it is easy to notice that none drivers
but LPC32xx NAND MLC does it. Also note that sometimes NAND *chips*
number is substituted by a number of NAND *controllers* on SoC...

Generally it sounds like a proper change should be a tree-wide one.

So, while I clearly understand the rationale, I would prefer to shift
the task to the MTD maintainers ;)

> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thank you for review!

--
Best wishes,
Vladimir