linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 1/6] sizes.h: move from asm-generic to <linux/sizes.h>
       [not found] <cover.1338222460.git.rubini@gnudd.com>
@ 2012-05-28 16:37 ` Alessandro Rubini
  2012-05-28 16:37 ` [PATCH V2 2/6] amba: use the new linux/sizes.h Alessandro Rubini
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-05-28 16:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

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.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Alan Cox <alan@linux.intel.com>
---
 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

diff --git a/include/asm-generic/sizes.h b/include/asm-generic/sizes.h
index ea5d4ef..1dcfad9 100644
--- a/include/asm-generic/sizes.h
+++ b/include/asm-generic/sizes.h
@@ -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>
diff --git a/include/linux/sizes.h b/include/linux/sizes.h
new file mode 100644
index 0000000..ce3e815
--- /dev/null
+++ b/include/linux/sizes.h
@@ -0,0 +1,47 @@
+/*
+ * include/linux/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 __LINUX_SIZES_H__
+#define __LINUX_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 /* __LINUX_SIZES_H__ */
-- 
1.7.7.2

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH V2 2/6] amba: use the new linux/sizes.h
       [not found] <cover.1338222460.git.rubini@gnudd.com>
  2012-05-28 16:37 ` [PATCH V2 1/6] sizes.h: move from asm-generic to <linux/sizes.h> Alessandro Rubini
@ 2012-05-28 16:37 ` Alessandro Rubini
  2012-05-28 16:37 ` [PATCH V2 3/6] ARM: " Alessandro Rubini
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-05-28 16:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Alan Cox <alan@linux.intel.com>
---
 drivers/amba/bus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index b7e7285..e8eb91b 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH V2 3/6] ARM: use the new linux/sizes.h
       [not found] <cover.1338222460.git.rubini@gnudd.com>
  2012-05-28 16:37 ` [PATCH V2 1/6] sizes.h: move from asm-generic to <linux/sizes.h> Alessandro Rubini
  2012-05-28 16:37 ` [PATCH V2 2/6] amba: use the new linux/sizes.h Alessandro Rubini
@ 2012-05-28 16:37 ` Alessandro Rubini
  2012-05-28 16:37 ` [PATCH V2 4/6] serial: " Alessandro Rubini
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-05-28 16:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Alan Cox <alan@linux.intel.com>
---
 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(-)

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index fcb5757..e965f1b 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -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>
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ea6b431..30a031c 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -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>
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index c21d06c..ad7fd8a 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -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>
 
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 4f55f50..566750f 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -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>
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e5dad60..2196116 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH V2 4/6] serial: use the new linux/sizes.h
       [not found] <cover.1338222460.git.rubini@gnudd.com>
                   ` (2 preceding siblings ...)
  2012-05-28 16:37 ` [PATCH V2 3/6] ARM: " Alessandro Rubini
@ 2012-05-28 16:37 ` Alessandro Rubini
  2012-05-28 16:37 ` [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11 Alessandro Rubini
  2012-05-28 16:38 ` [PATCH V2 6/6] drivers/amba: add support for a PCI bridge Alessandro Rubini
  5 siblings, 0 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-05-28 16:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Russell King <linux@arm.linux.org.uk>
---
 drivers/tty/serial/amba-pl011.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 4ad721f..d394b93 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
       [not found] <cover.1338222460.git.rubini@gnudd.com>
                   ` (3 preceding siblings ...)
  2012-05-28 16:37 ` [PATCH V2 4/6] serial: " Alessandro Rubini
@ 2012-05-28 16:37 ` Alessandro Rubini
  2012-06-28 20:06   ` H. Peter Anvin
  2012-07-01 10:44   ` Alessandro Rubini
  2012-05-28 16:38 ` [PATCH V2 6/6] drivers/amba: add support for a PCI bridge Alessandro Rubini
  5 siblings, 2 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-05-28 16:37 UTC (permalink / raw)
  To: linux-kernel
  Cc: Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

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.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
---
 arch/x86/Kconfig |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 91dea918..112718f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH V2 6/6] drivers/amba: add support for a PCI bridge
       [not found] <cover.1338222460.git.rubini@gnudd.com>
                   ` (4 preceding siblings ...)
  2012-05-28 16:37 ` [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11 Alessandro Rubini
@ 2012-05-28 16:38 ` Alessandro Rubini
  5 siblings, 0 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-05-28 16:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

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.

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Alan Cox <alan@linux.intel.com>
---
 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

diff --git a/drivers/Kconfig b/drivers/Kconfig
index c2b0cd2..72d5145 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -150,4 +150,6 @@ source "drivers/vme/Kconfig"
 
 source "drivers/modem_shm/Kconfig"
 
+source "drivers/amba/Kconfig"
+
 endmenu
diff --git a/drivers/amba/Kconfig b/drivers/amba/Kconfig
new file mode 100644
index 0000000..b5b5aca
--- /dev/null
+++ b/drivers/amba/Kconfig
@@ -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.
diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile
index 66e81c2..d30e947 100644
--- a/drivers/amba/Makefile
+++ b/drivers/amba/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_ARM_AMBA)		+= bus.o
+obj-$(CONFIG_PCI_AMBA)		+= pci-amba.o
 obj-$(CONFIG_TEGRA_AHB)		+= tegra-ahb.o
diff --git a/drivers/amba/pci-amba.c b/drivers/amba/pci-amba.c
new file mode 100644
index 0000000..12e7a7e
--- /dev/null
+++ b/drivers/amba/pci-amba.c
@@ -0,0 +1,95 @@
+/*
+ * Support for AMBA devices (both APB and AHB) behind a PCI bridge
+ * Copyright 2012 ST Microelectronics (Alessandro Rubini)
+ * GNU GPL version 2.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/amba/bus.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/slab.h>
+#include <linux/irq.h>
+#include <linux/sizes.h>
+
+static int __devinit pci_amba_probe(struct pci_dev *pdev,
+				     const struct pci_device_id *id)
+{
+	struct amba_device *adev;
+	char *name;
+	int ret;
+
+	pci_enable_msi(pdev);
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	/* Create a name: each of them must be different */
+	name = devm_kzalloc(&pdev->dev, strlen(dev_name(&pdev->dev)) + 6,
+		GFP_KERNEL);
+	sprintf(name, "amba-%s", dev_name(&pdev->dev));
+
+	/* Simply build an amba device and register it */
+	adev = amba_device_alloc(name,  pdev->resource[0].start, SZ_4K);
+	if (!adev)
+		return -ENOMEM;
+	adev->irq[0] = pdev->irq;
+
+	/* This bridge can host both APB and AHB devices, so set master */
+	pci_set_master(pdev);
+	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;
+	}
+
+	adev->dev.platform_data = pdev->dev.platform_data;
+	pci_set_drvdata(pdev, adev);
+
+	if ((ret = amba_device_add(adev, &pdev->resource[0])) < 0)
+		return ret;
+	return 0;
+};
+
+static void __devexit pci_amba_remove(struct pci_dev *pdev)
+{
+	struct amba_device *adev = pci_get_drvdata(pdev);
+	amba_device_unregister(adev);
+	pci_disable_msi(pdev);
+}
+
+static DEFINE_PCI_DEVICE_TABLE(pci_amba_table) = {
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_UART_HWFC)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_UART_NO_HWFC)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_SOC_DMA)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_I2C)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_SPI_HS)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_SDIO_EMMC)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_SDIO)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_AUDIO_ROUTER_DMA)},
+	{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_AUDIO_ROUTER_MSPS)},
+	{0,}
+};
+
+static struct pci_driver pci_amba_driver = {
+	.name		= "pci-amba",
+	.id_table	= pci_amba_table,
+	.probe		= pci_amba_probe,
+	.remove		= __devexit_p(pci_amba_remove),
+};
+
+static int __init pci_amba_init(void)
+{
+	return pci_register_driver(&pci_amba_driver);
+}
+
+static void __exit pci_amba_exit(void)
+{
+	pci_unregister_driver(&pci_amba_driver);
+}
+
+module_init(pci_amba_init);
+module_exit(pci_amba_exit);
+
+MODULE_LICENSE("GPL");
-- 
1.7.7.2

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-05-28 16:37 ` [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11 Alessandro Rubini
@ 2012-06-28 20:06   ` H. Peter Anvin
  2012-07-01 10:44   ` Alessandro Rubini
  1 sibling, 0 replies; 28+ messages in thread
From: H. Peter Anvin @ 2012-06-28 20:06 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, Giancarlo Asnaghi, Alan Cox, Russell King, x86,
	Greg Kroah-Hartman, Arnd Bergmann, linux-arm-kernel,
	linux-serial, linux-arch

On 05/28/2012 09:37 AM, Alessandro Rubini wrote:
> 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.
>
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
> ---
>   arch/x86/Kconfig |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 91dea918..112718f 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -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


Nacked-by: H. Peter Anvin <hpa@zytor.com>

This brings in tons of code into the x86 all{yes,mod}config builds, and 
as perhaps one could expect some of it doesn't compile.  For example:


/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c: In function 
‘dmac_alloc_resources’:
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:2056:2: warning: 
passing argument 3 of ‘dma_alloc_attrs’ from incompatible
  pointer type [enabled by default]
In file included from 
/home/hpa/kernel/tip.x86-platform/include/linux/dma-mapping.h:73:0,
                  from 
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:22:
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/dma-mapping.h:130:1: 
note: expected ‘dma_addr_t *’ but argument is of typ
e ‘u32 *’
   CC      sound/pci/au88x0/au8830.o
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:73:0: warning: 
"DS" redefined [enabled by default]
In file included from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/ptrace.h:5:0,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/vm86.h:130,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/processor.h:10,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/thread_info.h:22,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/thread_info.h:54,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/preempt.h:9,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/spinlock.h:50,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/vmalloc.h:4,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/io.h:195,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/io.h:22,
                  from 
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:15:
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/ptrace-abi.h:13:0: note: 
this is the location of the previous definition
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:89:0: warning: 
"ES" redefined [enabled by default]
In file included from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/ptrace.h:5:0,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/vm86.h:130,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/processor.h:10,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/thread_info.h:22,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/thread_info.h:54,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/preempt.h:9,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/spinlock.h:50,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/vmalloc.h:4,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/io.h:195,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/io.h:22,
                  from 
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:15:
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/ptrace-abi.h:14:0: note: 
this is the location of the previous definition
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:100:0: warning: 
"CS" redefined [enabled by default]
In file included from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/ptrace.h:5:0,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/vm86.h:130,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/processor.h:10,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/thread_info.h:22,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/thread_info.h:54,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/preempt.h:9,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/spinlock.h:50,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/vmalloc.h:4,
                  from 
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/io.h:195,
                  from 
/home/hpa/kernel/tip.x86-platform/include/linux/io.h:22,
                  from 
/home/hpa/kernel/tip.x86-platform/drivers/dma/pl330.c:15:
/home/hpa/kernel/tip.x86-platform/arch/x86/include/asm/ptrace-abi.h:19:0: note: 
this is the location of the previous definition
   CC      drivers/dma/pch_dma.o
   CC      drivers/dma/amba-pl08x.o
/home/hpa/kernel/tip.x86-platform/drivers/dma/amba-pl08x.c:86:32: fatal 
error: asm/hardware/pl080.h: No such file or directory
compilation terminated.
make[4]: *** [drivers/dma/amba-pl08x.o] Error 1
make[3]: *** [drivers/dma] Error 2
make[2]: *** [drivers] Error 2
make[2]: *** Waiting for unfinished jobs....



-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.




^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-05-28 16:37 ` [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11 Alessandro Rubini
  2012-06-28 20:06   ` H. Peter Anvin
@ 2012-07-01 10:44   ` Alessandro Rubini
  2012-07-01 10:59     ` Russell King - ARM Linux
                       ` (3 more replies)
  1 sibling, 4 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-07-01 10:44 UTC (permalink / raw)
  To: hpa
  Cc: linux-kernel, giancarlo.asnaghi, alan, linux, x86, gregkh, arnd,
	linux-arm-kernel, linux-serial, linux-arch

>> 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.

> Nacked-by: H. Peter Anvin <hpa@zytor.com>
> 
> This brings in tons of code into the x86 all{yes,mod}config builds, and 
> as perhaps one could expect some of it doesn't compile.  For example:

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 10:44   ` Alessandro Rubini
@ 2012-07-01 10:59     ` Russell King - ARM Linux
  2012-07-01 11:58       ` Jassi Brar
  2012-07-01 11:04     ` Alessandro Rubini
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2012-07-01 10:59 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: hpa, linux-kernel, giancarlo.asnaghi, alan, x86, gregkh, arnd,
	linux-arm-kernel, linux-serial, linux-arch

On Sun, Jul 01, 2012 at 12:44:01PM +0200, Alessandro Rubini wrote:
> 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 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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 10:44   ` Alessandro Rubini
  2012-07-01 10:59     ` Russell King - ARM Linux
@ 2012-07-01 11:04     ` Alessandro Rubini
  2012-07-01 11:11       ` Russell King - ARM Linux
  2012-07-01 14:13     ` H. Peter Anvin
  2012-07-03 13:00     ` Alessandro Rubini
  3 siblings, 1 reply; 28+ messages in thread
From: Alessandro Rubini @ 2012-07-01 11:04 UTC (permalink / raw)
  To: linux
  Cc: hpa, linux-kernel, giancarlo.asnaghi, alan, x86, gregkh, arnd,
	linux-arm-kernel, linux-serial, linux-arch

> I think the answer is... those primecell drivers need fixing in some way.

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

> As for PL08x, which depends on asm/hardware/pl080.h,

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.

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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 11:04     ` Alessandro Rubini
@ 2012-07-01 11:11       ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2012-07-01 11:11 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: hpa, linux-kernel, giancarlo.asnaghi, alan, x86, gregkh, arnd,
	linux-arm-kernel, linux-serial, linux-arch

On Sun, Jul 01, 2012 at 01:04:54PM +0200, Alessandro Rubini wrote:
> > I think the answer is... those primecell drivers need fixing in some way.
> 
> Ok. I'm going to attack them and make a report.
> 
> > As for PL08x, which depends on asm/hardware/pl080.h,
> 
> 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.
> 
> > Note also that hpa has only reported the first errors he encountered, so
> > there may be more, and that's somehting which needs checking.
> 
> Yes. I'm going to make a status report soon, counting and classifying
> all errors I encounter when enabling all amba drivers under x86.

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 10:59     ` Russell King - ARM Linux
@ 2012-07-01 11:58       ` Jassi Brar
  2012-07-01 12:10         ` Russell King - ARM Linux
  0 siblings, 1 reply; 28+ messages in thread
From: Jassi Brar @ 2012-07-01 11:58 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Alessandro Rubini, linux-arch, giancarlo.asnaghi, arnd, gregkh,
	x86, linux-kernel, linux-serial, hpa, linux-arm-kernel, alan

On 1 July 2012 16:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Sun, Jul 01, 2012 at 12:44:01PM +0200, Alessandro Rubini wrote:
>> 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 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.
>
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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 11:58       ` Jassi Brar
@ 2012-07-01 12:10         ` Russell King - ARM Linux
  2012-07-01 13:11           ` Jassi Brar
  0 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2012-07-01 12:10 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Alessandro Rubini, linux-arch, giancarlo.asnaghi, arnd, gregkh,
	x86, linux-kernel, linux-serial, hpa, linux-arm-kernel, alan

On Sun, Jul 01, 2012 at 05:28:16PM +0530, Jassi Brar wrote:
> On 1 July 2012 16:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > On Sun, Jul 01, 2012 at 12:44:01PM +0200, Alessandro Rubini wrote:
> >> 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 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.
> >
> 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 :)

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 12:10         ` Russell King - ARM Linux
@ 2012-07-01 13:11           ` Jassi Brar
  0 siblings, 0 replies; 28+ messages in thread
From: Jassi Brar @ 2012-07-01 13:11 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Alessandro Rubini, linux-arch, giancarlo.asnaghi, arnd, gregkh,
	x86, linux-kernel, linux-serial, hpa, linux-arm-kernel, alan

On 1 July 2012 17:40, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Sun, Jul 01, 2012 at 05:28:16PM +0530, Jassi Brar wrote:
>> On 1 July 2012 16:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>> > On Sun, Jul 01, 2012 at 12:44:01PM +0200, Alessandro Rubini wrote:
>> >> 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 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.
>> >
>> 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 :)
>
> 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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 10:44   ` Alessandro Rubini
  2012-07-01 10:59     ` Russell King - ARM Linux
  2012-07-01 11:04     ` Alessandro Rubini
@ 2012-07-01 14:13     ` H. Peter Anvin
  2012-07-02 16:58       ` Arnd Bergmann
  2012-07-03 13:00     ` Alessandro Rubini
  3 siblings, 1 reply; 28+ messages in thread
From: H. Peter Anvin @ 2012-07-01 14:13 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: linux-kernel, giancarlo.asnaghi, alan, linux, x86, gregkh, arnd,
	linux-arm-kernel, linux-serial, linux-arch

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

Alessandro Rubini <rubini@gnudd.com> wrote:

>>> 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.
>
>> Nacked-by: H. Peter Anvin <hpa@zytor.com>
>> 
>> This brings in tons of code into the x86 all{yes,mod}config builds,
>and 
>> as perhaps one could expect some of it doesn't compile.  For example:
>
>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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 14:13     ` H. Peter Anvin
@ 2012-07-02 16:58       ` Arnd Bergmann
  2012-07-02 18:05         ` Mark Brown
  0 siblings, 1 reply; 28+ messages in thread
From: Arnd Bergmann @ 2012-07-02 16:58 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Alessandro Rubini, linux-kernel, giancarlo.asnaghi, alan, linux,
	x86, gregkh, linux-arm-kernel, linux-serial, linux-arch

On Sunday 01 July 2012, H. Peter Anvin wrote:
> 
> There is no problem with adding ARM or !X86 dependencies to drivers
> now and fixing them later or as required.

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-02 16:58       ` Arnd Bergmann
@ 2012-07-02 18:05         ` Mark Brown
  2012-07-02 18:07           ` H. Peter Anvin
  0 siblings, 1 reply; 28+ messages in thread
From: Mark Brown @ 2012-07-02 18:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: H. Peter Anvin, Alessandro Rubini, linux-kernel,
	giancarlo.asnaghi, alan, linux, x86, gregkh, linux-arm-kernel,
	linux-serial, linux-arch

On Mon, Jul 02, 2012 at 04:58:27PM +0000, Arnd Bergmann wrote:

> 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.

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-02 18:05         ` Mark Brown
@ 2012-07-02 18:07           ` H. Peter Anvin
  2012-07-02 18:33             ` Mark Brown
  0 siblings, 1 reply; 28+ messages in thread
From: H. Peter Anvin @ 2012-07-02 18:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: Arnd Bergmann, Alessandro Rubini, linux-kernel,
	giancarlo.asnaghi, alan, linux, x86, gregkh, linux-arm-kernel,
	linux-serial, linux-arch

On 07/02/2012 11:05 AM, Mark Brown wrote:
> On Mon, Jul 02, 2012 at 04:58:27PM +0000, Arnd Bergmann wrote:
> 
>> 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.
> 
> 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



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-02 18:07           ` H. Peter Anvin
@ 2012-07-02 18:33             ` Mark Brown
  2012-07-02 19:41               ` H. Peter Anvin
  0 siblings, 1 reply; 28+ messages in thread
From: Mark Brown @ 2012-07-02 18:33 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Arnd Bergmann, Alessandro Rubini, linux-kernel,
	giancarlo.asnaghi, alan, linux, x86, gregkh, linux-arm-kernel,
	linux-serial, linux-arch

[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]

On Mon, Jul 02, 2012 at 11:07:57AM -0700, H. Peter Anvin wrote:

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

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.

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

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-02 18:33             ` Mark Brown
@ 2012-07-02 19:41               ` H. Peter Anvin
  2012-07-03 11:05                 ` Mark Brown
  0 siblings, 1 reply; 28+ messages in thread
From: H. Peter Anvin @ 2012-07-02 19:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: Arnd Bergmann, Alessandro Rubini, linux-kernel,
	giancarlo.asnaghi, alan, linux, x86, gregkh, linux-arm-kernel,
	linux-serial, linux-arch

On 07/02/2012 11:33 AM, Mark Brown wrote:
> 
>> Last I saw I saw a patch to that effect, asked what the benefit
>> was, and got no answer.
> 
> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-02 19:41               ` H. Peter Anvin
@ 2012-07-03 11:05                 ` Mark Brown
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2012-07-03 11:05 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Arnd Bergmann, Alessandro Rubini, linux-kernel,
	giancarlo.asnaghi, alan, linux, x86, gregkh, linux-arm-kernel,
	linux-serial, linux-arch

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

On Mon, Jul 02, 2012 at 12:41:59PM -0700, H. Peter Anvin wrote:
> On 07/02/2012 11:33 AM, Mark Brown wrote:

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

> > 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.)

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-01 10:44   ` Alessandro Rubini
                       ` (2 preceding siblings ...)
  2012-07-01 14:13     ` H. Peter Anvin
@ 2012-07-03 13:00     ` Alessandro Rubini
  2012-07-03 13:19       ` Arnd Bergmann
  2012-07-03 13:26       ` Alessandro Rubini
  3 siblings, 2 replies; 28+ messages in thread
From: Alessandro Rubini @ 2012-07-03 13:00 UTC (permalink / raw)
  To: hpa
  Cc: linux-kernel, giancarlo.asnaghi, alan, linux, x86, gregkh, arnd,
	linux-arm-kernel, linux-serial, linux-arch

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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-03 13:00     ` Alessandro Rubini
@ 2012-07-03 13:19       ` Arnd Bergmann
  2012-07-03 19:34         ` Russell King - ARM Linux
  2012-07-03 13:26       ` Alessandro Rubini
  1 sibling, 1 reply; 28+ messages in thread
From: Arnd Bergmann @ 2012-07-03 13:19 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: hpa, linux-kernel, giancarlo.asnaghi, alan, linux, x86, gregkh,
	linux-arm-kernel, linux-serial, linux-arch

On Tuesday 03 July 2012, Alessandro Rubini wrote:
> Peter Anvin:
> > There is no problem with adding ARM or !X86 dependencies to drivers
> > now and fixing them later or as required.
> 
> 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

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/.

>    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

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.

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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-03 13:00     ` Alessandro Rubini
  2012-07-03 13:19       ` Arnd Bergmann
@ 2012-07-03 13:26       ` Alessandro Rubini
  2012-07-03 19:46         ` Russell King - ARM Linux
  1 sibling, 1 reply; 28+ messages in thread
From: Alessandro Rubini @ 2012-07-03 13:26 UTC (permalink / raw)
  To: arnd
  Cc: hpa, linux-kernel, giancarlo.asnaghi, alan, linux, x86, gregkh,
	linux-arm-kernel, linux-serial, linux-arch


Arnd Bergmann:

> 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/.

Ok.

>>   proposed fix: use linux/sizes.h and provide readsl/writesl like others do

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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-03 13:19       ` Arnd Bergmann
@ 2012-07-03 19:34         ` Russell King - ARM Linux
  2012-08-05 20:28           ` Linus Walleij
  0 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2012-07-03 19:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alessandro Rubini, hpa, linux-kernel, giancarlo.asnaghi, alan,
	x86, gregkh, linux-arm-kernel, linux-serial, linux-arch

On Tue, Jul 03, 2012 at 01:19:40PM +0000, Arnd Bergmann wrote:
> On Tuesday 03 July 2012, Alessandro Rubini wrote:
> >    drivers/dma/amba-pl08x.c: needs <asm/hardware/pl080.h>
> >      proposed fix: move pl080.h to include/linux
> 
> 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/.

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-03 13:26       ` Alessandro Rubini
@ 2012-07-03 19:46         ` Russell King - ARM Linux
  0 siblings, 0 replies; 28+ messages in thread
From: Russell King - ARM Linux @ 2012-07-03 19:46 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: arnd, hpa, linux-kernel, giancarlo.asnaghi, alan, x86, gregkh,
	linux-arm-kernel, linux-serial, linux-arch

On Tue, Jul 03, 2012 at 03:26:41PM +0200, Alessandro Rubini wrote:
> 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.

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-07-03 19:34         ` Russell King - ARM Linux
@ 2012-08-05 20:28           ` Linus Walleij
  2012-08-07 10:06             ` Kukjin Kim
  0 siblings, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2012-08-05 20:28 UTC (permalink / raw)
  To: Russell King - ARM Linux, Alim Akhtar, Kukjin Kim
  Cc: Arnd Bergmann, Alessandro Rubini, hpa, linux-kernel,
	giancarlo.asnaghi, alan, x86, gregkh, linux-arm-kernel,
	linux-serial, linux-arch

On Tue, Jul 3, 2012 at 9:34 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Jul 03, 2012 at 01:19:40PM +0000, Arnd Bergmann wrote:
>> On Tuesday 03 July 2012, Alessandro Rubini wrote:
>> >    drivers/dma/amba-pl08x.c: needs <asm/hardware/pl080.h>
>> >      proposed fix: move pl080.h to include/linux
>>
>> 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/.
>
> NAK.  It's the entire register definitions for the PL08x, which we really
> should not be exporting to common code.

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...

> 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.

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

> 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.

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

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11
  2012-08-05 20:28           ` Linus Walleij
@ 2012-08-07 10:06             ` Kukjin Kim
  0 siblings, 0 replies; 28+ messages in thread
From: Kukjin Kim @ 2012-08-07 10:06 UTC (permalink / raw)
  To: 'Linus Walleij', 'Russell King - ARM Linux',
	'Alim Akhtar'
  Cc: 'Arnd Bergmann', 'Alessandro Rubini',
	hpa, linux-kernel, giancarlo.asnaghi, alan, x86, gregkh,
	linux-arm-kernel, linux-serial, linux-arch

Linus Walleij wrote:
> 

[...]

> 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?
> 
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?

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

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2012-08-07 10:06 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1338222460.git.rubini@gnudd.com>
2012-05-28 16:37 ` [PATCH V2 1/6] sizes.h: move from asm-generic to <linux/sizes.h> Alessandro Rubini
2012-05-28 16:37 ` [PATCH V2 2/6] amba: use the new linux/sizes.h Alessandro Rubini
2012-05-28 16:37 ` [PATCH V2 3/6] ARM: " Alessandro Rubini
2012-05-28 16:37 ` [PATCH V2 4/6] serial: " Alessandro Rubini
2012-05-28 16:37 ` [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11 Alessandro Rubini
2012-06-28 20:06   ` H. Peter Anvin
2012-07-01 10:44   ` Alessandro Rubini
2012-07-01 10:59     ` Russell King - ARM Linux
2012-07-01 11:58       ` Jassi Brar
2012-07-01 12:10         ` Russell King - ARM Linux
2012-07-01 13:11           ` Jassi Brar
2012-07-01 11:04     ` Alessandro Rubini
2012-07-01 11:11       ` Russell King - ARM Linux
2012-07-01 14:13     ` H. Peter Anvin
2012-07-02 16:58       ` Arnd Bergmann
2012-07-02 18:05         ` Mark Brown
2012-07-02 18:07           ` H. Peter Anvin
2012-07-02 18:33             ` Mark Brown
2012-07-02 19:41               ` H. Peter Anvin
2012-07-03 11:05                 ` Mark Brown
2012-07-03 13:00     ` Alessandro Rubini
2012-07-03 13:19       ` Arnd Bergmann
2012-07-03 19:34         ` Russell King - ARM Linux
2012-08-05 20:28           ` Linus Walleij
2012-08-07 10:06             ` Kukjin Kim
2012-07-03 13:26       ` Alessandro Rubini
2012-07-03 19:46         ` Russell King - ARM Linux
2012-05-28 16:38 ` [PATCH V2 6/6] drivers/amba: add support for a PCI bridge Alessandro Rubini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).