2012-05-28 16:38:18

by Alessandro Rubini

[permalink] [raw]
Subject: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11

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 <[email protected]>
Acked-by: Giancarlo Asnaghi <[email protected]>
---
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


2012-06-28 20:08:18

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11

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 <[email protected]>
> Acked-by: Giancarlo Asnaghi <[email protected]>
> ---
> 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 <[email protected]>

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.


2012-08-05 20:28:41

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11

On Tue, Jul 3, 2012 at 9:34 PM, Russell King - ARM Linux
<[email protected]> 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

2012-08-07 10:06:42

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH V2 5/6] x86: add CONFIG_ARM_AMBA, selected by STA2X11

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 <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.