Re: [PATCH v2 1/3] mtd: nand: Cleanup/rework the atmel_nand driver
From: Alexandre Belloni
Date: Tue Feb 21 2017 - 11:12:43 EST
On 21/02/2017 at 17:55:15 +0200, Andy Shevchenko wrote:
> > And how do you do with new drivers?
>
> To be more pedantic the new drivers do not have "minus" thousands LOC.
Because all the "minus" are located in the same file (the one that
disappear), they can be safely ignored. So it is basically the same as
having a new driver.
> > I'm regularly reviewing drivers that are
> > several thousands LOC, and I don't ask people to split things just
> > because it's too long. When I ask them to split in different commits,
> > it's because they are doing several unrelated changes at once.
>
> What did prevent you to:
> 1. Introduce new driver
> 2. Switch to new driver
> 3. Remove old one.
>
> ...if you are not splitting it in the first place?
>
Having a new Kconfig symbol and switching to it, then switching to the
previous one to avoid breaking existing configurations. That's a lot of
churn for exactly 0 benefit because as said, you can safely ignore the
removed file when reviewing.
> > Note that I considered refactoring the existing driver in smaller
> > steps, but it's almost impossible, because the code is too messy and I
> > would end up with a huge series of patches that is not easier to review.
>
> I can object this, but it will be no point except waste of time to
> this discussion.
>
> It's good that you considered several options. I suppose someone who
> is on topic can do comprehensive review.
>
Maybe the NAND subsystem maintainer can review the change... oh,
wait...nevermind.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com