sizes.h: move from asm-generic to <linux/sizes.h>

sizes.h is used throughout the AMBA code and drivers, so the header should be available to everyone in order to driver AMBA/PrimeCell peripherals behind a PCI bridge where the host can be any platform (I'm doing it under x86).

At this step <asm-generic/sizes.h> includes <linux/sizes.h>, to allow a grace period for both in-tree and out-of-tree drivers.

---
 include/asm-generic/sizes.h |   49 +-----------------------------------------
 include/linux/sizes.h       |   47 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 47 deletions(-)
 create mode 100644 include/linux/sizes.h

index ea5d4ef..1dcfad9 100644
@@ -1,47 +1,2 @@
-/*
- * linux/include/asm-generic/sizes.h
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#ifndef __ASM_GENERIC_SIZES_H__
-#define __ASM_GENERIC_SIZES_H__
-
-#define SZ_1				0x00000001
-#define SZ_2				0x00000002
-#define SZ_4				0x00000004
-#define SZ_8				0x00000008
-#define SZ_16				0x00000010
-#define SZ_32				0x00000020
-#define SZ_64				0x00000040
-#define SZ_128				0x00000080
-#define SZ_256				0x00000100
-#define SZ_512				0x00000200
-
-#define SZ_1K				0x00000400
-#define SZ_2K				0x00000800
-#define SZ_4K				0x00001000
-#define SZ_8K				0x00002000
-#define SZ_16K				0x00004000
-#define SZ_32K				0x00008000
-#define SZ_64K				0x00010000
-#define SZ_128K				0x00020000
-#define SZ_256K				0x00040000
-#define SZ_512K				0x00080000
-
-#define SZ_1M				0x00100000
-#define SZ_2M				0x00200000
-#define SZ_4M				0x00400000
-#define SZ_8M				0x00800000
-#define SZ_16M				0x01000000
-#define SZ_32M				0x02000000
-#define SZ_64M				0x04000000
-#define SZ_128M				0x08000000
-#define SZ_256M				0x10000000
-#define SZ_512M				0x20000000
-
-#define SZ_1G				0x40000000
-#define SZ_2G				0x80000000
-
-#endif /* __ASM_GENERIC_SIZES_H__ */
+/* This is a placeholder, to be removed over time */
+#include <linux/sizes.h>

1.7.7.2

---
 drivers/amba/bus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index b7e7285..e8eb91b 100644
@@ -16,9 +16,9 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/amba/bus.h>
+#include <linux/sizes.h>
 
 #include <asm/irq.h>
-#include <asm/sizes.h>
 
 #define to_amba_driver(d)	container_of(d, struct amba_driver, drv)
 

1.7.7.2

---
 arch/arm/include/asm/memory.h |    2 +-
 arch/arm/mm/dma-mapping.c     |    2 +-
 arch/arm/mm/init.c            |    2 +-
 arch/arm/mm/ioremap.c         |    2 +-
 arch/arm/mm/mmu.c             |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

index fcb5757..e965f1b 100644
@@ -16,7 +16,7 @@
 #include <linux/compiler.h>
 #include <linux/const.h>
 #include <linux/types.h>
-#include <asm/sizes.h>
+#include <linux/sizes.h>
 
 #ifdef CONFIG_NEED_MACH_MEMORY_H
 #include <mach/memory.h>

index ea6b431..30a031c 100644
@@ -23,12 +23,12 @@
 #include <linux/slab.h>
 #include <linux/iommu.h>
 #include <linux/vmalloc.h>
+#include <linux/sizes.h>
 
 #include <asm/memory.h>
 #include <asm/highmem.h>
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
-#include <asm/sizes.h>
 #include <asm/mach/arch.h>
 #include <asm/dma-iommu.h>
 #include <asm/mach/map.h>

index c21d06c..ad7fd8a 100644
@@ -21,13 +21,13 @@
 #include <linux/gfp.h>
 #include <linux/memblock.h>
 #include <linux/dma-contiguous.h>
+#include <linux/sizes.h>
 
 #include <asm/mach-types.h>
 #include <asm/memblock.h>
 #include <asm/prom.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
-#include <asm/sizes.h>
 #include <asm/tlb.h>
 #include <asm/fixmap.h>
 

index 4f55f50..566750f 100644
@@ -25,6 +25,7 @@
 #include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/io.h>
+#include <linux/sizes.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>

@@ -32,7 +33,6 @@
 #include <asm/mmu_context.h>
 #include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
-#include <asm/sizes.h>
 #include <asm/system_info.h>
 
 #include <asm/mach/map.h>

index e5dad60..2196116 100644
@@ -16,13 +16,13 @@
 #include <linux/memblock.h>
 #include <linux/fs.h>
 #include <linux/vmalloc.h>
+#include <linux/sizes.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
 #include <asm/sections.h>
 #include <asm/cachetype.h>
 #include <asm/setup.h>
-#include <asm/sizes.h>
 #include <asm/smp_plat.h>
 #include <asm/tlb.h>
 #include <asm/highmem.h>

1.7.7.2

---
 drivers/tty/serial/amba-pl011.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 4ad721f..d394b93 100644
@@ -53,9 +53,9 @@
 #include <linux/delay.h>
 #include <linux/types.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/sizes.h>
 
 #include <asm/io.h>
-#include <asm/sizes.h>
 
 #define UART_NR			14
 

1.7.7.2

The sta2x11 I/O Hub is a bridge from PCIe to AMBA. It reuses a number of amba drivers and needs to activate core bus support.

---
 arch/x86/Kconfig |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

index 91dea918..112718f 100644
@@ -500,6 +500,7 @@ config STA2X11
 	select SWIOTLB
 	select MFD_STA2X11
 	select ARCH_REQUIRE_GPIOLIB
+	select ARM_AMBA
 	default n
 	---help---
 	  This adds support for boards based on the STA2X11 IO-Hub,

@@ -2173,6 +2174,9 @@ config GEOS
 
 endif # X86_32
 
+config ARM_AMBA
+        bool
+
 config AMD_NB
 	def_bool y
 	depends on CPU_SUP_AMD && PCI

1.7.7.2

Ok. I apologize for not checking this. Actually, I only fixed the ones that I needed to compile.

How should I address the problem? (original code, published on sourceforge was simply replicating a number of amba drivers into pci drivers, but I don't think massive code duplication is ever sensible, thus I preferred to reuse the existing drivers).

I doubt all amba drivers can easily be made x86-compatible. I see these possible approaces, all of them with their cons

1- add STA2X11 dependencies to ARM_AMBA _and_ the drivers that do work

2- leave CONFIG_ARM_AMBA undefined and play select file in Makefiles
using obj-$(CONFIG_STA2X11)

3- add ARM dependencies to the drivers that do not compile on x86

any other option?

(1) and (2) bring chipset-specific stuff into the amba world, which is bad, especially since I suspect other similar bridges will appear soon.

(3) looks like a temporary hack and stuff can be fixed over time, but it has the disadvantage of adding a big number of needless drivers in the dsitributions' packages -- like my nacked patch did.

thanks
/alessandro

I think the answer is... those primecell drivers need fixing in some way. Re-defining CS, DS and ES in drivers is rather silly given that they're x86 segment register names - so if PL330 can be fixed to make its names more specific, that sorts it out.

As for PL08x, which depends on asm/hardware/pl080.h, I think that's going to have to be a config dependency - it would be nice to move that header into drivers/dma and make it private to the amba-pl08x driver, but we have definitions in there which platforms supply to the pl08x driver via platform data. I'd rather not stuff the register definitions into include/linux/platform_data/...

Note also that hpa has only reported the first errors he encountered, so there may be more, and that's somehting which needs checking.

Ok. I'm going to attack them and make a report.

We have pl080 under PCI, so we have patches in the queue to fix this header problem. I just didn't submit them yet, but now they are part of this fixup effort and I'll check them over.

Yes. I'm going to make a status report soon, counting and classifying all errors I encounter when enabling all amba drivers under x86.

thanks
/alessandro

Note that I have a *big* pile of changes to the PL08x stuff sitting in my tree - which is waiting for DMA (and OMAP specifically) to stabilize before I push them out into linux-next. They've been posted to the ARM and OMAP lists several times already in the last month or two.

I am OK prefixing the regs with PL330_ or somesuch.

The CS, DS and ES regs were named so as to match exactly the terminology of the PL330 manual. So apparently even the ARM Ltd didn't imagine ARM and X86 galaxies colliding so soon :)

Regards.

Whatever it says in documentation is neither here nor there. We're humans, we can re-interpret, and we are capable of adding prefixes to names when they're stupidly chosen.

Notice how I added MMCI_ and MCI_ to the register definitions for PL180 for example. I always do that kind of thing as a rule to avoid these kinds of problems right from the outset, and I don't understand why others don't do the same. We've been through soo many "this symbol clashes with something else in the kernel tree" now that we really should be doing this automatically, and not waiting for the clash to happen.

I fully agree with your point and IIRC I always add some prefix to definitions in header files.
Private defines in a .c file, without redundant prefixes, sounded like safe to me at the time, but perhaps I was wrong.

Thanks.

There is no problem with adding ARM or !X86 dependencies to drivers now and fixing them later or as required.

Sent from my mobile phone. Please excuse brevity and lack of formatting.

Right, but if the fix is trivial (e.g. missing #include statement), it's usually better to just fix the code.

Also, in many cases it's possible to say specifically what some code depends on. E.g. if a driver uses clk_get/clk_put, it should depend on CLKDEV_LOOKUP, not on !X86.

Arnd

Or we could get the stubs merged, or x86 could enable the clock API...

If there is a dependency there it should be registered, regardless if x86 enables the clock API.

Last I saw I saw a patch to that effect, asked what the benefit was, and got no answer.

-hpa

Not really, what we *should* have is the clock API available (at least for build purposes) everywhere. The current situation where the API is randomly available on some architectures makes it unusuable in generic code which is nuts and wasting time and effort, you either need ifdefs or Kconfig hoop jumping in all the users which isn't at all sane and means if you're doing anything generic you still need a backup plan.

What should be happening with this is the same as happens with all the other similar APIs - the API should stub itself out where it's not provided and the default should be that all architectures use the generic implementation. The overwhelming majority of clock API usage is just enabling the clock only while the device is running to save power when it isn't which is totally amenable to stubbing out on platforms that don't support that level of power management.

Are you positive about that? I don't recall you replying any of the times I sent out the patch and my mail archive isn't contradicting me either.

I said last time I saw a patch to that effect; it might not have been from you. I might not have seen yours for whatever reason (including losing it on my end.)

-hpa

I'm kind of surprised anyone else has been sending stuff (unless mine got resent by someone else, I did include it in some of my serises for the clock API); I know I've posted mine several times now. In any case, I hope the mail you're replying to answers your question about why it's useful.

In general I'd probably go further and say that (at least when a generic
implementation is available) there should be a very good reason for not
enabling an API on an architecture rather than the other way around.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120703/38f6f475/attachment.sig>

Peter Anvin:

Ok. This is my summary of the compilation errors I have when enabling ARM_AMBA undex x86 and enabling everything that appears in "make oldconfig":

This is a sum up of the errors in all driver that are enabled by telling "CONFIG_ARM_AMBA=y" in the x86 config.

drivers/dma/pl330.c: register names conflict with arch symbols
proposed fix: use proper prefix names

drivers/dma/amba-pl08x.c: needs <asm/hardware/pl080.h>
proposed fix: move pl080.h to include/linux

drivers/gpio/gpio-pl061.c: uses chained_irq_enter/exit
proposed fix: depend on CONFIG_ARM (the function only exists in arm)

drivers/mmc/host/mmci.c: uses <asm/sizes.h> drivers/mmc/host/mmci.c: uses readsl/writesl
proposed fix: use linux/sizes.h and provide readsl/writesl like others do

drivers/spi/spi-pl022.c: warning for an integer size mismatch
proposed fix: none by now

drivers/watchdog/sp805_wdt.c: uses writel_relaxed
proposed fix: depend on CONFIG_ARM (this is a spear-only cell by now)

So, if you agree, I would:

- fix the two dma engines (checking pl080 against russell's pending patches)

- make pl061 and sp805_wdt.c depends on CONFIG_ARM

- temporarily do the same for mmci, which I'll make work soon as I have it under my pci bridge

- ignore the warning on spi-pl022 until I find a fix

- resubmit my enabling of arm_amba for x86 and my pci-to-amba bridge driver.

thanks
/alessandro

Note that there is already an include/linux/amba/pl08x.h. I would just move the few parts of pl080.h that are actually needed with global visibility there, and move the rest to drivers/dma/.

Ack on the linux/sizes.h, that definitely makes sense.

I'm not sure I want to spread readsl/writesl beyond the architectures that already have it. Maybe instead change the driver to use ioread32_rep, which is already available on all architectures and is defined the same way as readsl on ARM.

This one on the other hand makes sense to be defined on all architectures IMHO. I don't mind restricting the driver to ARM for now, but having a generic writel_relaxed would be nice.

Arnd

Arnd Bergmann:

Ok.

I already have this in place for other drivers that I'm going to use with the pci-amba bridge. They have already been picked up by Rusell, but they are not in linux-next yet. So maybe other people have more errors than me related to sizes.h, but this for mmci is trivial with the other stuff in place.

Ok for the rest, I'll submit a patch set following your suggestions when the other sizes.h stuff reaches linux-next.

/alessandro

I've not pushed them into linux-next, because my nightly test builds for Versatile were failing due to the LEDS stuff in arm-soc, and that's one of the platforms your patches have the potential to affect.

Now that the LEDS stuff has been dropped out of arm-soc, the build results are a lot more promising, and I'm now happier to move them into linux-next.

NAK. It's the entire register definitions for the PL08x, which we really should not be exporting to common code.

Please wait until _after_ my DMA engine stuff (which is now in linux-next) makes its way upstream before touching any of this stuff, otherwise there's going to be conflicts.

As part of my patch series, this gets rid of a number of uses of it in arch/arm, but there's still the .cctl_memcpy initializer which does. I've not yet checked whether all implementations use the same value (they probably do), and if so then it should be eliminated from platform code and moved into the driver.

I've talked about asm/hardware/pl080.h within the last week on this very thread... and it's disappointing to have to re-say what I said because you've not read my previous postings.

The major reason why that file is there is that there is *another* PL080 driver in arch/arm/mach-s3c64xx/dma.c which I repeatedly asked the Samsung people to replace with the drivers/dma/amba-pl08x.c driver. :-(

When I worked on the PL08x driver in drivers/dma I reused this header to avoid code duplication.

Now that thing is stranding in the way. Alim, Kukjin, what's happening?

I feel tempted to update Alim's patch myself and push it on you soon...

That stuff is in now, looking real good. Good work on this!

Sounds like a plan. If we just get rid of the duplicate implementation we're going somewhere.

Yours,
Linus Walleij

[...]

Afaik, Alim was working on that but I'm not sure how long he needs more?

Alim, please share the progress of the pl080 work?

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

This is a PCI driver that registers AMBA devices for the range of supported devices. It is currently used by STA2X11, which exports AMBA peripherals under PCIe. The original AMBA drivers work with no changes or minimal ones.

---
 drivers/Kconfig         |    2 +
 drivers/amba/Kconfig    |   10 +++++
 drivers/amba/Makefile   |    1 +
 drivers/amba/pci-amba.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100644 drivers/amba/Kconfig
 create mode 100644 drivers/amba/pci-amba.c

index c2b0cd2..72d5145 100644
@@ -150,4 +150,6 @@ source "drivers/vme/Kconfig"
 
 source "drivers/modem_shm/Kconfig"
 
+source "drivers/amba/Kconfig"
+
 endmenu

new file mode 100644
index 0000000..b5b5aca
@@ -0,0 +1,10 @@
+
+config PCI_AMBA
+	tristate "PCI-to-AMBA bridge"
+	depends on ARM_AMBA && PCI
+	---help---
+	  This compiles a PCI driver that registers AMBA devices, so
+	  the respective AMBA driver can be used unchanged if you have
+	  a PCI to amba bridge. This is required for STA2X11 support.
+
+	  If uncertain, choose N.

index 66e81c2..d30e947 100644
@@ -1,2 +1,3 @@
 obj-$(CONFIG_ARM_AMBA)		+= bus.o
+obj-$(CONFIG_PCI_AMBA)		+= pci-amba.o
 obj-$(CONFIG_TEGRA_AHB)		+= tegra-ahb.o

1.7.7.2