| Message ID | 20210928221507.199198-6-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup series about Hamming helpers | expand |
On Tue, 2021-09-28 at 22:15:04 UTC, Miquel Raynal wrote: > This reverts commit 46fcb57e6b7283533ebf8ba17a6bd30fa88bdc9f. > > Before the introduction of the ECC framework infrastructure, many > drivers used the ->calculate/correct() Hamming helpers directly. The > point of this framework was to avoid this kind of hackish calls and use a > proper and generic API but it is true that in certain cases, drivers > still need to use these helpers in order to do ECC computations on > behalf of their limited hardware. > > Right after the introduction of the ECC engine core introduction, it was > spotted that it was not possible to use the shiny rawnand software ECC > helpers so easily because an ECC engine object should have been > allocated and initialized first. While this works well in most cases, > for these drivers just leveraging the power of a single helper in > conjunction with some pretty old and limited hardware, it did not fit. > > The idea back then was to declare intermediate helpers which would make > use of the exported software ECC engine bare functions while keeping the > rawnand layer compatibility. As there was already functions with the > rawnand_sw_hamming_ prefix it was decided to declare new local helpers > for this purpose in each driver needing one. > > Besides being far from optimal, this design choice was blamed by Linus > when he pulled the "fixes" pull request [1] so that is why now it is > time to clean this mess up. > > The implementation of the rawnand_ecc_sw_* helpers has now been enhanced > to support both cases, when the ECC object is instantiated and when it is > not. This way, we can still use the existing and exported rawnand > helpers while avoiding the need for each driver to declare its own > helper, thus this fix from [2] can now be safely reverted. > > [1] https://lore.kernel.org/lkml/CAHk-=wh_ZHF685Fni8V9is17mj=pFisUaZ_0=gq6nbK+ZcyQmg@mail.gmail.com/ > [2] https://lore.kernel.org/linux-mtd/20210413161840.345208-1-miquel.raynal@bootlin.com > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next. Miquel
diff --git a/drivers/mtd/nand/raw/sharpsl.c b/drivers/mtd/nand/raw/sharpsl.c index 2f1fe464e663..5612ee628425 100644 --- a/drivers/mtd/nand/raw/sharpsl.c +++ b/drivers/mtd/nand/raw/sharpsl.c @@ -11,7 +11,6 @@ #include <linux/module.h> #include <linux/delay.h> #include <linux/mtd/mtd.h> -#include <linux/mtd/nand-ecc-sw-hamming.h> #include <linux/mtd/rawnand.h> #include <linux/mtd/partitions.h> #include <linux/mtd/sharpsl.h> @@ -97,15 +96,6 @@ static int sharpsl_nand_calculate_ecc(struct nand_chip *chip, return readb(sharpsl->io + ECCCNTR) != 0; } -static int sharpsl_nand_correct_ecc(struct nand_chip *chip, - unsigned char *buf, - unsigned char *read_ecc, - unsigned char *calc_ecc) -{ - return ecc_sw_hamming_correct(buf, read_ecc, calc_ecc, - chip->ecc.size, false); -} - static int sharpsl_attach_chip(struct nand_chip *chip) { if (chip->ecc.engine_type != NAND_ECC_ENGINE_TYPE_ON_HOST) @@ -116,7 +106,7 @@ static int sharpsl_attach_chip(struct nand_chip *chip) chip->ecc.strength = 1; chip->ecc.hwctl = sharpsl_nand_enable_hwecc; chip->ecc.calculate = sharpsl_nand_calculate_ecc; - chip->ecc.correct = sharpsl_nand_correct_ecc; + chip->ecc.correct = rawnand_sw_hamming_correct; return 0; }
This reverts commit 46fcb57e6b7283533ebf8ba17a6bd30fa88bdc9f. Before the introduction of the ECC framework infrastructure, many drivers used the ->calculate/correct() Hamming helpers directly. The point of this framework was to avoid this kind of hackish calls and use a proper and generic API but it is true that in certain cases, drivers still need to use these helpers in order to do ECC computations on behalf of their limited hardware. Right after the introduction of the ECC engine core introduction, it was spotted that it was not possible to use the shiny rawnand software ECC helpers so easily because an ECC engine object should have been allocated and initialized first. While this works well in most cases, for these drivers just leveraging the power of a single helper in conjunction with some pretty old and limited hardware, it did not fit. The idea back then was to declare intermediate helpers which would make use of the exported software ECC engine bare functions while keeping the rawnand layer compatibility. As there was already functions with the rawnand_sw_hamming_ prefix it was decided to declare new local helpers for this purpose in each driver needing one. Besides being far from optimal, this design choice was blamed by Linus when he pulled the "fixes" pull request [1] so that is why now it is time to clean this mess up. The implementation of the rawnand_ecc_sw_* helpers has now been enhanced to support both cases, when the ECC object is instantiated and when it is not. This way, we can still use the existing and exported rawnand helpers while avoiding the need for each driver to declare its own helper, thus this fix from [2] can now be safely reverted. [1] https://lore.kernel.org/lkml/CAHk-=wh_ZHF685Fni8V9is17mj=pFisUaZ_0=gq6nbK+ZcyQmg@mail.gmail.com/ [2] https://lore.kernel.org/linux-mtd/20210413161840.345208-1-miquel.raynal@bootlin.com Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/sharpsl.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)