linux-kernel - Re: [PATCH 00/11] mtd: nand: denali: first round of cleanups of Denali NAND driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20161119192639.72f32c2a@bbrezillon>
Date:   Sat, 19 Nov 2016 19:26:39 +0100
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     linux-mtd@...ts.infradead.org, Alan Cox <alan@...ux.intel.com>,
        David Woodhouse <David.Woodhouse@...el.com>,
        Jason Roberts <jason.e.roberts@...el.com>,
        Chuanxiao Dong <chuanxiao.dong@...el.com>,
        Dinh Nguyen <dinguyen@...era.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Marek Vasut <marek.vasut@...il.com>,
        Brian Norris <computersforpeace@...il.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Cyrille Pitchen <cyrille.pitchen@...el.com>
Subject: Re: [PATCH 00/11] mtd: nand: denali: first round of cleanups of
 Denali NAND driver

Hi Masahiro,

On Sun, 20 Nov 2016 01:15:05 +0900
Masahiro Yamada <yamada.masahiro@...ionext.com> wrote:

> Hi Boris,
> 
> 
> 2016-11-19 17:30 GMT+09:00 Boris Brezillon <boris.brezillon@...e-electrons.com>:
> > Hi Masahiro,
> >
> > On Wed,  9 Nov 2016 13:35:19 +0900
> > Masahiro Yamada <yamada.masahiro@...ionext.com> wrote:
> >  
> >> I am tackling on this driver to use it for my SoCs.
> >> The difficulty is a bunch of platform specific stuff
> >> (more specifically, Intel MRST specific) is hard-coded in this driver.
> >>
> >> I need lots of rework to utilize the driver for generic cases,
> >> but at the same time, I found the driver code is really dirty,
> >> lots of unused code, odd comments, etc.
> >>
> >> The first thing I needed to do was to clean up the code.
> >> My work is still under the way, but I decided to drop this series
> >> for now.  I hope this series is easy to review, so I guess
> >> splitting into a small chunks is better than a one-shot patch bomb.  
> >
> > Well, all I've seen coming from you so far are minor cleanups and
> > cosmetic changes (including one introducing a regression).
> > I'll apply this series, but please, next time, try to send the whole
> > series, so that we can see the big picture, and not just random
> > cleanups.
> >
> > Thanks,
> >
> > Boris  
> 
> Right, this series alone is not useful at all.
> 
> I've been mostly stuck in some local works this week,
> but hopefully I will find some time to complete the series next week.
> 
> If you want to see the big picture,
> you can postpone this series until then.
> 

I already applied the series. Looking forward to the next steps ;-).

Sorry if I've been harsh to you when saying that your contributions
were 'useless'. It's just that we regularly receive random 'cleanups'
(generated with coccinelle, or similar tools), and, while these kind of
things help getting consistent code across the kernel, or
simplifying/clarifying existing code, it's not the kind of rework that
really improve the framework or address drivers flaws.

It's also sometime used by developers who just want to have patches in
mainline, and never reach the next step (contribute things to really
improve a framework or add support for new features/hardware).
Another reason I'm reluctant to apply such cleanup changes is that,
sometime a simple change might actually break things (see the 'mtd:
nand: denali_pci: add missing pci_release_regions() calls' patch).

And the last aspect is that cosmetic changes pollutes other
contributions by delaying their review.

Don't mistake me, if all these cleanups are serving a higher purpose (in
your case, adding support for a new IP revision in a clean way), then
that's all fine. But otherwise, I tend to dequeue those patches last.

Regards,

Boris