Re: [GIT PULL] omap plat header removal for v3.8 merge window, part1

[Arnd Bergmann <arnd@arndb.de>]

On Friday 26 October 2012, Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [121026 07:04]:
>
> > I tried running my old multiplatform scripts again and have a few
> > comments, but none of them serious:
> > 
> > $ git grep include.*mach-omap2
> > arch/arm/plat-omap/debug-devices.c:#include "../mach-omap2/debug-devices.h"
> > arch/arm/plat-omap/dma.c:#include "../mach-omap2/soc.h"
> > arch/arm/plat-omap/dmtimer.c:#include "../mach-omap2/omap-pm.h"
> > arch/arm/plat-omap/i2c.c:#include "../mach-omap2/soc.h"
> > arch/arm/plat-omap/include/plat/cpu.h:#include "../../mach-omap2/soc.h"
> > arch/arm/plat-omap/omap-pm-noop.c:#include "../mach-omap2/omap_device.h"
> > arch/arm/plat-omap/omap-pm-noop.c:#include "../mach-omap2/omap-pm.h"
> > arch/arm/plat-omap/sram.c:#include "../mach-omap2/soc.h"
> > arch/arm/plat-omap/sram.c:#include "../mach-omap2/iomap.h"
> > arch/arm/plat-omap/sram.c:#include "../mach-omap2/prm2xxx_3xxx.h"
> > arch/arm/plat-omap/sram.c:#include "../mach-omap2/sdrc.h"
> > 
> > I don't like the relative include paths too much. I would have preferred
> > adding the mach-omap2/include/mach path in the plat-omap Makefile, but
> > I suppose you want to leave it like it is now since you mention you 
> > have already built on top of it.
> 
> Well I wanted to keep them local to arch/arm/*omap*/ directories,
> and not have them exposed at all for drivers.
> 
> Other than that I don't have an issue using non-relative paths, except
> mach-omap2/include/mach traditionally has been exposed to drivers
> as the legacy #include <mach/*.h>, so IMHO local headers in the
> arch/arm/*omap*/ directories are better.
> 
> But now I wonder if we can somehow have drivers not be able to
> include these local headers?

Well, once CONFIG_MULTIPLATFORM gets enabled, the mach/*.h files are
not visible to drivers any more, but they are still visible to files
in plat-omap if you add a line like

ccflags-$(CONFIG_ARCH_OMAP2) := -I$(srctree)/arch/arm/mach-omap2/include
ccflags-$(CONFIG_ARCH_OMAP1) := -I$(srctree)/arch/arm/mach-omap1/include

to arch/arm/plat-omap/Makefile. That is how the other multiplatform
Makefiles do it. If a driver writer really wants to cheat, they can of
course do the same thing in their directory, but they can also do
#include "../../../../arch/arm/mach-omap2/foo.h"

> > sound/soc/omap/am3517evm.c:#include <mach-omap2/hardware.h>
> > sound/soc/omap/am3517evm.c:#include <mach-omap2/gpio.h>
> > sound/soc/omap/ams-delta.c:#include <mach-omap2/board-ams-delta.h>
> > sound/soc/omap/n810.c:#include <mach-omap2/hardware.h>
> > sound/soc/omap/sdp3430.c:#include <mach-omap2/hardware.h>
> > sound/soc/omap/sdp3430.c:#include <mach-omap2/gpio.h>
> > sound/soc/omap/zoom2.c:#include <mach-omap2/hardware.h>
> > sound/soc/omap/zoom2.c:#include <mach-omap2/gpio.h>
> > sound/soc/omap/zoom2.c:#include <mach-omap2/board-zoom.h>
> > 
> > Not sure if you were just missing these or if you already have other
> > patch lined up for them.
> 
> In which branch do you see the above?
> 
> I'm not seeing them in v3.7-rc2, looks like all sound/soc/omap/*.c
> files have already been fixed up.

I am looking at the arm-soc for-next branch, which is based on v3.7-rc2.

Note that before my script, this reads

#include <mach/hardware.h>

not

#include <mach-omap2/hardware.h>

but the effect is the same if the file is not available.

> In any case, let's not have #include <mach-omap1/*.h> or
> #include <mach-omap2/*.h> includes in the drivers, they are
> wrong. Those headers are meant to be local, the legacy shared
> headers should be in #include <mach/*.h>. Whatever needs to
> be passed from arch/arm/*omap*/ to drivers, should be done
> using include/linux/platform_data/*.h files.

I'm not suggesting to use mach-omap2/*.h, it's just what my
old script shows when hunting for platform header files that are
used in device drivers.

> For omap1, we might as well keep the existing ones from
> #include <mach/*.h> as they are until somebody wants to fix
> up things properly for omap1 for CONFIG_MULTIPLATFORM.

Fair enough.

	Arnd