From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Mon, 17 Aug 2015 10:59:57 +0200 Subject: [U-Boot] [PATCH 04/15] sunxi_nand_spl: Do not bother writing the spare-area reg in syndrome mode In-Reply-To: <1439799576.3480.55.camel@hellion.org.uk> 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> Message-ID: <55D1A28D.3030607@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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. > 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