Bridging PCI to amba
Subsystems (6)
cover Bridging PCI to amba
This patch set introduces use of the pl011 AMBA serial port under a PCI bridge. To compile AMBA under x86, though I need <asm/sizes.h>, which is moved to <linux/sizes.h> as suggested earlier.
I'm hereby volunteering to handle the moving of the various users of <asm/sizes.h> to <linux/sizes.h>; this set only moves the ARM core files and the ones that I need under x86.
The whole patch set is sent to the same set of recipients:
all relevant lists, Russell King (for arm), Greg-KH (for uart) and Arnd Bergmann (for generic include).
With this set in place (plus a clok API not included here) I have 4 serial ports working. We have a number of other devices that can use existing drivers, but we definitely need <linux/sizes.h> first.
spusa.root# uname -r
3.4.0-next-20120524-00014-gae0c129
spusa.root# dmesg | grep ttyA
pl011-pci-03:0005: ttyAMA0 at MMIO 0xcf400000 (irq = 46) is a PL011 rev3 pl011-pci-03:0006: ttyAMA1 at MMIO 0xcec00000 (irq = 47) is a PL011 rev3 pl011-pci-03:0007: ttyAMA2 at MMIO 0xce400000 (irq = 48) is a PL011 rev3 pl011-pci-04:0005: ttyAMA3 at MMIO 0xd3400000 (irq = 49) is a PL011 rev3
spusa.root# grep -C1 pl011 /proc/iomem
ce400000-ce7fffff : 0000:03:00.7
ce400000-ce400fff : pl011-pci-03:0007
ce400000-ce400fff : uart-pl011
ce800000-cebfffff : 0000:03:00.6
cec00000-ceffffff : 0000:03:00.6
cec00000-cec00fff : pl011-pci-03:0006
cec00000-cec00fff : uart-pl011
cf000000-cf3fffff : 0000:03:00.5
cf400000-cf7fffff : 0000:03:00.5
cf400000-cf400fff : pl011-pci-03:0005
cf400000-cf400fff : uart-pl011
cf800000-cfbfffff : 0000:03:00.4
--
d3400000-d37fffff : 0000:04:00.5
d3400000-d3400fff : pl011-pci-04:0005
d3400000-d3400fff : uart-pl011
d3800000-d3bfffff : 0000:04:00.4Alessandro Rubini (6):
sizes.h: move from asm-generic to <linux/sizes.h>
amba: use the new linux/sizes.h
ARM: use the new linux/sizes.h
serial: use the new linux/sizes.h
x86: add CONFIG_ARM_AMBA, selected by STA2X11
serial: add amba-pl011-pci
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 +- arch/x86/Kconfig | 4 ++ drivers/amba/bus.c | 2 +- drivers/tty/serial/Kconfig | 10 +++- drivers/tty/serial/Makefile | 1 + drivers/tty/serial/amba-pl011-pci.c | 101 +++++++++++++++++++++++++++++++++++ drivers/tty/serial/amba-pl011.c | 2 +- include/asm-generic/sizes.h | 49 +---------------- include/linux/sizes.h | 47 ++++++++++++++++ 13 files changed, 171 insertions(+), 55 deletions(-) create mode 100644 drivers/tty/serial/amba-pl011-pci.c create mode 100644 include/linux/sizes.h
Signature · 1 line
1.7.7.2
PATCH 1/6 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
include/asm-generic/sizes.h +2 -47
@@ -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>
include/linux/sizes.h new file +47 -0
47 lines not on this page · raw message
Signature · 1 line
1.7.7.2
PATCH 2/6 amba: use the new linux/sizes.h
In reply to Alessandro Rubini--- drivers/amba/bus.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/amba/bus.c +1 -1
@@ -18,7 +18,7 @@#include <linux/amba/bus.h> #include <asm/irq.h> -#include <asm/sizes.h> +#include <linux/sizes.h> #define to_amba_driver (d) container_of(d, struct amba_driver, drv)
Signature · 1 line
1.7.7.2
PATCH 3/6 ARM: use the new linux/sizes.h
In reply to Alessandro Rubini--- 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(-)
arch/arm/include/asm/memory.h +1 -1
@@ -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>
arch/arm/mm/dma-mapping.c +1 -1
@@ -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>
arch/arm/mm/init.c +1 -1
@@ -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>
arch/arm/mm/ioremap.c +1 -1
@@ -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>
arch/arm/mm/mmu.c +1 -1
@@ -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>
Signature · 1 line
1.7.7.2
PATCH 4/6 pl011: use the new linux/sizes.h
In reply to Alessandro Rubini--- drivers/tty/serial/amba-pl011.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/tty/serial/amba-pl011.c +1 -1
@@ -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
Signature · 1 line
1.7.7.2
PATCH 5/6 x86: add CONFIG_ARM_AMBA, selected by STA2X11
In reply to Alessandro RubiniThe 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(-)
arch/x86/Kconfig +4 -0
@@ -499,6 +499,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,
@@ -2154,6 +2155,9 @@ config GEOS endif # X86_32 +config ARM_AMBA + bool + config AMD_NB def_bool y depends on CPU_SUP_AMD && PCI
Signature · 1 line
1.7.7.2
PATCH 6/6 serial: add amba-pl011-pci
In reply to Alessandro RubiniThis is a simple PCI driver that registers an amba device in its probe function. It successfully drives the 4 serial ports of the sta2x11 I/O Hub.
--- drivers/tty/serial/Kconfig | 10 +++- drivers/tty/serial/Makefile | 1 + drivers/tty/serial/amba-pl011-pci.c | 101 +++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletions(-) create mode 100644 drivers/tty/serial/amba-pl011-pci.c
drivers/tty/serial/Kconfig +9 -1
@@ -39,14 +39,22 @@ config SERIAL_AMBA_PL010_CONSOLE config SERIAL_AMBA_PL011 tristate "ARM AMBA PL011 serial port support" depends on ARM_AMBA + default y if STA2X11 select SERIAL_CORE help This selects the ARM(R) AMBA(R) PrimeCell PL011 UART. If you have an Integrator/PP2, Integrator/CP or Versatile platform, say Y or M - here. + here. This is also needed to use the sta2x11 I/O Hub for Atom. If unsure, say N. +config SERIAL_AMBA_PL011_PCI + tristate "ARM AMBA PL011 behind a PCI-to-AMBA bridge" + depends on SERIAL_AMBA_PL011 && PCI + default y if STA2X11 + help + Say Y if your AMBA bus is behind a PCI bridge (e.g.: sta2x11) + config SERIAL_AMBA_PL011_CONSOLE bool "Support for console on AMBA serial port" depends on SERIAL_AMBA_PL011=y
drivers/tty/serial/Makefile +1 -0
@@ -19,6 +19,7 @@ obj-$(CONFIG_SERIAL_8250) += 8250/ obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o +obj-$(CONFIG_SERIAL_AMBA_PL011_PCI) += amba-pl011-pci.o obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o obj-$(CONFIG_SERIAL_PXA) += pxa.o obj-$(CONFIG_SERIAL_PNX8XXX) += pnx8xxx_uart.o
drivers/tty/serial/amba-pl011-pci.c new file +101 -0
101 lines not on this page · raw message
Signature · 1 line
1.7.7.2
Arnd Bergmann
maintainerRe: [PATCH 6/6] serial: add amba-pl011-pci
Quoted 7 lines · Alessandro Rubini
This is a simple PCI driver that registers an amba device in its probe function. It successfully drives the 4 serial ports of the sta2x11 I/O Hub.
Signed-off-by Alessandro Rubini <[email protected]>
Aside from the dma mask, this looks almost entirely generic. Would it be possible to make this a generic pci-amba driver that lives under drivers/amba/ and does not care about the type of device behind it?
Arnd
Re: [PATCH 6/6] serial: add amba-pl011-pci
Quoted 3 lines
Aside from the dma mask, this looks almost entirely generic. Would it be possible to make this a generic pci-amba driver that lives under
drivers/amba/and does not care about the type of device behind it?
Yes, it's possible. Actually, this was my longer-time plan: propose a factorization when more of those were ready. So I'll remove the special name in the device and offer an implementation as generic as possible.
BTW, are the prerequisite patches of with you?
thanks
/alessandro
Arnd Bergmann
maintainerRe: [PATCH 6/6] serial: add amba-pl011-pci
Quoted 11 lines · Alessandro Rubini
Quoted 3 lines
Aside from the dma mask, this looks almost entirely generic. Would it be possible to make this a generic pci-amba driver that lives under
drivers/amba/and does not care about the type of device behind it?Yes, it's possible. Actually, this was my longer-time plan: propose a factorization when more of those were ready. So I'll remove the special name in the device and offer an implementation as generic as possible.
BTW, are the prerequisite patches of with you?
Yes, they all look good to me, although patch 5 would have to change if you do patch 6 the way we discussed here.
Arnd
Re: [PATCH 2/6] amba: use the new linux/sizes.h
In reply to Alessandro RubiniQuoted 12 lines · Alessandro Rubini
Signed-off-by Alessandro Rubini <[email protected]>Cc Russell King <[email protected]>--- drivers/amba/bus.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/amba/bus.c +0
-0
drivers/amba/bus.c line 21
Quoted 5 lines
drivers/amba/bus.c +1
-1@@ -18,7 +18,7 @@#include <linux/amba/bus.h> #include <asm/irq.h> -#include <asm/sizes.h> +#include <linux/sizes.h>
Please move this up alongside the other linux/ includes.
Re: [PATCH 6/6] serial: add amba-pl011-pci
In reply to Alessandro Rubinidrivers/tty/serial/Kconfig config SERIAL_AMBA_PL010_CONSOLE · line 42
Quoted 9 lines · Alessandro Rubini
drivers/tty/serial/Kconfig +1
-0@@ -39,14 +39,22 @@ config SERIAL_AMBA_PL010_CONSOLE config SERIAL_AMBA_PL011 tristate "ARM AMBA PL011 serial port support" depends on ARM_AMBA + default y if STA2X11
I don't think we want to encourage an ever growing list of platforms here. If we did this on ARM, this would be hellishly long.
Quoted 4 lines
drivers/tty/serial/Kconfig +4
-0+/* This is a template, copied every time a new pci device appears */ +static AMBA_APB_DEVICE(pl011_pci_template, "pl011-pci", 0, + 0 /* base */, {0} /* irqs */, NULL /* data */); +
drivers/tty/serial/Kconfig config SERIAL_AMBA_PL010_CONSOLE · line 56
Quoted 10 lines
drivers/tty/serial/Kconfig +10
-0+static int __devinit pl011_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *id) +{ + struct amba_device *adev; + struct amba_pl011_data *data; + int ret; + + /* Simply build an amba device and register it */ + adev = kmemdup(&pl011_pci_template_device, + sizeof(pl011_pci_template_device), GFP_KERNEL);
NAK. We have interfaces in the AMBA code for dynamically allocating AMBA devices now - please use them instead of coding your own. They avoid bugs.
drivers/tty/serial/Kconfig config SERIAL_AMBA_PL010_CONSOLE
Quoted 5 lines
drivers/tty/serial/Kconfig +5
-0+ /* change name */ + adev->dev.init_name = kasprintf(GFP_ATOMIC, "pl011-pci-%02x:%04x", + pdev->bus->number, pdev->devfn); + + printk(KERN_INFO "%s %i\n", __func__, __LINE__);
This looks like debugging.
drivers/tty/serial/Kconfig
Quoted 13 lines
drivers/tty/serial/Kconfig +13
-0+ if ((ret = amba_device_register(adev, &pdev->resource[0])) < 0) { + kfree(adev); + return ret; + } + return 0; +}; + +static void __devexit pl011_pci_remove(struct pci_dev *pdev) +{ + struct amba_device *adev = pci_get_drvdata(pdev); + amba_device_unregister(adev); + kfree(adev->dev.platform_data); + kfree(adev);
This comes nowhere close to being sane with the driver model. This is an oops waiting to happen. You can't guarantee that 'adev' will be unused by the time amba_device_unregister() (or in fact any such call into the driver model) returns.
This is because these suckers (and all devices) are refcounted. If you use the correct interfaces, the devices will be freed for you automatically at the correct time.
The only thing that won't be is adev->dev.platform_data - the correct way to do this is the following along with the correct allocation interfaces:
struct amba_device *adev = pci_get_drvdata(pdev); void *priv = adev->dev.platform_data; amba_device_unregister(adev); kfree(priv);
In other words, save off the platform data pointer, unregister the struct device, and then free the platform data (it will not be used at that point.)
Re: [PATCH 6/6] serial: add amba-pl011-pci
In reply to Alessandro RubiniOne other point.
drivers/tty/serial/amba-pl011-pci.c line 19
Quoted 3 lines · Alessandro Rubini
drivers/tty/serial/amba-pl011-pci.c new file +3
-0+/* This is a template, copied every time a new pci device appears */ +static AMBA_APB_DEVICE(pl011_pci_template, "pl011-pci", 0, + 0 /* base */, {0} /* irqs */, NULL /* data */);
APB device. It's a _peripheral_, it only has a _slave_ port which can't initiate DMA, so:
drivers/tty/serial/amba-pl011-pci.c line 44
Quoted 5 lines
drivers/tty/serial/amba-pl011-pci.c new file +5
-0+if (pdev->vendor ==PCI_VENDOR_ID_STMICRO ) { +/* Under sta2x11, DMA is there but limited to 512M */ + adev->dma_mask =SZ_512M -1 ; + adev->dev.coherent_dma_mask =SZ_512M -1 ; + }
This is pointless and unnecessary. The PL011 driver itself doesn't use _this_ struct device for anything to do with DMA at all. That's all handled by the DMA engine's struct device.
That's true of all APB devices. Only AHB devices with a master port can initiate bus transactions, and hence cause accesses to other parts of the system.
Re: [PATCH 6/6] serial: add amba-pl011-pci
In reply to Russell King - ARM LinuxQuoted 4 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ default y if STA2X11I don't think we want to encourage an ever growing list of platforms here. If we did this on ARM, this would be hellishly long.
Ok.
Quoted 3 lines
NAK. We have interfaces in the AMBA code for dynamically allocating AMBA devices now - please use them instead of coding your own. They avoid bugs.
Sure. Thanks for noting. Maybe it wasn't there when I coded this initially. Will do.
Quoted 3 lines
drivers/tty/serial/amba-pl011-pci.c line 60
Quoted 1 line
drivers/tty/serial/amba-pl011-pci.c new file +1
-0+printk (KERN_INFO "%s %i\n" , __func__, __LINE__);This looks like debugging.
Yes. After sending I noted this and another point. I apologize. Version 2 will be fixed in all respects.
Quoted 3 lines
This comes nowhere close to being sane with the driver model. [...] In other words, save off the platform data pointer, unregister the struct device, and then free the platform data (it will not be used at that point.)
Thanks a lot for this and also the comments on APB/AHB in the other message.
/alessandro