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.
Enhancing the implementation of the rawnand_ecc_sw_* helpers to support
both cases, when the ECC object is instantiated and when it is not is a
quite elegant way to solve this situation. This way, we can still use
the existing and exported rawnand helpers while avoiding the need for
each driver to declare its own helper.
Following this change, most of the fixes sent in [2] can now be safely
reverted. Only the fsmc fix will need to be kept because there is
actually something specific to the driver to do in its ->correct()
helper.
[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>
Link: https://lore.kernel.org/linux-mtd/20210928221507.199198-3-miquel.raynal@bootlin.com
The software Hamming ECC engine stores the nsteps variable in its own
private structure while it is also exported as a public ECC field.
Let's get rid of the redundant private one and let's use the
nand_ecc_context structure when possible.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Adam Ford <aford173@gmail.com> #logicpd Torpedo
Link: https://lore.kernel.org/linux-mtd/20210127203020.9574-10-miquel.raynal@bootlin.com
This code has been written in 2008 and is fine, but in order to keep
robots happy, I think it's time to change a little bit this code just
to clarify the different possible values of eccsize_mult. Indeed, this
variable may only take the value 1 or 2 because step_size, in the case
of the software Hamming ECC engine may only be 256 or 512. Depending
on the value of eccsize_mult, an extra rp17 variable is set, or not
and triggers the following warning:
smatch warnings:
ecc_sw_hamming_calculate() error: uninitialized symbol 'rp17'.
As highlighted by Dan Carpenter, if the only possible values for
eccsize_mult are 1 and 2, then the code is fine, but "it's hard to
tell just from looking".
So instead of shifting step_size, let's use a ternary condition to
assign to eccsize_mult the only two possible values and clarify the
driver's logic.
Now that the situation is clarified for humans, ensure rp17 is
initialized to 0 to keep compilers and robots silent as well.
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20201030172333.28390-1-miquel.raynal@bootlin.com
This code is meant to be reused by the SPI-NAND core. Now that the
driver has been cleaned and reorganized, use a generic ECC engine
object to store the driver's data instead of accessing members of the
nand_chip structure. This means adding proper init/cleanup helpers.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20200929230124.31491-17-miquel.raynal@bootlin.com
Prefix by ecc_sw_hamming_ the functions which should be internal only
but are exported for "raw" operations.
Prefix by nand_ecc_sw_hamming_ the other functions which will be used
in the context of the declaration of an Hamming proper ECC engine
object.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20200929230124.31491-16-miquel.raynal@bootlin.com
Some functions should never have been exported (the ones prefixed by
__*), in this case simply drop the documentation, we never want
anybody to use this function from the outside.
For the other functions, enhance the style.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20200929230124.31491-14-miquel.raynal@bootlin.com
The include file pretends being the header for "ECC algorithm", while
it is just the header for the Hamming implementation. Make this clear
by rewording the sentence.
Do the same with the module description.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20200929230124.31491-13-miquel.raynal@bootlin.com
