Re: [PATCH 0/2] x86: Add IMR support to Quark/Galileo

[Darren Hart <dvhart@infradead.org>]

On Wed, Dec 31, 2014 at 12:12:58PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 29, 2014 at 7:23 PM, Bryan O'Donoghue
> <pure.logic@nexus-software.ie> wrote:
> > This patchset adds an IMR driver to the kernel plus platform code for
> > Intel Galileo Gen1/Gen2 boards.
> 
> []
> 
> > Bryan O'Donoghue (2):
> >   x86: Add Isolated Memory Regions for Quark X1000
> >   platform/x86 Add Intel Galileo platform specific setup
> 
> I'm going to review this soon, but here few comments below.

Thanks for your review Andy, good advice throughout.

> 
> >  arch/x86/Kconfig                     |  23 ++
> >  arch/x86/include/asm/imr.h           |  79 ++++++
> >  arch/x86/include/asm/intel-quark.h   |  31 ++
> 
> Could it be just a quark.h? Like for ce4100.
> Those intel- prefixes in the modules looks awkward especially when
> pathname consists x86 already.
> 
> >  arch/x86/kernel/Makefile             |   1 +
> >  arch/x86/kernel/imr.c                | 529 +++++++++++++++++++++++++++++++++++
> >  drivers/platform/x86/Kconfig         |  15 +
> >  drivers/platform/x86/Makefile        |   1 +
> >  drivers/platform/x86/intel_galileo.c | 175 ++++++++++++
> 
> Here what about to make an hierarchy like:
> intel/galileo.c
> intel/mid/... would be those modules with intel_mid_ prefixes in
> future. See my proposal regarding to drivers/mfd [1]

As Bryan is only adding one file to the platform/drivers/x86, let's skip the
reorg as part of this patch series. We can consider that separately if someone
wants to make the argument that the time has come to add another directory
layer.

-- 
Darren Hart
Intel Open Source Technology Center