[U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode

Hans de Goede hdegoede at redhat.com
Mon Aug 17 10:59:57 CEST 2015


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 at 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 at hellion.org.uk    >
>

Thanks for all the reviews.

Regards,

Hans


More information about the U-Boot mailing list