Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753378AbcKSS0r (ORCPT ); Sat, 19 Nov 2016 13:26:47 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:34833 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752947AbcKSS0p (ORCPT ); Sat, 19 Nov 2016 13:26:45 -0500 Date: Sat, 19 Nov 2016 19:26:39 +0100 From: Boris Brezillon To: Masahiro Yamada Cc: linux-mtd@lists.infradead.org, Alan Cox , David Woodhouse , Jason Roberts , Chuanxiao Dong , Dinh Nguyen , Linux Kernel Mailing List , Marek Vasut , Brian Norris , Richard Weinberger , David Woodhouse , Cyrille Pitchen Subject: Re: [PATCH 00/11] mtd: nand: denali: first round of cleanups of Denali NAND driver Message-ID: <20161119192639.72f32c2a@bbrezillon> In-Reply-To: References: <1478666130-13413-1-git-send-email-yamada.masahiro@socionext.com> <20161119093011.17b53356@bbrezillon> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2783 Lines: 72 Hi Masahiro, On Sun, 20 Nov 2016 01:15:05 +0900 Masahiro Yamada wrote: > Hi Boris, > > > 2016-11-19 17:30 GMT+09:00 Boris Brezillon : > > Hi Masahiro, > > > > On Wed, 9 Nov 2016 13:35:19 +0900 > > Masahiro Yamada 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