[U-Boot] [U-Boot, 2/2] NAND: add support for reading ONFI page table
Florian Fainelli
florian at openwrt.org
Sat Dec 11 18:08:36 CET 2010
On Saturday 11 December 2010 01:15:13 Scott Wood wrote:
> On Fri, Dec 10, 2010 at 12:16:42PM -0000, Florian Fainelli wrote:
> > From: Florian Fainelli <florian at openwrt.org>
> >
> > This patch adds support for reading an ONFI page parameter from a NAND
> > device supporting it. If this is the case, struct nand_chip onfi_version
> > member contains the supported ONFI version, 0 otherwise.
> >
> > This allows NAND drivers past nand_scan_ident to set the best timings for
> > the NAND chip.
>
> Wrap changelogs around 72 characters, as git log indents them a bit.
>
> > Signed-off-by: Florian Fainelli <florian at openwrt.org>
> >
> > ---
> > drivers/mtd/nand/nand_base.c | 129
> > ++++++++++++++++++++++++++++++++++-------
> >
> > include/linux/mtd/nand.h | 68 ++++++++++++++++++++++
> > 2 files changed, 175 insertions(+), 22 deletions(-)
>
> Tested on what I assume is a non-ONFI chip, no breakage.
>
> Please wrap this in a CONFIG option, so that we don't increase the
> image size of boards that don't need to support this.
Makes sense, I will do this
>
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 21cc5a3..a3a0507 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -2406,15 +2406,94 @@ static void nand_set_defaults(struct nand_chip
> > *chip, int busw)
> >
> > chip->controller = &chip->hwcontrol;
> >
> > }
> >
> > +static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
> > +{
> > + int i;
> > +
> > + while (len--) {
> > + crc ^= *p++ << 8;
> > + for (i = 0; i < 8; i++)
> > + crc = (crc << 1) ^ ((crc & 0x8000) ? 0x8005 : 0);
> > + }
> > +
> > + return crc;
> > +}
> > +
> > +/*
> > + * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0
> > otherwise + */
> > +static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip
> > *chip, + int busw)
> > +{
>
> Keep lines under 80 columns.
>
> > +
> > + /* try ONFI for unknow chip or LP */
>
> "unknown".
>
> > + chip->cmdfunc(mtd, NAND_CMD_READID, 0x20, -1);
> > + if (chip->read_byte(mtd) != 'O' || chip->read_byte(mtd) != 'N' ||
> > + chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
> > + return 0;
>
> I'm not sure how this comment matches the code -- it looks like it's
> checking for an explicit ONFI ID. I don't see anything about unknown
> chips or large pages.
It does not actually, I did not think about checking it when porting this from Linux.
>
> > + /* check version */
> > + val = le16_to_cpu(p->revision);
> > + if (val == 1 || val > (1 << 4)) {
> > + printk(KERN_INFO "%s: unsupported ONFI version: %d\n",
> > + __func__, val);
> > + return 0;
> > + }
>
> Ideally I'd like to see continuation lines lined up nicely with
> the start of arguments on the previous line, but at least
> don't tab it all the way over to the right edge of the screen.
>
> > chip->chipsize = (uint64_t)type->chipsize << 20;
> >
> > + chip->onfi_version = 0;
> > +
> > + ret = nand_flash_detect_onfi(mtd, chip, busw);
> > + if (ret)
> > + goto ident_done;
>
> Move the non-ONFI code out into its own function, instead of using
> goto.
>
> > + __le32 blocks_per_lun;
> > + u8 lun_count;
> > + u8 addr_cycles;
> > + u8 bits_per_cell;
> > + __le16 bb_per_lun;
> > + __le16 block_endurance;
> > + u8 guaranteed_good_blocks;
> > + __le16 guaranteed_block_endurance;
>
> [snip]
>
> > +} __attribute__((packed));
>
> Sigh. Someone on the standards body needs to learn about alignment.
>
> -Scott
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list