From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Date: Mon, 17 Aug 2015 12:28:16 +0100 Subject: [U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode In-Reply-To: <55D1A28D.3030607@redhat.com> References: <1439668968-3882-1-git-send-email-hdegoede@redhat.com> <1439668968-3882-5-git-send-email-hdegoede@redhat.com> <1439799576.3480.55.camel@hellion.org.uk> <55D1A28D.3030607@redhat.com> Message-ID: <1439810896.3480.72.camel@hellion.org.uk> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > 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@hellion.org.uk > > > > > Thanks for all the reviews. > > Regards, > > Hans >