| Message ID | 20120124160044.GM19798@mudshark.cambridge.arm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 24, 2012 at 04:00:44PM +0000, Will Deacon wrote: > Hi Russell, > > On Fri, Jan 20, 2012 at 09:29:30AM +0000, Russell King - ARM Linux wrote: > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > arch/arm/mach-realview/core.h | 20 ++++----------- > > arch/arm/mach-realview/realview_eb.c | 38 +++++++++++++++--------------- > > arch/arm/mach-realview/realview_pb1176.c | 38 +++++++++++++++--------------- > > arch/arm/mach-realview/realview_pb11mp.c | 38 +++++++++++++++--------------- > > arch/arm/mach-realview/realview_pba8.c | 38 +++++++++++++++--------------- > > arch/arm/mach-realview/realview_pbx.c | 38 +++++++++++++++--------------- > > 6 files changed, 100 insertions(+), 110 deletions(-) > > After applying this patch, I get compile-time errors in realview_*.c. > E.g.: > > arch/arm/mach-realview/realview_pb1176.c:158:1: error: ‘AACI’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:159:1: error: ‘MMCI0’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:160:1: error: ‘KMI0’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:161:1: error: ‘KMI1’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:162:1: error: ‘PB1176_UART4’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:165:1: error: ‘PB1176_SMC’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:166:1: error: ‘SCTL’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:167:1: error: ‘PB1176_WATCHDOG’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:168:1: error: ‘PB1176_GPIO0’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:169:1: error: ‘GPIO1’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:170:1: error: ‘GPIO2’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:171:1: error: ‘PB1176_RTC’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:172:1: error: ‘SCI’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:173:1: error: ‘PB1176_UART0’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:174:1: error: ‘PB1176_UART1’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:175:1: error: ‘PB1176_UART2’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:176:1: error: ‘PB1176_UART3’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:177:1: error: ‘PB1176_SSP’ undeclared here (not in a function) > arch/arm/mach-realview/realview_pb1176.c:178:1: error: ‘PB1176_CLCD’ undeclared here (not in a function) > > > diff --git a/arch/arm/mach-realview/core.h b/arch/arm/mach-realview/core.h > > index 735b57a..b1c6097 100644 > > --- a/arch/arm/mach-realview/core.h > > +++ b/arch/arm/mach-realview/core.h > > @@ -28,21 +28,11 @@ > > #include <asm/setup.h> > > #include <asm/leds.h> > > > > -#define AMBA_DEVICE(name,busid,base,plat) \ > > -static struct amba_device name##_device = { \ > > - .dev = { \ > > - .coherent_dma_mask = ~0, \ > > - .init_name = busid, \ > > - .platform_data = plat, \ > > - }, \ > > - .res = { \ > > - .start = REALVIEW_##base##_BASE, \ > > - .end = (REALVIEW_##base##_BASE) + SZ_4K - 1, \ > > - .flags = IORESOURCE_MEM, \ > > - }, > > Which is because the old AMBA_DEVICE macro munges REALVIEW_ and _BASE > around the peripheral identifier... > > > +#define APB_DEVICE(name, busid, base, plat) \ > > +static AMBA_APB_DEVICE(name, busid, 0, base, base##_IRQ, plat) > > + > > +#define AHB_DEVICE(name, busid, base, plat) \ > > +static AMBA_AHB_DEVICE(name, busid, 0, base, base##_IRQ, plat) > > ...whereas the new macros just use base directly. > > Fixing the macro solves the build problem: > > > diff --git a/arch/arm/mach-realview/core.h b/arch/arm/mach-realview/core.h > index b1c6097..f8f2c0a 100644 > --- a/arch/arm/mach-realview/core.h > +++ b/arch/arm/mach-realview/core.h > @@ -29,10 +29,10 @@ > #include <asm/leds.h> > > #define APB_DEVICE(name, busid, base, plat) \ > -static AMBA_APB_DEVICE(name, busid, 0, base, base##_IRQ, plat) > +static AMBA_APB_DEVICE(name, busid, 0, REALVIEW_##base##_BASE, base##_IRQ, plat) > > #define AHB_DEVICE(name, busid, base, plat) \ > -static AMBA_AHB_DEVICE(name, busid, 0, base, base##_IRQ, plat) > +static AMBA_AHB_DEVICE(name, busid, 0, REALVIEW_##base##_BASE, base##_IRQ, plat) > > struct machine_desc; > > > but then I see a warning during boot: > > [ 1.669654] ------------[ cut here ]------------ > [ 1.684021] WARNING: at drivers/amba/bus.c:514 amba_device_add+0x1b4/0x1d0() > [ 1.705585] Modules linked in: > [ 1.715195] [<c0013a00>] (unwind_backtrace+0x0/0xfc) from [<c03d1350>] (dump_stack+0x20/0x24) > [ 1.741288] [<c03d1350>] (dump_stack+0x20/0x24) from [<c001f9b8>] (warn_slowpath_common+0x5c/0x74) > [ 1.768689] [<c001f9b8>] (warn_slowpath_common+0x5c/0x74) from [<c001f9fc>] (warn_slowpath_null+0x2c/0x34) > [ 1.798165] [<c001f9fc>] (warn_slowpath_null+0x2c/0x34) from [<c023dcd4>] (amba_device_add+0x1b4/0x1d0) > [ 1.826857] [<c023dcd4>] (amba_device_add+0x1b4/0x1d0) from [<c023dd84>] (amba_device_register+0x94/0xc4) > [ 1.856106] [<c023dd84>] (amba_device_register+0x94/0xc4) from [<c0551934>] (realview_pb1176_init+0x74/0xac) > [ 1.886120] [<c0551934>] (realview_pb1176_init+0x74/0xac) from [<c054d60c>] (customize_machine+0x24/0x30) > [ 1.915340] [<c054d60c>] (customize_machine+0x24/0x30) from [<c000879c>] (do_one_initcall+0x48/0x1a0) > [ 1.943505] [<c000879c>] (do_one_initcall+0x48/0x1a0) from [<c054b870>] (kernel_init+0x80/0x128) > [ 1.970378] [<c054b870>] (kernel_init+0x80/0x128) from [<c000ea78>] (kernel_thread_exit+0x0/0x8) > [ 1.997192] ---[ end trace 1b75b31a2719ed1e ]--- > > I suspect that comes from PB1176_GPIO0_IRQ being -1. It seems you've also tested the code which detects -1 IRQs too, which is good. PB1176 needs that GPIO0_IRQ fixing, but I suspect passing zero may not be entirely a good thing for it as request_irq(0,...) probably succeeds there. I think that needs fixing before this warning can go away. In the mean time, it's just a warning, and its saying there's something wrong there which needs fixing but without anything currently broken. So it's working as designed.
On 01/24/2012 06:26 PM, Will Deacon wrote: > > Right. I took a quick look at the TRM for the PB1176, and I think we do have > an interrupt for GPIO0, it's just on the other GIC. This patch should do the > trick, but I'm not sure what I can do to tickle the GPIO stuff anyway: For a simple test just insert/remove the MMC card and see it add/remove the card (appear in console, and /proc/interrupts) Or do you mean tickle GPIO0? That is more tricky I think... Yours, Linus Walleij
On Tue, Jan 24, 2012 at 05:26:00PM +0000, Will Deacon wrote: > Right. I took a quick look at the TRM for the PB1176, and I think we do have > an interrupt for GPIO0, it's just on the other GIC. This patch should do the > trick, but I'm not sure what I can do to tickle the GPIO stuff anyway: > > > diff --git a/arch/arm/mach-realview/include/mach/irqs-pb1176.h b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > index 5c3c625..708f841 100644 > --- a/arch/arm/mach-realview/include/mach/irqs-pb1176.h > +++ b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > @@ -40,6 +40,7 @@ > #define IRQ_DC1176_L2CC (IRQ_DC1176_GIC_START + 13) > #define IRQ_DC1176_RTC (IRQ_DC1176_GIC_START + 14) > #define IRQ_DC1176_CLCD (IRQ_DC1176_GIC_START + 15) /* CLCD controller */ > +#define IRQ_DC1176_GPIO0 (IRQ_DC1176_GIC_START + 16) > #define IRQ_DC1176_SSP (IRQ_DC1176_GIC_START + 17) /* SSP port */ > #define IRQ_DC1176_UART0 (IRQ_DC1176_GIC_START + 18) /* UART 0 on development chip */ > #define IRQ_DC1176_UART1 (IRQ_DC1176_GIC_START + 19) /* UART 1 on development chip */ > @@ -73,7 +74,6 @@ > #define IRQ_PB1176_DMAC (IRQ_PB1176_GIC_START + 24) /* DMA controller */ > #define IRQ_PB1176_RTC (IRQ_PB1176_GIC_START + 25) /* Real Time Clock */ > > -#define IRQ_PB1176_GPIO0 -1 > #define IRQ_PB1176_SCTL -1 > > #define NR_GIC_PB1176 2 > diff --git a/arch/arm/mach-realview/realview_pb1176.c b/arch/arm/mach-realview/realview_pb1176.c > index 1b6e60c..b1d7caf 100644 > --- a/arch/arm/mach-realview/realview_pb1176.c > +++ b/arch/arm/mach-realview/realview_pb1176.c > @@ -143,7 +143,7 @@ static struct pl022_ssp_controller ssp0_plat_data = { > #define PB1176_CLCD_IRQ { IRQ_DC1176_CLCD } > #define SCTL_IRQ { } > #define PB1176_WATCHDOG_IRQ { IRQ_DC1176_WATCHDOG } > -#define PB1176_GPIO0_IRQ { IRQ_PB1176_GPIO0 } > +#define PB1176_GPIO0_IRQ { IRQ_DC1176_GPIO0 } > #define GPIO1_IRQ { IRQ_PB1176_GPIO1 } > #define PB1176_RTC_IRQ { IRQ_DC1176_RTC } > #define SCI_IRQ { IRQ_PB1176_SCI } I guess we should believe the TRM on this - can you put this in the patch system please? Thanks.
On Tue, Jan 24, 2012 at 09:45:31PM +0000, Russell King - ARM Linux wrote: > On Tue, Jan 24, 2012 at 05:26:00PM +0000, Will Deacon wrote: > > diff --git a/arch/arm/mach-realview/include/mach/irqs-pb1176.h b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > > index 5c3c625..708f841 100644 > > --- a/arch/arm/mach-realview/include/mach/irqs-pb1176.h > > +++ b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > > @@ -40,6 +40,7 @@ > > #define IRQ_DC1176_L2CC (IRQ_DC1176_GIC_START + 13) > > #define IRQ_DC1176_RTC (IRQ_DC1176_GIC_START + 14) > > #define IRQ_DC1176_CLCD (IRQ_DC1176_GIC_START + 15) /* CLCD controller */ > > +#define IRQ_DC1176_GPIO0 (IRQ_DC1176_GIC_START + 16) > > #define IRQ_DC1176_SSP (IRQ_DC1176_GIC_START + 17) /* SSP port */ > > #define IRQ_DC1176_UART0 (IRQ_DC1176_GIC_START + 18) /* UART 0 on development chip */ > > #define IRQ_DC1176_UART1 (IRQ_DC1176_GIC_START + 19) /* UART 1 on development chip */ > > @@ -73,7 +74,6 @@ > > #define IRQ_PB1176_DMAC (IRQ_PB1176_GIC_START + 24) /* DMA controller */ > > #define IRQ_PB1176_RTC (IRQ_PB1176_GIC_START + 25) /* Real Time Clock */ > > > > -#define IRQ_PB1176_GPIO0 -1 > > #define IRQ_PB1176_SCTL -1 > > > > #define NR_GIC_PB1176 2 > > diff --git a/arch/arm/mach-realview/realview_pb1176.c b/arch/arm/mach-realview/realview_pb1176.c > > index 1b6e60c..b1d7caf 100644 > > --- a/arch/arm/mach-realview/realview_pb1176.c > > +++ b/arch/arm/mach-realview/realview_pb1176.c > > @@ -143,7 +143,7 @@ static struct pl022_ssp_controller ssp0_plat_data = { > > #define PB1176_CLCD_IRQ { IRQ_DC1176_CLCD } > > #define SCTL_IRQ { } > > #define PB1176_WATCHDOG_IRQ { IRQ_DC1176_WATCHDOG } > > -#define PB1176_GPIO0_IRQ { IRQ_PB1176_GPIO0 } > > +#define PB1176_GPIO0_IRQ { IRQ_DC1176_GPIO0 } > > #define GPIO1_IRQ { IRQ_PB1176_GPIO1 } > > #define PB1176_RTC_IRQ { IRQ_DC1176_RTC } > > #define SCI_IRQ { IRQ_PB1176_SCI } > > I guess we should believe the TRM on this - can you put this in the patch > system please? Sure. Which branch shall I take it against (before or after your amba changes)? Will
On Wed, Jan 25, 2012 at 09:58:00AM +0000, Will Deacon wrote: > Sure. Which branch shall I take it against (before or after your amba > changes)? If it's before them, we can think about putting it in as a fix during this -rc independently of the rest of the changes. If it's after, then it'll probably add a conflict. So, it'll be much easier to have it before, and I'll update what's necessary in the amba branch.
On Wed, Jan 25, 2012 at 10:22:02AM +0000, Russell King - ARM Linux wrote: > On Wed, Jan 25, 2012 at 09:58:00AM +0000, Will Deacon wrote: > > Sure. Which branch shall I take it against (before or after your amba > > changes)? > > If it's before them, we can think about putting it in as a fix during > this -rc independently of the rest of the changes. If it's after, > then it'll probably add a conflict. > > So, it'll be much easier to have it before, and I'll update what's > necessary in the amba branch. Okey doke, I've submitted it as 7300/1 against 3.3-rc1. Will
On Wed, Jan 25, 2012 at 11:06:47AM +0000, Russell King - ARM Linux wrote: > Right, so with the stack of amba patches on top, it looks like this, > which to me looks sane. I haven't build-tested it though. Will, Did you see this? Any comment? Thanks. > > diff --git a/arch/arm/mach-realview/include/mach/irqs-pb1176.h b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > index 5c3c625..708f841 100644 > --- a/arch/arm/mach-realview/include/mach/irqs-pb1176.h > +++ b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > @@ -40,6 +40,7 @@ > #define IRQ_DC1176_L2CC (IRQ_DC1176_GIC_START + 13) > #define IRQ_DC1176_RTC (IRQ_DC1176_GIC_START + 14) > #define IRQ_DC1176_CLCD (IRQ_DC1176_GIC_START + 15) /* CLCD controller */ > +#define IRQ_DC1176_GPIO0 (IRQ_DC1176_GIC_START + 16) > #define IRQ_DC1176_SSP (IRQ_DC1176_GIC_START + 17) /* SSP port */ > #define IRQ_DC1176_UART0 (IRQ_DC1176_GIC_START + 18) /* UART 0 on development chip */ > #define IRQ_DC1176_UART1 (IRQ_DC1176_GIC_START + 19) /* UART 1 on development chip */ > @@ -73,7 +74,6 @@ > #define IRQ_PB1176_DMAC (IRQ_PB1176_GIC_START + 24) /* DMA controller */ > #define IRQ_PB1176_RTC (IRQ_PB1176_GIC_START + 25) /* Real Time Clock */ > > -#define IRQ_PB1176_GPIO0 -1 > #define IRQ_PB1176_SCTL -1 > > #define NR_GIC_PB1176 2 > diff --git a/arch/arm/mach-realview/realview_pb1176.c b/arch/arm/mach-realview/realview_pb1176.c > index 1b6e60c..b1d7caf 100644 > --- a/arch/arm/mach-realview/realview_pb1176.c > +++ b/arch/arm/mach-realview/realview_pb1176.c > @@ -143,7 +143,7 @@ static struct pl022_ssp_controller ssp0_plat_data = { > #define PB1176_CLCD_IRQ { IRQ_DC1176_CLCD } > #define SCTL_IRQ { } > #define PB1176_WATCHDOG_IRQ { IRQ_DC1176_WATCHDOG } > -#define PB1176_GPIO0_IRQ { IRQ_PB1176_GPIO0 } > +#define PB1176_GPIO0_IRQ { IRQ_DC1176_GPIO0 } > #define GPIO1_IRQ { IRQ_PB1176_GPIO1 } > #define PB1176_RTC_IRQ { IRQ_DC1176_RTC } > #define SCI_IRQ { IRQ_PB1176_SCI } > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Jan 26, 2012 at 05:25:27PM +0000, Russell King - ARM Linux wrote: > On Wed, Jan 25, 2012 at 11:06:47AM +0000, Russell King - ARM Linux wrote: > > Right, so with the stack of amba patches on top, it looks like this, > > which to me looks sane. I haven't build-tested it though. > > Will, Hi Russell, > Did you see this? Any comment? Sorry, this somehow got buried in other mail. > > > > diff --git a/arch/arm/mach-realview/include/mach/irqs-pb1176.h b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > > index 5c3c625..708f841 100644 > > --- a/arch/arm/mach-realview/include/mach/irqs-pb1176.h > > +++ b/arch/arm/mach-realview/include/mach/irqs-pb1176.h > > @@ -40,6 +40,7 @@ > > #define IRQ_DC1176_L2CC (IRQ_DC1176_GIC_START + 13) > > #define IRQ_DC1176_RTC (IRQ_DC1176_GIC_START + 14) > > #define IRQ_DC1176_CLCD (IRQ_DC1176_GIC_START + 15) /* CLCD controller */ > > +#define IRQ_DC1176_GPIO0 (IRQ_DC1176_GIC_START + 16) > > #define IRQ_DC1176_SSP (IRQ_DC1176_GIC_START + 17) /* SSP port */ > > #define IRQ_DC1176_UART0 (IRQ_DC1176_GIC_START + 18) /* UART 0 on development chip */ > > #define IRQ_DC1176_UART1 (IRQ_DC1176_GIC_START + 19) /* UART 1 on development chip */ > > @@ -73,7 +74,6 @@ > > #define IRQ_PB1176_DMAC (IRQ_PB1176_GIC_START + 24) /* DMA controller */ > > #define IRQ_PB1176_RTC (IRQ_PB1176_GIC_START + 25) /* Real Time Clock */ > > > > -#define IRQ_PB1176_GPIO0 -1 > > #define IRQ_PB1176_SCTL -1 > > > > #define NR_GIC_PB1176 2 > > diff --git a/arch/arm/mach-realview/realview_pb1176.c b/arch/arm/mach-realview/realview_pb1176.c > > index 1b6e60c..b1d7caf 100644 > > --- a/arch/arm/mach-realview/realview_pb1176.c > > +++ b/arch/arm/mach-realview/realview_pb1176.c > > @@ -143,7 +143,7 @@ static struct pl022_ssp_controller ssp0_plat_data = { > > #define PB1176_CLCD_IRQ { IRQ_DC1176_CLCD } > > #define SCTL_IRQ { } > > #define PB1176_WATCHDOG_IRQ { IRQ_DC1176_WATCHDOG } > > -#define PB1176_GPIO0_IRQ { IRQ_PB1176_GPIO0 } > > +#define PB1176_GPIO0_IRQ { IRQ_DC1176_GPIO0 } > > #define GPIO1_IRQ { IRQ_PB1176_GPIO1 } > > #define PB1176_RTC_IRQ { IRQ_DC1176_RTC } > > #define SCI_IRQ { IRQ_PB1176_SCI } This looks fine to me. It matches what I posted originally, which was the intention. Acked-by: Will Deacon <will.deacon@arm.com> Will
On Thu, Jan 26, 2012 at 05:37:21PM +0000, Will Deacon wrote:
> Acked-by: Will Deacon <will.deacon@arm.com>
Thanks, I'll add that to patch 21 with the fix.
diff --git a/arch/arm/mach-realview/core.h b/arch/arm/mach-realview/core.h index b1c6097..f8f2c0a 100644 --- a/arch/arm/mach-realview/core.h +++ b/arch/arm/mach-realview/core.h @@ -29,10 +29,10 @@ #include <asm/leds.h> #define APB_DEVICE(name, busid, base, plat) \ -static AMBA_APB_DEVICE(name, busid, 0, base, base##_IRQ, plat) +static AMBA_APB_DEVICE(name, busid, 0, REALVIEW_##base##_BASE, base##_IRQ, plat) #define AHB_DEVICE(name, busid, base, plat) \ -static AMBA_AHB_DEVICE(name, busid, 0, base, base##_IRQ, plat) +static AMBA_AHB_DEVICE(name, busid, 0, REALVIEW_##base##_BASE, base##_IRQ, plat) struct machine_desc;