[U-Boot] [PATCH] mtd: nand: revive "nand scrub" command

[Scott Wood]

On Mon, 2014-12-15 at 12:13 +0100, Marek Vasut wrote:
> On Monday, December 15, 2014 at 11:54:08 AM, Masahiro Yamada wrote:
> > On Fri, 12 Dec 2014 07:19:21 +0100
> > 
> > Heiko Schocher <hs at denx.de> wrote:
> > > Hello Scott,
> > > 
> > > Am 11.12.2014 22:43, schrieb Scott Wood:
> > > > On Thu, 2014-12-11 at 22:37 +0100, Marek Vasut wrote:
> > > >> On Thursday, December 11, 2014 at 09:37:10 PM, Scott Wood wrote:
> > > >>> On Thu, 2014-12-11 at 19:49 +0900, Masahiro Yamada wrote:
> > > >>>> Since commit ff94bc40af34 (mtd, ubi, ubifs: resync with Linux-3.14),
> > > >>>> the "nand scrub" command has not been working.
> > > >>>> 
> > > >>>> The scrub is a U-Boot extension and we have to modify nand_base.c
> > > >>>> that originates in Linux.
> > > >>>> 
> > > >>>> Mark the code with #ifdef __UBOOT__ so we will never accidentally
> > > >>>> drop it when we re-sync the NAND framework with Linux in the future.
> > > >>> 
> > > >>> No more "#ifdef __UBOOT__" please.
> > > >> 
> > > >> Do you happen to have a helpful suggestion how to clearly mark those
> > > >> bits of code then please ?
> > > > 
> > > > This was already discussed. :-)
> > > > 
> > > > See the archives for why I think this is bad.
> > > > 
> > > >>> Instead, never again do a "start
> > > >>> from scratch" resync the way that the above commit was done.
> > > >> 
> > > >> This was already discussed, no need to revive this topic here now.
> > > > 
> > > > Sorry, but these patches fixing breakages that resulted from that merge
> > > > demonstrate that there is a need to revive it, if there's anyone that
> > > > still thinks it's a good idea -- Heiko seemed to be in agreement that
> > > > there's no need to do that for future syncs:
> > > > http://lists.denx.de/pipermail/u-boot/2014-November/194256.html
> > > 
> > > Yes, I hope a resync works now fine ... but I prefer to mark the
> > > differences between linux and u-boot somehow, because, you immediately
> > > see the differences between linux and u-boot, when you read the u-boot
> > > code ...
> > 
> > I agree with Heiko.
> 
> I second that.

If you or Heiko want to take over NAND custodianship, the offer is still
there.

I find these ifdefs to be harmful to code legibility, editability, and
searchability.

I find these ifdefs to be harmful to merging from Linux, especially when
the ifndef section is large enough that the ifndef itself doesn't
conflict with the diff context.  This can lead to silent mismerges if
the U-Boot alternative needed to be updated.

I find these ifdefs to be unreliable, because not every change is going
to end up getting marked (e.g. commit 35c204d8a9d0).  This can be
actively harmful if people see some changes marked, and rely on the
markers rather than checking with a diff.  And yes, that means I need to
send a patch to strip out the ifdefs from NAND files, just like I did
back in commit 5b8e6bb517ea4.

-Scott