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

[Heiko Schocher]

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