From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Tue, 16 Dec 2014 07:21:19 +0100 Subject: [U-Boot] [PATCH] mtd: nand: revive "nand scrub" command In-Reply-To: <20141216012619.GJ20704@bill-the-cat> References: <1418334191.5581.58.camel@freescale.com> <548A88E9.5030203@denx.de> <20141215195407.0531.AA925319@jp.panasonic.com> <201412151213.19794.marex@denx.de> <1418684896.5581.76.camel@freescale.com> <20141216012619.GJ20704@bill-the-cat> Message-ID: <548FCF5F.9020809@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Tom, Scott, Am 16.12.2014 02:26, schrieb Tom Rini: > On Mon, Dec 15, 2014 at 05:08:16PM -0600, Scott Wood wrote: >> 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 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. > > As we're neck deep in "See? Scott was right" I'm inclined to go with > what Scott is saying. We tried it, it actively didn't work, lets step > back, evaluate and move along. Ok, I have no objections against that, it was just my point of view ... And yes, I missed here and there some pieces when syncing with linux 3.14 ... sorry for that, but I could not test every scenario for every board ... and we had no clean base for using git, which at last in my attempt ended in a big mess when syncing with linux ... so I started more or less from scratch ... So ... Who will send a "delete ifdef" patch? Who will test, how a resync with linux (saying 3.17 ?) work now with git? If this test is done, we can decide, which way is the "way to go" ... ? bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany