diff mbox

mtd: increase max page/OOB size to support 16K pagesize NAND

Message ID C3050A4DBA34F345975765E43127F10F25E47455@SZXEMA512-MBX.china.huawei.com
State New, archived
Headers show

Commit Message

Caizhiyong Dec. 13, 2013, 5:03 a.m. UTC 
From: Cai Zhiyong <caizhiyong@huawei.com>
Date: Fri, 13 Dec 2013 12:52:46 +0800
Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND.

The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes.
So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE.

Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
---
 include/linux/mtd/nand.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Huang Shijie Dec. 13, 2013, 7:27 a.m. UTC | #1 
On Fri, Dec 13, 2013 at 05:03:49AM +0000, Caizhiyong wrote:
> From: Cai Zhiyong <caizhiyong@huawei.com>
> Date: Fri, 13 Dec 2013 12:52:46 +0800
> Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND.
> 
> The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes.
> So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE.
> 
> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
> ---
>  include/linux/mtd/nand.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index f3ea8da..2f0a7f2 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>   * is supported now. If you add a chip with bigger oobsize/page
>   * adjust this accordingly.
>   */
> -#define NAND_MAX_OOBSIZE	744
> -#define NAND_MAX_PAGESIZE	8192
> +#define NAND_MAX_OOBSIZE	1280
> +#define NAND_MAX_PAGESIZE	16384
maybe it is time to remove these two macros.

thanks
Huang Shijie
Brian Norris Dec. 13, 2013, 8:01 a.m. UTC | #2 
On Fri, Dec 13, 2013 at 05:03:49AM +0000, Caizhiyong wrote:
> From: Cai Zhiyong <caizhiyong@huawei.com>
> Date: Fri, 13 Dec 2013 12:52:46 +0800
> Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND.
> 
> The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes.
> So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE.

It would help if we had nand_base/nand_ids support for TC58TEG5DCJTA
before we try to increase NAND_MAX_PAGESIZE and NAND_MAX_OOBSIZE yet
again. AFAICT, nand_base will not detect new, large Toshiba flash
properly anyway.

> Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
> ---
>  include/linux/mtd/nand.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index f3ea8da..2f0a7f2 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>   * is supported now. If you add a chip with bigger oobsize/page
>   * adjust this accordingly.
>   */
> -#define NAND_MAX_OOBSIZE	744
> -#define NAND_MAX_PAGESIZE	8192
> +#define NAND_MAX_OOBSIZE	1280
> +#define NAND_MAX_PAGESIZE	16384

It might be time to draw a line in the sand: next person who needs to
increase MAX_{PAGE,OOB}SIZE gets the job of killing the macro? We are
long overdue for dynamic buffer allocation.

There are two drivers that use the macros, plus nand_chip.buffers.*. The
drivers can be weaned off by allocating a small, fixed-size temporary
buffer for the few tasks that need it. With nand_base.c, I think we can
get by with waiting to allocate nand_chip.buffers until after we detect
the writesize/oobsize.

So Cai, are you up for this?

>  
>  /*
>   * Constants for hardware specific CLE/ALE/NCE function
> 

Brian
Brian Norris Dec. 13, 2013, 8:04 a.m. UTC | #3 
On Fri, Dec 13, 2013 at 03:27:12PM +0800, Huang Shijie wrote:
> On Fri, Dec 13, 2013 at 05:03:49AM +0000, Caizhiyong wrote:
> > --- a/include/linux/mtd/nand.h
> > +++ b/include/linux/mtd/nand.h
> > @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
> >   * is supported now. If you add a chip with bigger oobsize/page
> >   * adjust this accordingly.
> >   */
> > -#define NAND_MAX_OOBSIZE	744
> > -#define NAND_MAX_PAGESIZE	8192
> > +#define NAND_MAX_OOBSIZE	1280
> > +#define NAND_MAX_PAGESIZE	16384
> maybe it is time to remove these two macros.

It's like you read my mind :)

Brian
Caizhiyong Dec. 13, 2013, 11:10 a.m. UTC | #4 
> On Fri, Dec 13, 2013 at 05:03:49AM +0000, Caizhiyong wrote:
> > From: Cai Zhiyong <caizhiyong@huawei.com>
> > Date: Fri, 13 Dec 2013 12:52:46 +0800
> > Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND.
> >
> > The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes.
> > So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE.
> 
> It would help if we had nand_base/nand_ids support for TC58TEG5DCJTA
> before we try to increase NAND_MAX_PAGESIZE and NAND_MAX_OOBSIZE yet
> again. AFAICT, nand_base will not detect new, large Toshiba flash
> properly anyway.

There are a lot of 16K page size NAND, they are used in android system.

Many NAND need read retry and data randomization, kernel does not support this feature.
Kernel also does not support synchronous NAND.
As you know, is someone doing some work to support those feature now?

If Kernel support TC58TEG5DCJTA, the read retry and data randomization also
need be supported, it will be a big project.

> 
> > Signed-off-by: Cai Zhiyong <caizhiyong@huawei.com>
> > ---
> >  include/linux/mtd/nand.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> > index f3ea8da..2f0a7f2 100644
> > --- a/include/linux/mtd/nand.h
> > +++ b/include/linux/mtd/nand.h
> > @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs,
> uint64_t len);
> >   * is supported now. If you add a chip with bigger oobsize/page
> >   * adjust this accordingly.
> >   */
> > -#define NAND_MAX_OOBSIZE	744
> > -#define NAND_MAX_PAGESIZE	8192
> > +#define NAND_MAX_OOBSIZE	1280
> > +#define NAND_MAX_PAGESIZE	16384
> 
> It might be time to draw a line in the sand: next person who needs to
> increase MAX_{PAGE,OOB}SIZE gets the job of killing the macro? We are
> long overdue for dynamic buffer allocation.
> 
> There are two drivers that use the macros, plus nand_chip.buffers.*. The
> drivers can be weaned off by allocating a small, fixed-size temporary
> buffer for the few tasks that need it. With nand_base.c, I think we can
> get by with waiting to allocate nand_chip.buffers until after we detect
> the writesize/oobsize.
> 
> So Cai, are you up for this?

Ok, I will try update it.

> 
> >
> >  /*
> >   * Constants for hardware specific CLE/ALE/NCE function
> >
> 
> Brian
Huang Shijie Dec. 13, 2013, 3:11 p.m. UTC | #5 
On Fri, Dec 13, 2013 at 11:10:10AM +0000, Caizhiyong wrote:
> 
> There are a lot of 16K page size NAND, they are used in android system.
> 
> Many NAND need read retry and data randomization, kernel does not support this feature.
Brian is adding the read-retry feature.

I think the data randomization should be done by the hardware.
If we do it by software, it costs lot of the cpu cycles.


> Kernel also does not support synchronous NAND.

The synchronous NAND should be supported by the nand controller driver,
not the kernel.


thanks
Huang Shijie
Caizhiyong Dec. 16, 2013, 2:03 a.m. UTC | #6 
> >
> > There are a lot of 16K page size NAND, they are used in android system.
> >
> > Many NAND need read retry and data randomization, kernel does not support this
> feature.
> Brian is adding the read-retry feature.
> 
> I think the data randomization should be done by the hardware.
> If we do it by software, it costs lot of the cpu cycles.
>
> 
> > Kernel also does not support synchronous NAND.
> 
> The synchronous NAND should be supported by the nand controller driver,
> not the kernel.

Yes. Those feature should be support by NAND controller hardware and driver.
But the driver does not know what NAND should use those feature.
Perhaps, the NAND id table, should add something.

Looking forward to your feedback.
Best regards.
Cai Zhiyong.
Huang Shijie Dec. 16, 2013, 3:06 a.m. UTC | #7 
On Mon, Dec 16, 2013 at 02:03:54AM +0000, Caizhiyong wrote:
> > >
> > > There are a lot of 16K page size NAND, they are used in android system.
> > >
> > > Many NAND need read retry and data randomization, kernel does not support this
> > feature.
> > Brian is adding the read-retry feature.
> > 
> > I think the data randomization should be done by the hardware.
> > If we do it by software, it costs lot of the cpu cycles.
> >
> > 
> > > Kernel also does not support synchronous NAND.
> > 
> > The synchronous NAND should be supported by the nand controller driver,
> > not the kernel.
> 
> Yes. Those feature should be support by NAND controller hardware and driver.
> But the driver does not know what NAND should use those feature.
> Perhaps, the NAND id table, should add something.
We can get the synchronous info from the ONFI nand.

In actually, if you do not add this patch, i will change it too.
I have several 16K nand chips on my hands now. :)

thanks
Huang Shijie
Caizhiyong Dec. 16, 2013, 4:08 a.m. UTC | #8 
> -----Original Message-----
> From: Huang Shijie [mailto:b32955@freescale.com]
> Sent: Monday, December 16, 2013 11:07 AM
> To: Caizhiyong
> Cc: Huang Shijie; Brian Norris; David Woodhouse; Quyaxin;
> linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org; Artem Bityutskiy;
> Wanglin (Albert); David Woodhouse
> Subject: Re: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND
> 
> On Mon, Dec 16, 2013 at 02:03:54AM +0000, Caizhiyong wrote:
> > > >
> > > > There are a lot of 16K page size NAND, they are used in android system.
> > > >
> > > > Many NAND need read retry and data randomization, kernel does not support
> this
> > > feature.
> > > Brian is adding the read-retry feature.
> > >
> > > I think the data randomization should be done by the hardware.
> > > If we do it by software, it costs lot of the cpu cycles.
> > >
> > >
> > > > Kernel also does not support synchronous NAND.
> > >
> > > The synchronous NAND should be supported by the nand controller driver,
> > > not the kernel.
> >
> > Yes. Those feature should be support by NAND controller hardware and driver.
> > But the driver does not know what NAND should use those feature.
> > Perhaps, the NAND id table, should add something.
> We can get the synchronous info from the ONFI nand.
> 
> In actually, if you do not add this patch, i will change it too.
> I have several 16K nand chips on my hands now. :)

I have not found a good way to solve these problems. I expect your patch.

> 
> thanks
> Huang Shijie
>
diff mbox

Patch

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index f3ea8da..2f0a7f2 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -56,8 +56,8 @@  extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
  * is supported now. If you add a chip with bigger oobsize/page
  * adjust this accordingly.
  */
-#define NAND_MAX_OOBSIZE	744
-#define NAND_MAX_PAGESIZE	8192
+#define NAND_MAX_OOBSIZE	1280
+#define NAND_MAX_PAGESIZE	16384
 
 /*
  * Constants for hardware specific CLE/ALE/NCE function