From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?windows-1252?Q?Andreas_Bie=DFmann?= Date: Fri, 16 Jan 2015 09:24:13 +0100 Subject: [U-Boot] [PATCH] mtd: atmel_nand: according to pmecc version to perform 0xff page correction In-Reply-To: <54B8B4A8.8010702@atmel.com> References: <1421380486-22138-1-git-send-email-josh.wu@atmel.com> <54B8A154.3000603@atmel.com> <54B8B4A8.8010702@atmel.com> Message-ID: <54B8CAAD.5060407@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bo, Josh, On 01/16/2015 07:50 AM, Josh Wu wrote: > Hi, Bo > > On 1/16/2015 1:27 PM, Bo Shen wrote: >> Hi Josh, >> >> On 01/16/2015 11:54 AM, Josh Wu wrote: >>> As the PMECC hardware has different version. In SAMA5D4 chip, the >>> PMECC ip >>> can generate 0xff pmecc ECC value for all 0xff sector. >>> >>> According to this, add PMECC version check, if it's SAMA5D4 then we >>> always >>> let PMECC hardware to correct it. >>> >>> Signed-off-by: Josh Wu >> >> except the nitpick. >> >> Acked-by: Bo Shen Acked-by: Andreas Bie?mann > > Thanks for the quick review. > >> >>> >>> --- >>> >>> drivers/mtd/nand/atmel_nand.c | 9 +++++++++ >>> drivers/mtd/nand/atmel_nand_ecc.h | 20 ++++++++++++++++++++ >>> 2 files changed, 29 insertions(+) >>> >>> diff --git a/drivers/mtd/nand/atmel_nand.c >>> b/drivers/mtd/nand/atmel_nand.c >>> index 620b6e8..b16e3aa 100644 >>> --- a/drivers/mtd/nand/atmel_nand.c >>> +++ b/drivers/mtd/nand/atmel_nand.c >>> @@ -44,6 +44,7 @@ struct atmel_nand_host { >>> u8 pmecc_corr_cap; >>> u16 pmecc_sector_size; >>> u32 pmecc_index_table_offset; >>> + u32 pmecc_version; >>> >>> int pmecc_bytes_per_sector; >>> int pmecc_sector_number; >>> @@ -486,6 +487,10 @@ static int pmecc_correction(struct mtd_info >>> *mtd, u32 pmecc_stat, uint8_t *buf, >>> int i, err_nbr, eccbytes; >>> uint8_t *buf_pos; >>> >>> + /* SAMA5D4 PMECC IP can correct errors for all 0xff page */ >>> + if (host->pmecc_version >= PMECC_VERSION_SAMA5D4) >> >> I think we can hard coded here, then we can drop the definition in >> header file. > I don't like hard coded magic number in personly. me too! > >> >>> + goto normal_check; >>> + >>> eccbytes = nand_chip->ecc.bytes; >>> for (i = 0; i < eccbytes; i++) >>> if (ecc[i] != 0xff) >>> @@ -961,6 +966,10 @@ static int atmel_pmecc_nand_init_params(struct >>> nand_chip *nand, >>> nand->ecc.write_page = atmel_nand_pmecc_write_page; >>> nand->ecc.strength = cap; >>> >>> + /* Check the PMECC ip version */ >>> + host->pmecc_version = pmecc_readl(host->pmerrloc, version); >>> + dev_dbg(host->dev, "PMECC IP version is: %x\n", >>> host->pmecc_version); >>> + >>> atmel_pmecc_core_init(mtd); >>> >>> return 0; >>> diff --git a/drivers/mtd/nand/atmel_nand_ecc.h >>> b/drivers/mtd/nand/atmel_nand_ecc.h >>> index eac860d..b2d2682 100644 >>> --- a/drivers/mtd/nand/atmel_nand_ecc.h >>> +++ b/drivers/mtd/nand/atmel_nand_ecc.h >>> @@ -123,6 +123,20 @@ struct pmecc_errloc_regs { >>> u32 sigma[25]; /* 0x28-0x88 Error Location Sigma Registers */ >>> u32 el[24]; /* 0x8C-0xE8 Error Location Registers */ >>> u32 reserved1[5]; /* 0xEC-0xFC Reserved */ >>> + >>> + /* >>> + * 0x100-0x1F8: >>> + * Reserved for AT91SAM9X5, AT91SAM9N12. >>> + * HSMC registers for SAMA5D3, SAMA5D4. >>> + */ >> >> I think no need to add this. > actually, I would like to keep those comments. > As in the datasheet, sama5d3 and sama5d4's PMECC ERRLOC only have the > register range: 0x0~0xFF. > and the range 0x100-0x1F8 is for the HSMC. > > So people will be confused if they find HSMC definitions in the > PMECC_ERRLOC structure. > I think list this comment would be cleaner. This is reasonable, please let the comment in. > >> >>> + u32 reserved2[63]; >>> + >>> + /* >>> + * 0x1FC: >>> + * PMECC version for AT91SAM9X5, AT91SAM9N12. >>> + * HSMC version for SAMA5D3, SAMA5D4. Can refer as PMECC version. >>> + */ >> >> ditto. > Like mentioned above, I want avoid the confusion about PMECC_ERRLOC/HSMC. > >> >>> + u32 version; >>> }; >>> >>> /* For Error Location Configuration Register */ >>> @@ -137,6 +151,12 @@ struct pmecc_errloc_regs { >>> #define PMERRLOC_ERR_NUM_MASK (0x1f << 8) >>> #define PMERRLOC_CALC_DONE (1 << 0) >>> >>> +/* PMECC IP version */ >>> +#define PMECC_VERSION_SAMA5D4 0x113 >>> +#define PMECC_VERSION_SAMA5D3 0x112 >>> +#define PMECC_VERSION_AT91SAM9N12 0x102 >> >> No where will use the upper three definitions, we can drop them. > I don't have strong option about this. Just think the version > information is useful for other to reference. Can someone else get this information from the datasheets? If not please let this information there. Best regards Andreas Bie?mann