| Message ID | 20170502081323.3138-2-lynxis@fe80.eu |
|---|---|
| State | New |
| Headers | show |
| Series | fixing 1bit hamming | expand |
On Tue, 2 May 2017 10:13:21 +0200 Alexander Couzens <lynxis@fe80.eu> wrote: > The old 1bit hamming layout requires the ecc data to be exact at > predefined offsets. " The old 1-bit hamming layout requires ECC data to be placed at a fixed offset, and not necessarily at the end of the OOB area. " > This can not changed because old installations > would break. " Add this old layout back in order to fix legacy setup. " > > Signed-off-by: Alexander Couzens <lynxis@fe80.eu> You should probably add Fixes and Cc:stable tags. > --- > drivers/mtd/nand/nand_base.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/nand.h | 1 + > 2 files changed, 72 insertions(+) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index b0524f8accb6..daf3df157885 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -139,6 +139,77 @@ const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = { > }; > EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops); > > +/* support the old large page layout for Hamming 1 bit where the ECC start at > + * a defined offset. Please use regular comment style: /* * <content> */ s/support/Support/ BTW, I'm not sure I understand this sentence. How about " Support the old "large page" layout used for 1-bit Hamming ECC where ECC are placed at a fixed offset. " > Should only used by old devices to compatible with old > + * layout. Use nand_ooblayout_lp_ops if possible. > + */ > +static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct nand_ecc_ctrl *ecc = &chip->ecc; > + > + if (section) > + return -ERANGE; > + > + switch (mtd->oobsize) { > + case 64: > + oobregion->offset = 40; > + break; > + case 128: > + oobregion->offset = 80; > + break; > + default: > + return -ERANGE; Hm, can you return -EINVAL instead? > + } > + > + oobregion->length = ecc->total; > + if (oobregion->offset + oobregion->length > mtd->oobsize) > + return -ERANGE; > + > + return 0; > +} > + > +static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct nand_ecc_ctrl *ecc = &chip->ecc; > + int ecc_offset = 0; > + > + if (section < 0 || section > 1) > + return -ERANGE; > + > + switch (mtd->oobsize) { > + case 64: > + ecc_offset = 40; > + break; > + case 128: > + ecc_offset = 80; > + break; > + default: > + return -ERANGE; Ditto. > + } > + > + if (section == 0) { > + /* return the space before the ecc */ Not sure this comment is needed, it's pretty obvious that this section is placed before the ECC bytes (and after the BBM). > + oobregion->offset = 2; > + oobregion->length = ecc_offset - 2; > + } else { /* section == 1 */ Drop this comment. > + /* return free space after ecc */ Ditto. > + oobregion->offset = ecc_offset + ecc->total; > + oobregion->length = mtd->oobsize - oobregion->offset; > + } > + > + return 0; > +} > + > +const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = { > + .ecc = nand_ooblayout_ecc_lp_hamming, > + .free = nand_ooblayout_free_lp_hamming, > +}; > +EXPORT_SYMBOL_GPL(nand_ooblayout_lp_hamming_ops); > + > static int check_offs_len(struct mtd_info *mtd, > loff_t ofs, uint64_t len) > { > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 9591e0fbe5bd..da33933950a5 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -914,6 +914,7 @@ struct nand_chip { > > extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops; > extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops; > +extern const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops; > > static inline void nand_set_flash_node(struct nand_chip *chip, > struct device_node *np) ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Just a nit, I prefer "mtd: nand: " instead of "mtd/nand: ", and since you have to send a v3 anyway, I thought it wouldn't be a problem to mention that ;-). On Tue, 2 May 2017 10:13:21 +0200 Alexander Couzens <lynxis@fe80.eu> wrote: > The old 1bit hamming layout requires the ecc data to be exact at > predefined offsets. This can not changed because old installations > would break. > > Signed-off-by: Alexander Couzens <lynxis@fe80.eu> > --- > drivers/mtd/nand/nand_base.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/nand.h | 1 + > 2 files changed, 72 insertions(+) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index b0524f8accb6..daf3df157885 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -139,6 +139,77 @@ const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = { > }; > EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops); > > +/* support the old large page layout for Hamming 1 bit where the ECC start at > + * a defined offset. Should only used by old devices to compatible with old > + * layout. Use nand_ooblayout_lp_ops if possible. > + */ > +static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) Another nit: try to align arguments to the open parenthesis when possible. > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct nand_ecc_ctrl *ecc = &chip->ecc; > + > + if (section) > + return -ERANGE; > + > + switch (mtd->oobsize) { > + case 64: > + oobregion->offset = 40; > + break; > + case 128: > + oobregion->offset = 80; > + break; > + default: > + return -ERANGE; > + } > + > + oobregion->length = ecc->total; > + if (oobregion->offset + oobregion->length > mtd->oobsize) > + return -ERANGE; > + > + return 0; > +} > + > +static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section, > + struct mtd_oob_region *oobregion) Ditto. > +{ > + struct nand_chip *chip = mtd_to_nand(mtd); > + struct nand_ecc_ctrl *ecc = &chip->ecc; > + int ecc_offset = 0; > + > + if (section < 0 || section > 1) > + return -ERANGE; > + > + switch (mtd->oobsize) { > + case 64: > + ecc_offset = 40; > + break; > + case 128: > + ecc_offset = 80; > + break; > + default: > + return -ERANGE; > + } > + > + if (section == 0) { > + /* return the space before the ecc */ > + oobregion->offset = 2; > + oobregion->length = ecc_offset - 2; > + } else { /* section == 1 */ > + /* return free space after ecc */ > + oobregion->offset = ecc_offset + ecc->total; > + oobregion->length = mtd->oobsize - oobregion->offset; > + } > + > + return 0; > +} > + > +const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = { > + .ecc = nand_ooblayout_ecc_lp_hamming, > + .free = nand_ooblayout_free_lp_hamming, > +}; > +EXPORT_SYMBOL_GPL(nand_ooblayout_lp_hamming_ops); Please do not export symbols if it's not really needed. > + > static int check_offs_len(struct mtd_info *mtd, > loff_t ofs, uint64_t len) > { > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 9591e0fbe5bd..da33933950a5 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -914,6 +914,7 @@ struct nand_chip { > > extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops; > extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops; > +extern const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops; Drop that line too. > > static inline void nand_set_flash_node(struct nand_chip *chip, > struct device_node *np) ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index b0524f8accb6..daf3df157885 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -139,6 +139,77 @@ const struct mtd_ooblayout_ops nand_ooblayout_lp_ops = { }; EXPORT_SYMBOL_GPL(nand_ooblayout_lp_ops); +/* support the old large page layout for Hamming 1 bit where the ECC start at + * a defined offset. Should only used by old devices to compatible with old + * layout. Use nand_ooblayout_lp_ops if possible. + */ +static int nand_ooblayout_ecc_lp_hamming(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct nand_ecc_ctrl *ecc = &chip->ecc; + + if (section) + return -ERANGE; + + switch (mtd->oobsize) { + case 64: + oobregion->offset = 40; + break; + case 128: + oobregion->offset = 80; + break; + default: + return -ERANGE; + } + + oobregion->length = ecc->total; + if (oobregion->offset + oobregion->length > mtd->oobsize) + return -ERANGE; + + return 0; +} + +static int nand_ooblayout_free_lp_hamming(struct mtd_info *mtd, int section, + struct mtd_oob_region *oobregion) +{ + struct nand_chip *chip = mtd_to_nand(mtd); + struct nand_ecc_ctrl *ecc = &chip->ecc; + int ecc_offset = 0; + + if (section < 0 || section > 1) + return -ERANGE; + + switch (mtd->oobsize) { + case 64: + ecc_offset = 40; + break; + case 128: + ecc_offset = 80; + break; + default: + return -ERANGE; + } + + if (section == 0) { + /* return the space before the ecc */ + oobregion->offset = 2; + oobregion->length = ecc_offset - 2; + } else { /* section == 1 */ + /* return free space after ecc */ + oobregion->offset = ecc_offset + ecc->total; + oobregion->length = mtd->oobsize - oobregion->offset; + } + + return 0; +} + +const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = { + .ecc = nand_ooblayout_ecc_lp_hamming, + .free = nand_ooblayout_free_lp_hamming, +}; +EXPORT_SYMBOL_GPL(nand_ooblayout_lp_hamming_ops); + static int check_offs_len(struct mtd_info *mtd, loff_t ofs, uint64_t len) { diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 9591e0fbe5bd..da33933950a5 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -914,6 +914,7 @@ struct nand_chip { extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops; extern const struct mtd_ooblayout_ops nand_ooblayout_lp_ops; +extern const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops; static inline void nand_set_flash_node(struct nand_chip *chip, struct device_node *np)
The old 1bit hamming layout requires the ecc data to be exact at predefined offsets. This can not changed because old installations would break. Signed-off-by: Alexander Couzens <lynxis@fe80.eu> --- drivers/mtd/nand/nand_base.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 1 + 2 files changed, 72 insertions(+) -- 2.12.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/