Re: [linux-sunxi] [PATCH 01/23] mtd: kill the ecclayout->oobavail field
Hi Priit, On Tue, 08 Dec 2015 08:43:05 +0200 Priit Laeswrote: > On Mon, 2015-12-07 at 23:25 +0100, Boris Brezillon wrote: > > ecclayout->oobavail is just redundant with the mtd->oobavail field. > > Moreover, it prevents static const definition of ecc layouts since > > the > > NAND framework is calculating this value based on the ecclayout- > > >oobfree > > field. > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/mtd/devices/docg3.c | 5 ++- > > drivers/mtd/mtdswap.c | 16 - > > drivers/mtd/nand/brcmnand/brcmnand.c | 3 -- > > drivers/mtd/nand/docg4.c | 1 - > > drivers/mtd/nand/hisi504_nand.c | 1 - > > drivers/mtd/nand/nand_base.c | 12 +++ > > drivers/mtd/onenand/onenand_base.c| 16 - > > drivers/mtd/tests/oobtest.c | 49 +-- > > > > drivers/staging/mt29f_spinand/mt29f_spinand.c | 1 - > > fs/jffs2/wbuf.c | 6 ++-- > > include/linux/mtd/mtd.h | 1 - > > 11 files changed, 48 insertions(+), 63 deletions(-) > > > [..] > > > > diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c > > b/drivers/mtd/nand/brcmnand/brcmnand.c > > index 35d78f7..a906ec2 100644 > > --- a/drivers/mtd/nand/brcmnand/brcmnand.c > > +++ b/drivers/mtd/nand/brcmnand/brcmnand.c > > @@ -845,9 +845,6 @@ static struct nand_ecclayout > > *brcmnand_create_layout(int ecc_level, > > break; > > } > > out: > > - /* Sum available OOB */ > > - for (i = 0; i < MTD_MAX_OOBFREE_ENTRIES_LARGE; i++) > > - layout->oobavail += layout->oobfree[i].length; > > return layout; > > } > > You can get rid of the 'out' label and replace the single goto in this > function with 'return layout'. Yep, I'll fix that. > > [...] > > > > diff --git a/drivers/mtd/nand/nand_base.c > > b/drivers/mtd/nand/nand_base.c > > index 0748a13..1107f5c1 100644 > > --- a/drivers/mtd/nand/nand_base.c > > +++ b/drivers/mtd/nand/nand_base.c > > @@ -2037,7 +2037,7 @@ static int nand_do_read_oob(struct mtd_info > > *mtd, loff_t from, > > stats = mtd->ecc_stats; > > > > if (ops->mode == MTD_OPS_AUTO_OOB) > > - len = chip->ecc.layout->oobavail; > > + len = mtd->oobavail; > > else > > len = mtd->oobsize; > > > > @@ -2728,7 +2728,7 @@ static int nand_do_write_oob(struct mtd_info > > *mtd, loff_t to, > > __func__, (unsigned int)to, (int)ops- > > >ooblen); > > > > if (ops->mode == MTD_OPS_AUTO_OOB) > > - len = chip->ecc.layout->oobavail; > > + len = mtd->oobavail; > > else > > len = mtd->oobsize; > > > [...] > > diff --git a/drivers/mtd/onenand/onenand_base.c > > b/drivers/mtd/onenand/onenand_base.c > > index 43b3392..d70bbfd 100644 > > --- a/drivers/mtd/onenand/onenand_base.c > > +++ b/drivers/mtd/onenand/onenand_base.c > > @@ -1125,7 +1125,7 @@ static int onenand_mlc_read_ops_nolock(struct > > mtd_info *mtd, loff_t from, > > (int)len); > > > > if (ops->mode == MTD_OPS_AUTO_OOB) > > - oobsize = this->ecclayout->oobavail; > > + oobsize = mtd->oobavail; > > else > > oobsize = mtd->oobsize; > > > > @@ -1230,7 +1230,7 @@ static int onenand_read_ops_nolock(struct > > mtd_info *mtd, loff_t from, > > (int)len); > > > > if (ops->mode == MTD_OPS_AUTO_OOB) > > - oobsize = this->ecclayout->oobavail; > > + oobsize = mtd->oobavail; > > else > > oobsize = mtd->oobsize; > > > > @@ -1365,7 +1365,7 @@ static int onenand_read_oob_nolock(struct > > mtd_info *mtd, loff_t from, > > ops->oobretlen = 0; > > > > if (mode == MTD_OPS_AUTO_OOB) > > - oobsize = this->ecclayout->oobavail; > > + oobsize = mtd->oobavail; > > else > > oobsize = mtd->oobsize; > > > > @@ -1887,7 +1887,7 @@ static int onenand_write_ops_nolock(struct > > mtd_info *mtd, loff_t to, > > return 0; > > > > if (ops->mode == MTD_OPS_AUTO_OOB) > > - oobsize = this->ecclayout->oobavail; > > + oobsize = mtd->oobavail; > > else > > oobsize = mtd->oobsize; > > > > @@ -2063,7 +2063,7 @@ static int onenand_write_oob_nolock(struct > > mtd_info *mtd, loff_t to, > > ops->oobretlen = 0; > > > > if (mode == MTD_OPS_AUTO_OOB) > > - oobsize = this->ecclayout->oobavail; > > + oobsize = mtd->oobavail; > > else > > oobsize = mtd->oobsize; > > This identical construction seems to occur multiple times in multiple > files. Would it make sense to create a macro for it? Right, I'll make another patch move this logic into an inline function. Thanks for the review. Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel
Re: [linux-sunxi] [PATCH 01/23] mtd: kill the ecclayout->oobavail field
On Mon, 2015-12-07 at 23:25 +0100, Boris Brezillon wrote: > ecclayout->oobavail is just redundant with the mtd->oobavail field. > Moreover, it prevents static const definition of ecc layouts since > the > NAND framework is calculating this value based on the ecclayout- > >oobfree > field. > > Signed-off-by: Boris Brezillon> --- > drivers/mtd/devices/docg3.c | 5 ++- > drivers/mtd/mtdswap.c | 16 - > drivers/mtd/nand/brcmnand/brcmnand.c | 3 -- > drivers/mtd/nand/docg4.c | 1 - > drivers/mtd/nand/hisi504_nand.c | 1 - > drivers/mtd/nand/nand_base.c | 12 +++ > drivers/mtd/onenand/onenand_base.c| 16 - > drivers/mtd/tests/oobtest.c | 49 +-- > > drivers/staging/mt29f_spinand/mt29f_spinand.c | 1 - > fs/jffs2/wbuf.c | 6 ++-- > include/linux/mtd/mtd.h | 1 - > 11 files changed, 48 insertions(+), 63 deletions(-) > [..] > > diff --git a/drivers/mtd/nand/brcmnand/brcmnand.c > b/drivers/mtd/nand/brcmnand/brcmnand.c > index 35d78f7..a906ec2 100644 > --- a/drivers/mtd/nand/brcmnand/brcmnand.c > +++ b/drivers/mtd/nand/brcmnand/brcmnand.c > @@ -845,9 +845,6 @@ static struct nand_ecclayout *brcmnand_create_layout(int > ecc_level, > break; > } > out: > - /* Sum available OOB */ > - for (i = 0; i < MTD_MAX_OOBFREE_ENTRIES_LARGE; i++) > - layout->oobavail += layout->oobfree[i].length; > return layout; > } You can get rid of the 'out' label and replace the single goto in this function with 'return layout'. [...] > > diff --git a/drivers/mtd/nand/nand_base.c > b/drivers/mtd/nand/nand_base.c > index 0748a13..1107f5c1 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -2037,7 +2037,7 @@ static int nand_do_read_oob(struct mtd_info > *mtd, loff_t from, > stats = mtd->ecc_stats; > > if (ops->mode == MTD_OPS_AUTO_OOB) > - len = chip->ecc.layout->oobavail; > + len = mtd->oobavail; > else > len = mtd->oobsize; > > @@ -2728,7 +2728,7 @@ static int nand_do_write_oob(struct mtd_info > *mtd, loff_t to, > __func__, (unsigned int)to, (int)ops- > >ooblen); > > if (ops->mode == MTD_OPS_AUTO_OOB) > - len = chip->ecc.layout->oobavail; > + len = mtd->oobavail; > else > len = mtd->oobsize; > [...] > diff --git a/drivers/mtd/onenand/onenand_base.c > b/drivers/mtd/onenand/onenand_base.c > index 43b3392..d70bbfd 100644 > --- a/drivers/mtd/onenand/onenand_base.c > +++ b/drivers/mtd/onenand/onenand_base.c > @@ -1125,7 +1125,7 @@ static int onenand_mlc_read_ops_nolock(struct > mtd_info *mtd, loff_t from, > (int)len); > > if (ops->mode == MTD_OPS_AUTO_OOB) > - oobsize = this->ecclayout->oobavail; > + oobsize = mtd->oobavail; > else > oobsize = mtd->oobsize; > > @@ -1230,7 +1230,7 @@ static int onenand_read_ops_nolock(struct > mtd_info *mtd, loff_t from, > (int)len); > > if (ops->mode == MTD_OPS_AUTO_OOB) > - oobsize = this->ecclayout->oobavail; > + oobsize = mtd->oobavail; > else > oobsize = mtd->oobsize; > > @@ -1365,7 +1365,7 @@ static int onenand_read_oob_nolock(struct > mtd_info *mtd, loff_t from, > ops->oobretlen = 0; > > if (mode == MTD_OPS_AUTO_OOB) > - oobsize = this->ecclayout->oobavail; > + oobsize = mtd->oobavail; > else > oobsize = mtd->oobsize; > > @@ -1887,7 +1887,7 @@ static int onenand_write_ops_nolock(struct > mtd_info *mtd, loff_t to, > return 0; > > if (ops->mode == MTD_OPS_AUTO_OOB) > - oobsize = this->ecclayout->oobavail; > + oobsize = mtd->oobavail; > else > oobsize = mtd->oobsize; > > @@ -2063,7 +2063,7 @@ static int onenand_write_oob_nolock(struct > mtd_info *mtd, loff_t to, > ops->oobretlen = 0; > > if (mode == MTD_OPS_AUTO_OOB) > - oobsize = this->ecclayout->oobavail; > + oobsize = mtd->oobavail; > else > oobsize = mtd->oobsize; This identical construction seems to occur multiple times in multiple files. Would it make sense to create a macro for it? Päikest, Priit Laes :) -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/23] mtd: kill the ecclayout->oobavail field
ecclayout->oobavail is just redundant with the mtd->oobavail field. Moreover, it prevents static const definition of ecc layouts since the NAND framework is calculating this value based on the ecclayout->oobfree field. Signed-off-by: Boris Brezillon--- drivers/mtd/devices/docg3.c | 5 ++- drivers/mtd/mtdswap.c | 16 - drivers/mtd/nand/brcmnand/brcmnand.c | 3 -- drivers/mtd/nand/docg4.c | 1 - drivers/mtd/nand/hisi504_nand.c | 1 - drivers/mtd/nand/nand_base.c | 12 +++ drivers/mtd/onenand/onenand_base.c| 16 - drivers/mtd/tests/oobtest.c | 49 +-- drivers/staging/mt29f_spinand/mt29f_spinand.c | 1 - fs/jffs2/wbuf.c | 6 ++-- include/linux/mtd/mtd.h | 1 - 11 files changed, 48 insertions(+), 63 deletions(-) diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c index c3a2695..e7b2e43 100644 --- a/drivers/mtd/devices/docg3.c +++ b/drivers/mtd/devices/docg3.c @@ -72,13 +72,11 @@ MODULE_PARM_DESC(reliable_mode, "Set the docg3 mode (0=normal MLC, 1=fast, " * @eccbytes: 8 bytes are used (1 for Hamming ECC, 7 for BCH ECC) * @eccpos: ecc positions (byte 7 is Hamming ECC, byte 8-14 are BCH ECC) * @oobfree: free pageinfo bytes (byte 0 until byte 6, byte 15 - * @oobavail: 8 available bytes remaining after ECC toll */ static struct nand_ecclayout docg3_oobinfo = { .eccbytes = 8, .eccpos = {7, 8, 9, 10, 11, 12, 13, 14}, .oobfree = {{0, 7}, {15, 1} }, - .oobavail = 8, }; static inline u8 doc_readb(struct docg3 *docg3, u16 reg) @@ -1438,7 +1436,7 @@ static int doc_write_oob(struct mtd_info *mtd, loff_t ofs, oobdelta = mtd->oobsize; break; case MTD_OPS_AUTO_OOB: - oobdelta = mtd->ecclayout->oobavail; + oobdelta = mtd->oobavail; break; default: return -EINVAL; @@ -1860,6 +1858,7 @@ static int __init doc_set_driver_info(int chip_id, struct mtd_info *mtd) mtd->_write_oob = doc_write_oob; mtd->_block_isbad = doc_block_isbad; mtd->ecclayout = _oobinfo; + mtd->oobavail = 8; mtd->ecc_strength = DOC_ECC_BCH_T; return 0; diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c index fc8b3d1..d330eb1 100644 --- a/drivers/mtd/mtdswap.c +++ b/drivers/mtd/mtdswap.c @@ -346,7 +346,7 @@ static int mtdswap_read_markers(struct mtdswap_dev *d, struct swap_eb *eb) if (mtd_can_have_bb(d->mtd) && mtd_block_isbad(d->mtd, offset)) return MTDSWAP_SCANNED_BAD; - ops.ooblen = 2 * d->mtd->ecclayout->oobavail; + ops.ooblen = 2 * d->mtd->oobavail; ops.oobbuf = d->oob_buf; ops.ooboffs = 0; ops.datbuf = NULL; @@ -359,7 +359,7 @@ static int mtdswap_read_markers(struct mtdswap_dev *d, struct swap_eb *eb) data = (struct mtdswap_oobdata *)d->oob_buf; data2 = (struct mtdswap_oobdata *) - (d->oob_buf + d->mtd->ecclayout->oobavail); + (d->oob_buf + d->mtd->oobavail); if (le16_to_cpu(data->magic) == MTDSWAP_MAGIC_CLEAN) { eb->erase_count = le32_to_cpu(data->count); @@ -933,7 +933,7 @@ static unsigned int mtdswap_eblk_passes(struct mtdswap_dev *d, ops.mode = MTD_OPS_AUTO_OOB; ops.len = mtd->writesize; - ops.ooblen = mtd->ecclayout->oobavail; + ops.ooblen = mtd->oobavail; ops.ooboffs = 0; ops.datbuf = d->page_buf; ops.oobbuf = d->oob_buf; @@ -945,7 +945,7 @@ static unsigned int mtdswap_eblk_passes(struct mtdswap_dev *d, for (i = 0; i < mtd_pages; i++) { patt = mtdswap_test_patt(test + i); memset(d->page_buf, patt, mtd->writesize); - memset(d->oob_buf, patt, mtd->ecclayout->oobavail); + memset(d->oob_buf, patt, mtd->oobavail); ret = mtd_write_oob(mtd, pos, ); if (ret) goto error; @@ -964,7 +964,7 @@ static unsigned int mtdswap_eblk_passes(struct mtdswap_dev *d, if (p1[j] != patt) goto error; - for (j = 0; j < mtd->ecclayout->oobavail; j++) + for (j = 0; j < mtd->oobavail; j++) if (p2[j] != (unsigned char)patt) goto error; @@ -1387,7 +1387,7 @@ static int mtdswap_init(struct mtdswap_dev *d, unsigned int eblocks, if (!d->page_buf) goto page_buf_fail; - d->oob_buf = kmalloc(2 * mtd->ecclayout->oobavail, GFP_KERNEL); + d->oob_buf = kmalloc(2 * mtd->oobavail, GFP_KERNEL);