Re: [PATCH][v3] mtd/ifc: Add support for IFC controller version 2.0

[Brian Norris <computersforpeace@gmail.com>]

Hi,

On Wed, Jun 29, 2016 at 02:53:03PM +0000, Raghav Dogra wrote:
> 
> 
> > -----Original Message-----
> > From: Leo Li [mailto:pku.leo@gmail.com]
> > Sent: Saturday, May 28, 2016 3:34 AM

1 month delay? So much for the rush...

> > To: Brian Norris <computersforpeace@gmail.com>; Raghav Dogra
> > <raghav.dogra@nxp.com>
> > Cc: Boris Brezillon <boris.brezillon@free-electrons.com>; Yang-Leo Li
> > <leoyang.li@nxp.com>; Prabhakar Kushwaha
> > <prabhakar.kushwaha@nxp.com>; Scott Wood <oss@buserror.net>; linux-
> > mtd@lists.infradead.org; linuxppc-dev <linuxppc-dev@lists.ozlabs.org>;
> > Raghav Dogra <raghav@freescale.com>; Jaiprakash Singh
> > <b44839@freescale.com>
> > Subject: Re: [PATCH][v3] mtd/ifc: Add support for IFC controller version 2.0
> > 
> > On Fri, May 27, 2016 at 4:12 PM, Brian Norris
> > <computersforpeace@gmail.com> wrote:
> > > On Fri, May 27, 2016 at 10:44:01PM +0200, Boris Brezillon wrote:
> > >> On Fri, 27 May 2016 15:15:00 -0500
> > >> Leo Li <pku.leo@gmail.com> wrote:
> > >> >
> > >> > The pull request does have patch "mtd/ifc: Add support for IFC
> > >> > controller version 2.0", but it doesn't have another patch
> > >> > "driver/memory: Update dependency of IFC for
> > >> > Layerscape"(https://patchwork.ozlabs.org/patch/557389/) needed to
> > >> > make the driver selectable on new hardware.
> > >
[...]

> > >> BTW, in the patch description you say you're only modifying a Kconfig
> > >> dependency, but you're actually doing more than that: you're removing
> > >> an asm header inclusion and manually include several other headers
> > >> (which I guess were previously included by asm/prom.h).
> > >
> > > Please resend this patch with a more complete commit description; I'd
> > > like it to get actual review (and time in linux-next) before it gets
> > > merged, so at best, it'll wait a few -rc's. I also suspect the patch
> > > isn't optimal. I believe Scott has suggested [1] that we didn't need
> > > the FSL_SOC dependency on the LBC driver. I think IFC looks like a
> > > similar case?
> 
> Hi Brian,
> 
> The patch being talked about does not add a FSL_SOC dependency on the IFC driver.
> It uses a generic ARCH_LAYERSCAPE macro to enable IFC. This should be Ok? 

Maybe... but if we know that this driver doesn't actually have an
FSL_SOC dependency, and the FSL maintainers don't really want it in the
first place, then a simpler patch is to just remove the FSL_SOC
dependency, rather than making the deps more complicated.

But anyway, if you resend with the comments addresses (e.g., better
commit description), then we can consider applying it. If the FSL folks
have nothing to contribute here, then I don't see why we wouldn't take
your patch.

Regards,
Brian