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

[Ian Campbell]

On Mon, 2015-08-17 at 10:59 +0200, Hans de Goede wrote:
> 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.

Right, that was the line of thinking which took me here too.

> > 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.

Super, thanks for the explanation. The Ack stands then.

> 
> > 
> > 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
>