Hi,
On 17-08-15 10:19, Ian Campbell wrote:
On Sat, 2015-08-15 at 22:02 +0200, Hans de Goede wrote:
In syndrome mode we set the NFC_SEQ bit in the command register, so the spare-area register is not used. Also the value currently being written is actual wrong, the ecc sits at "column + CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE" not just CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE.
So the current code only serves to confuse the user -> remove it.
Signed-off-by: Hans de Goede hdegoede@redhat.com
There's a bunch of other uses of the syndrome parameter in this function. Does syndrome=true work even without this particular bit of code?
Yes, since in syndrome we are passing in the NFC_SEQ bit in the command register, which tells the controller that the ecc data is directly behind the actual data, the spare-area address register is not used.
I stumbled over this because the code we had was writing the wrong address for the ecc data to the spare-area address register, leading me to wonder why syndrome mode was working at all.
I suppose I'm asking, should the paramter and the other uses be removed? Or should an ASSERT(!syndrome) be added, or am I worrying about nothing and everything is just fine as it is after this patch?
Everything is just fine after this patch, actually it was fine before this patch except that it did an unnecessary write, with the wrong ecc data address which I found confusing, hence this patch to remove that write.
I suspect the latter, so if that is indeed the case: Acked-by: Ian Campbell < ijc@hellion.org.uk >
Thanks for all the reviews.
Regards,
Hans