Message-ID: <20151218220340.GP10460@google.com>
Date:	Fri, 18 Dec 2015 14:03:40 -0800
From:	Brian Norris <computersforpeace@...il.com>
To:	Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:	David Woodhouse <dwmw2@...radead.org>,
	linux-mtd@...ts.infradead.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
	Hartley Sweeten <hsweeten@...ionengravers.com>,
	Ryan Mallon <rmallon@...il.com>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <kernel@...gutronix.de>,
	Imre Kaloz <kaloz@...nwrt.org>,
	Krzysztof Halasa <khalasa@...p.pl>,
	Tony Lindgren <tony@...mide.com>, linux-omap@...r.kernel.org,
	Alexander Clouter <alex@...riz.org.uk>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Jason Cooper <jason@...edaemon.net>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Andrew Lunn <andrew@...n.ch>, Daniel Mack <daniel@...que.org>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Robert Jarzmik <robert.jarzmik@...e.fr>,
	Marek Vasut <marek.vasut@...il.com>,
	Steven Miao <realmz6@...il.com>,
	adi-buildroot-devel@...ts.sourceforge.net,
	Mikael Starvik <starvik@...s.com>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	linux-cris-kernel@...s.com, Josh Wu <josh.wu@...el.com>,
	Wan ZongShun <mcuos.com@...il.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Maxim Levitsky <maximlevitsky@...il.com>,
	Kukjin Kim <kgene@...nel.org>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	linux-samsung-soc@...r.kernel.org,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Chen-Yu Tsai <wens@...e.org>, linux-sunxi@...glegroups.com,
	Stefan Agner <stefan@...er.ch>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	devel@...verdev.osuosl.org
Subject: Re: [PATCH v4 00/58]  mtd: nand: refactor the NAND subsystem (part 1)

Hi,

On Thu, Dec 10, 2015 at 08:59:44AM +0100, Boris Brezillon wrote:
> Hello,
> 
> This huge series aims at clarifying the relationship between the mtd and
> nand_chip structures and hiding NAND framework internals to NAND
> controller drivers.
> 
> The first part of the series (patch 1 to 4) is a set of fixes/simple
> reworks easing the migration to mtd_to_nand().
> 
> The second part of the series embeds the mtd structure into the nand_chip
> one so that NAND controller drivers don't have to bother allocating the
> MTD device and linking it with the NAND chip.
> 
> The last part of the series hides accesses to the chip->priv field behind
> two helper functions.
> 
> This allows removal of some of the boilerplate code done in all NAND
> controller drivers, but most importantly, it unifies a bit the way NAND
> chip structures are instantiated (even though we still have two different
> kinds of drivers: those embedding the nand_chip struct into their private
> nand chip representation, and those allocating two different structures
> and linking them together with the chip->priv field).
> 
> As said in the title, this refactoring is only the first step. I plan to
> rework the NAND controller / NAND chip separation for pretty much the same
> reasons: clarifying the separation between the two concepts, and getting
> rid of more boilerplate code in NAND controller drivers.
> 
> Stay tuned ;-).
> 
> Best Regards,
> 
> Boris

Thanks a lot for all the work here! Nice cleanups. I think I've pushed
everything correctly to l2-mtd.git. Please verify that things look sane
to you, if possible. There were a few v5's thrown in after the fact, as
well as other cleanups that required apply-conflicts.

I'll summarize below anything I remember that's changed since the cover
letter, for posterity.

> Changes since v3:
> - fix some bugs introduced when migrating to nand_to_mtd()
> - split the huge commit switching all drivers to nand_to_mtd() into several
>   commits (one per driver) to ease review and integration
> - add a simple fixes/reworks at the beginning of the series (mainly to
>   ease migration to nand_to_mtd())
> - drop already applied patches.
> 
> Changes since v2:
> - fix some build warnings/erros
> 
> Changes since v1:
> - dropped already applied patches
> - fixed some typos
> - manually fixed some modifications omitted by the coccinelle scripts
> - manually reworked modifactions done by coccinelle scripts to improve
>   readability and fix coding style issues
> 
> *** BLURB HERE ***

Nice blurb :)

> 
> Boris Brezillon (58):
>   mtd: nand: denali: add missing nand_release() call in denali_remove()

You sent v5 of this, and I added in the comment that was
written/requested afterward. That caused some small conflicts later.

>   mtd: nand: fsmc: create and use mtd_to_fsmc()
>   mtd: nand: nuc900: create and use mtd_to_nuc900()
>   mtd: nand: omap2: create and use mtd_to_omap()
>   mtd: nand: ams-delta: use the mtd instance embedded in struct
>     nand_chip
>   mtd: nand: atmel: use the mtd instance embedded in struct nand_chip
>   mtd: nand: au1550nd: use the mtd instance embedded in struct nand_chip
>   mtd: nand: bcm47xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: bf5xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: brcm: use the mtd instance embedded in struct nand_chip
>   mtd: nand: cafe: use the mtd instance embedded in struct nand_chip
>   mtd: nand: cmx270: use the mtd instance embedded in struct nand_chip

^^ I dropped one comment in this one

>   mtd: nand: cs553x: use the mtd instance embedded in struct nand_chip
>   mtd: nand: davinci: use the mtd instance embedded in struct nand_chip
>   mtd: nand: denali: use the mtd instance embedded in struct nand_chip

You sent v5, but there were still trivial conflicts.

>   mtd: nand: diskonchip: use the mtd instance embedded in struct
>     nand_chip
>   mtd: nand: docg4: use the mtd instance embedded in struct nand_chip

I cleaned some things up after this one, causing more trivial conflicts
later in the controller_data accessor patches.

>   mtd: nand: fsl_elbc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: fsl_ifc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: fsl_upm: use the mtd instance embedded in struct nand_chip
>   mtd: nand: fsmc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: gpio: use the mtd instance embedded in struct nand_chip
>   mtd: nand: gpmi: use the mtd instance embedded in struct nand_chip
>   mtd: nand: hisi504: use the mtd instance embedded in struct nand_chip
>   mtd: nand: jz4740: use the mtd instance embedded in struct nand_chip
>   mtd: nand: lpc32xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: mpc5121: use the mtd instance embedded in struct nand_chip
>   mtd: nand: mxc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: nandsim: use the mtd instance embedded in struct nand_chip
>   mtd: nand: ndfc: use the mtd instance embedded in struct nand_chip
>   mtd: nand: nuc900: use the mtd instance embedded in struct nand_chip
>   mtd: nand: omap2: use the mtd instance embedded in struct nand_chip
>   mtd: nand: orion: use the mtd instance embedded in struct nand_chip
>   mtd: nand: pasemi: use the mtd instance embedded in struct nand_chip
>   mtd: nand: plat: use the mtd instance embedded in struct nand_chip
>   mtd: nand: pxa3xx: use the mtd instance embedded in struct nand_chip
>   mtd: nand: r852: use the mtd instance embedded in struct nand_chip
>   mtd: nand: s3c2410: use the mtd instance embedded in struct nand_chip
>   mtd: nand: sh_flctl: use the mtd instance embedded in struct nand_chip
>   mtd: nand: sharpsl: use the mtd instance embedded in struct nand_chip
>   mtd: nand: socrates: use the mtd instance embedded in struct nand_chip
>   mtd: nand: sunxi: use the mtd instance embedded in struct nand_chip
>   mtd: nand: tmio: use the mtd instance embedded in struct nand_chip
>   mtd: nand: txx9ndfmc: use the mtd instance embedded in struct
>     nand_chip
>   mtd: nand: vf610: use the mtd instance embedded in struct nand_chip
>   mtd: nand: update the documentation to reflect framework changes
>   staging: mt29f_spinand: use the mtd instance embedded in struct
>     nand_chip
>   cris: nand: use the mtd instance embedded in struct nand_chip
>   mtd: nand: update mtd_to_nand()
>   mtd: nand: remove useless mtd->priv = chip assignments
>   cris: nand: remove useless mtd->priv = chip assignments
>   staging: mt29f_spinand: remove useless mtd->priv = chip assignment
>   mtd: nand: simplify nand_dt_init() usage
>   mtd: nand: kill the chip->flash_node field
>   mtd: nand: add helpers to access ->priv

I have some comments on this one, so I'm not pushing the last 4 patches
for now.

>   ARM: make use of nand_set/get_controller_data() helpers

BTW, I see that plat_nand.c and arch/arm/mach-ixp4xx/ixdp425-setup.c are
making conflicting use of the ->priv field. You've essentially fixed the
conflict in:

  [PATCH] mtd: nand: remove unused and buggy get_platform_nandchip() helper function
  http://lists.infradead.org/pipermail/linux-mtd/2015-December/064409.html

But really, that's a pretty big hack still... I don't care to fix it
myself right now. Just whining :)

>   mtd: nand: make use of nand_set/get_controller_data() helpers
>   staging: mt29f_spinand: make use of nand_set/get_controller_data()
>     helpers

[...]

So, this has passed all my build tests and LGTM. Pushed patches 1 - 54
to l2-mtd.git!

Much thanks,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists