* [patch 2/4] [ARM] mmp: support marvell ARMADA610 @ 2009-12-07 6:19 Haojian Zhuang 2009-12-29 3:49 ` Eric Miao 0 siblings, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2009-12-07 6:19 UTC (permalink / raw) To: linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2009-12-07 6:19 [patch 2/4] [ARM] mmp: support marvell ARMADA610 Haojian Zhuang @ 2009-12-29 3:49 ` Eric Miao 2009-12-31 14:42 ` Haojian Zhuang 0 siblings, 1 reply; 15+ messages in thread From: Eric Miao @ 2009-12-29 3:49 UTC (permalink / raw) To: linux-arm-kernel Haojian, Several suggestions: 1. entry-macro.S, could we simplify (and improve the performance a bit) to something like (haven't tested yet - let me know the result): .macro get_irqnr_preamble, base, tmp mrc p15, 0, \tmp, c0, c0, 0 @ CPUID mov \tmp, \tmp, lsr #4 and \tmp, \tmp, #0xfff cmp \tmp, #0x581 @ MMP2 moveq \base, #MMP2_ICU_PJ4_IRQ_SEL movne \base, #ICU_AP_IRQ_SEL_INT_NUM .endm .macro arch_ret_to_user, tmp1, tmp2 .endm .macro get_irqnr_and_base, irqnr, irqstat, base, tmp ldr \tmp, [\base, #0] and \irqnr, \tmp, #0x3f @ Interrupt Number tst \tmp, #(1 << 6) @ Interrupt Pending ? .endm 2. arch/arm/mach-mmp/irq.c We could well separate irq.c to irq-pxa168.c and irq-mmp2.c since the difference looks much. 3. ARMADA610 - I'd still prefer it being called MMP2 4. in timer_config() would be nicer to have: ccr &= (cpu_is_mmp2()) ? TMR_CCR_CS_0(0) : TMR_CCR_CS_0(3) a little bit cleaner - but it is nit picking indeed :) ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2009-12-29 3:49 ` Eric Miao @ 2009-12-31 14:42 ` Haojian Zhuang 2010-01-01 3:46 ` Eric Miao 0 siblings, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2009-12-31 14:42 UTC (permalink / raw) To: linux-arm-kernel On Mon, Dec 28, 2009 at 10:49 PM, Eric Miao <eric.y.miao@gmail.com> wrote: > Haojian, > > Several suggestions: > > 1. entry-macro.S, could we simplify (and improve the performance a bit) to > something like (haven't tested yet - let me know the result): > > ? ? ? ?.macro ?get_irqnr_preamble, base, tmp > ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID > ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 > ? ? ? ?and ? ? \tmp, \tmp, #0xfff We shouldn't use #0xfff in and instruction. Immediate number shouldn't beyond 8bit. We have to use ldr instruction or multiple instrunctions. > ? ? ? ?cmp ? ? \tmp, #0x581 ? ? ? ? ? ? ? ? ? ?@ MMP2 > ? ? ? ?moveq ? \base, #MMP2_ICU_PJ4_IRQ_SEL > ? ? ? ?movne ? \base, #ICU_AP_IRQ_SEL_INT_NUM > ? ? ? ?.endm > > ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 > ? ? ? ?.endm > > ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp > ? ? ? ?ldr ? ? \tmp, [\base, #0] > ? ? ? ?and ? ? \irqnr, \tmp, #0x3f ? ? ? ? ? ? @ Interrupt Number > ? ? ? ?tst ? ? \tmp, #(1 << 6) ? ? ? ? ? ? ? ? @ Interrupt Pending ? > ? ? ? ?.endm > I don't like this way. Although more jump instructions are used in irq entry, it can be extended easier for supporting more silicons. I've updated these patches in attached mail. Thanks Haojian -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-mmp-support-marvell-MMP2.patch Type: text/x-patch Size: 21538 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091231/acb4ede1/attachment-0004.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002-mmp-add-device-support-in-mmp2.patch Type: text/x-patch Size: 17396 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091231/acb4ede1/attachment-0005.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0003-mmp-support-flint-development-board.patch Type: text/x-patch Size: 3889 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091231/acb4ede1/attachment-0006.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0004--ARM-mmp-add-mmp2-configuration.patch Type: text/x-patch Size: 30784 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091231/acb4ede1/attachment-0007.bin> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2009-12-31 14:42 ` Haojian Zhuang @ 2010-01-01 3:46 ` Eric Miao 2010-01-04 2:27 ` Haojian Zhuang 0 siblings, 1 reply; 15+ messages in thread From: Eric Miao @ 2010-01-01 3:46 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 31, 2009 at 10:42 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote: > On Mon, Dec 28, 2009 at 10:49 PM, Eric Miao <eric.y.miao@gmail.com> wrote: >> Haojian, >> >> Several suggestions: >> >> 1. entry-macro.S, could we simplify (and improve the performance a bit) to >> something like (haven't tested yet - let me know the result): >> >> ? ? ? ?.macro ?get_irqnr_preamble, base, tmp >> ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID >> ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 >> ? ? ? ?and ? ? \tmp, \tmp, #0xfff > > We shouldn't use #0xfff in and instruction. Immediate number shouldn't > beyond 8bit. We have to use ldr instruction or multiple instrunctions. Well, 0xff0 then, or ldr, whichever I don't mind. > >> ? ? ? ?cmp ? ? \tmp, #0x581 ? ? ? ? ? ? ? ? ? ?@ MMP2 >> ? ? ? ?moveq ? \base, #MMP2_ICU_PJ4_IRQ_SEL >> ? ? ? ?movne ? \base, #ICU_AP_IRQ_SEL_INT_NUM >> ? ? ? ?.endm >> >> ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 >> ? ? ? ?.endm >> >> ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp >> ? ? ? ?ldr ? ? \tmp, [\base, #0] >> ? ? ? ?and ? ? \irqnr, \tmp, #0x3f ? ? ? ? ? ? @ Interrupt Number >> ? ? ? ?tst ? ? \tmp, #(1 << 6) ? ? ? ? ? ? ? ? @ Interrupt Pending ? >> ? ? ? ?.endm >> > > I don't like this way. Although more jump instructions are used in irq > entry, it can be extended easier for supporting more silicons. > It really hurt performance a lot. While the difference between MMP series is just the offset of the IRQ_NUM register, it could be calculated in get_irqnr_preamble, which doesn't have to be executed each time in the IRQ handling loop. See arch/arm/kernel/entry-armv.S PXA is different since PXA25x checks the IRQ status by MMIO register while PXA27x and above check that by co-processor. > I've updated these patches in attached mail. > Thanks, I'll take a look and happy new year. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-01 3:46 ` Eric Miao @ 2010-01-04 2:27 ` Haojian Zhuang 2010-01-04 2:49 ` Eric Miao 0 siblings, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2010-01-04 2:27 UTC (permalink / raw) To: linux-arm-kernel On Thu, Dec 31, 2009 at 10:46 PM, Eric Miao <eric.y.miao@gmail.com> wrote: > On Thu, Dec 31, 2009 at 10:42 PM, Haojian Zhuang > <haojian.zhuang@gmail.com> wrote: >> On Mon, Dec 28, 2009 at 10:49 PM, Eric Miao <eric.y.miao@gmail.com> wrote: >>> Haojian, >>> >>> Several suggestions: >>> >>> 1. entry-macro.S, could we simplify (and improve the performance a bit) to >>> something like (haven't tested yet - let me know the result): >>> >>> ? ? ? ?.macro ?get_irqnr_preamble, base, tmp >>> ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID >>> ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 >>> ? ? ? ?and ? ? \tmp, \tmp, #0xfff >> >> We shouldn't use #0xfff in and instruction. Immediate number shouldn't >> beyond 8bit. We have to use ldr instruction or multiple instrunctions. > > Well, 0xff0 then, or ldr, whichever I don't mind. > >> >>> ? ? ? ?cmp ? ? \tmp, #0x581 ? ? ? ? ? ? ? ? ? ?@ MMP2 >>> ? ? ? ?moveq ? \base, #MMP2_ICU_PJ4_IRQ_SEL >>> ? ? ? ?movne ? \base, #ICU_AP_IRQ_SEL_INT_NUM >>> ? ? ? ?.endm >>> >>> ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 >>> ? ? ? ?.endm >>> >>> ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp >>> ? ? ? ?ldr ? ? \tmp, [\base, #0] >>> ? ? ? ?and ? ? \irqnr, \tmp, #0x3f ? ? ? ? ? ? @ Interrupt Number >>> ? ? ? ?tst ? ? \tmp, #(1 << 6) ? ? ? ? ? ? ? ? @ Interrupt Pending ? >>> ? ? ? ?.endm >>> >> >> I don't like this way. Although more jump instructions are used in irq >> entry, it can be extended easier for supporting more silicons. >> > > It really hurt performance a lot. While the difference between MMP series > is just the offset of the IRQ_NUM register, it could be calculated in > get_irqnr_preamble, which doesn't have to be executed each time > in the IRQ handling loop. See arch/arm/kernel/entry-armv.S get_irqnr_preamble can only carries one base parameter and one tmp parameter. base parameter is already used for interrupt control register. Only one tmp parameter is left for use. If we want to check chip id, one parameter is not enought. So putting these code into get_irqnr_and_base macro is more reasonable. Happy new year! :) Best Regards Haojian ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 2:27 ` Haojian Zhuang @ 2010-01-04 2:49 ` Eric Miao 2010-01-04 3:29 ` Haojian Zhuang 2010-01-04 10:30 ` Russell King - ARM Linux 0 siblings, 2 replies; 15+ messages in thread From: Eric Miao @ 2010-01-04 2:49 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 4, 2010 at 10:27 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote: > On Thu, Dec 31, 2009 at 10:46 PM, Eric Miao <eric.y.miao@gmail.com> wrote: >> On Thu, Dec 31, 2009 at 10:42 PM, Haojian Zhuang >> <haojian.zhuang@gmail.com> wrote: >>> On Mon, Dec 28, 2009 at 10:49 PM, Eric Miao <eric.y.miao@gmail.com> wrote: >>>> Haojian, >>>> >>>> Several suggestions: >>>> >>>> 1. entry-macro.S, could we simplify (and improve the performance a bit) to >>>> something like (haven't tested yet - let me know the result): >>>> >>>> ? ? ? ?.macro ?get_irqnr_preamble, base, tmp >>>> ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID >>>> ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 >>>> ? ? ? ?and ? ? \tmp, \tmp, #0xfff >>> >>> We shouldn't use #0xfff in and instruction. Immediate number shouldn't >>> beyond 8bit. We have to use ldr instruction or multiple instrunctions. >> >> Well, 0xff0 then, or ldr, whichever I don't mind. >> >>> >>>> ? ? ? ?cmp ? ? \tmp, #0x581 ? ? ? ? ? ? ? ? ? ?@ MMP2 >>>> ? ? ? ?moveq ? \base, #MMP2_ICU_PJ4_IRQ_SEL >>>> ? ? ? ?movne ? \base, #ICU_AP_IRQ_SEL_INT_NUM >>>> ? ? ? ?.endm >>>> >>>> ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 >>>> ? ? ? ?.endm >>>> >>>> ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp >>>> ? ? ? ?ldr ? ? \tmp, [\base, #0] >>>> ? ? ? ?and ? ? \irqnr, \tmp, #0x3f ? ? ? ? ? ? @ Interrupt Number >>>> ? ? ? ?tst ? ? \tmp, #(1 << 6) ? ? ? ? ? ? ? ? @ Interrupt Pending ? >>>> ? ? ? ?.endm >>>> >>> >>> I don't like this way. Although more jump instructions are used in irq >>> entry, it can be extended easier for supporting more silicons. >>> >> >> It really hurt performance a lot. While the difference between MMP series >> is just the offset of the IRQ_NUM register, it could be calculated in >> get_irqnr_preamble, which doesn't have to be executed each time >> in the IRQ handling loop. See arch/arm/kernel/entry-armv.S > > get_irqnr_preamble can only carries one base parameter and one tmp > parameter. base parameter is already used for interrupt control > register. Only one tmp parameter is left for use. If we want to check > chip id, one parameter is not enought. > I'm a bit confused, but those are just registers that the macro is able to use, not something as parameters?? ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 2:49 ` Eric Miao @ 2010-01-04 3:29 ` Haojian Zhuang 2010-01-04 4:27 ` Eric Miao 2010-01-04 10:30 ` Russell King - ARM Linux 1 sibling, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2010-01-04 3:29 UTC (permalink / raw) To: linux-arm-kernel >>> It really hurt performance a lot. While the difference between MMP series >>> is just the offset of the IRQ_NUM register, it could be calculated in >>> get_irqnr_preamble, which doesn't have to be executed each time >>> in the IRQ handling loop. See arch/arm/kernel/entry-armv.S >> >> get_irqnr_preamble can only carries one base parameter and one tmp >> parameter. base parameter is already used for interrupt control >> register. Only one tmp parameter is left for use. If we want to check >> chip id, one parameter is not enought. >> > > I'm a bit confused, but those are just registers that the macro is able to use, > not something as parameters?? > I could totaly use two parameters / registers. One is already used for irq controller. Loading the value from irq controller register needs one register. Storing camparison needs another register. At least, I need two registers except irq controller register. But there's no enough parameter / register in get_irqnr_preamble. Thanks Haojian ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 3:29 ` Haojian Zhuang @ 2010-01-04 4:27 ` Eric Miao 2010-01-04 10:15 ` Haojian Zhuang 0 siblings, 1 reply; 15+ messages in thread From: Eric Miao @ 2010-01-04 4:27 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 4, 2010 at 11:29 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote: >>>> It really hurt performance a lot. While the difference between MMP series >>>> is just the offset of the IRQ_NUM register, it could be calculated in >>>> get_irqnr_preamble, which doesn't have to be executed each time >>>> in the IRQ handling loop. See arch/arm/kernel/entry-armv.S >>> >>> get_irqnr_preamble can only carries one base parameter and one tmp >>> parameter. base parameter is already used for interrupt control >>> register. Only one tmp parameter is left for use. If we want to check >>> chip id, one parameter is not enought. >>> >> >> I'm a bit confused, but those are just registers that the macro is able to use, >> not something as parameters?? >> > > I could totaly use two parameters / registers. One is already used for > irq controller. Loading the value from irq controller register needs > one register. Storing camparison needs another register. At least, I > need two registers except irq controller register. But there's no > enough parameter / register in get_irqnr_preamble. > Two solutions: 1. use the first two nibbles 0x58, 0x80, 0x84 to decide if it's mmp1 or mmp2, e.g. .macro get_irqnr_preamble, base, tmp mrc p15, 0, \tmp, c0, c0, 0 @ CPUID mov \tmp, \tmp, lsr #4 and \tmp, \tmp, #0xff0 cmp \tmp, #0x580 ldrne \base, =ICU_AP_IRQ_SEL_INT_NUM ldreq \base, =ICU_MMP2_PJ4_IRQ_SEL .endm .macro arch_ret_to_user, tmp1, tmp2 .endm .macro get_irqnr_and_base, irqnr, irqstat, base, tmp ldr \tmp, [\base] @ AP INT SEL register and \irqnr, \tmp, #0x3f tst \tmp, #(1 << 6) .endm 2. use \base temporarily, which is un-necessary and less preferable: .macro get_irqnr_preamble, base, tmp mrc p15, 0, \tmp, c0, c0, 0 @ CPUID mov \tmp, \tmp, lsr #4 and \tmp, \tmp, #0xff0 and \tmp, \tmp, #0x00f ldr \base, =0x581 cmp \tmp, \base ldreq \base, =ICU_MMP2_PJ4_IRQ_SEL mov \base, #0x840 cmp \tmp, \base ldreq \base, =ICU_PXA168_IRQ_SEL mov \base, #0x800 cmp \tmp, \base ldreq \base, =ICU_PXA910_IRQ_SEL /* panic other wise */ .endm .macro arch_ret_to_user, tmp1, tmp2 .endm .macro get_irqnr_and_base, irqnr, irqstat, base, tmp ldr \tmp, [\base] @ AP INT SEL register and \irqnr, \tmp, #0x3f tst \tmp, #(1 << 6) .endm ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 4:27 ` Eric Miao @ 2010-01-04 10:15 ` Haojian Zhuang 2010-01-04 10:27 ` Russell King - ARM Linux 0 siblings, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2010-01-04 10:15 UTC (permalink / raw) To: linux-arm-kernel On Sun, Jan 3, 2010 at 11:27 PM, Eric Miao <eric.y.miao@gmail.com> wrote: > On Mon, Jan 4, 2010 at 11:29 AM, Haojian Zhuang > <haojian.zhuang@gmail.com> wrote: >>>>> It really hurt performance a lot. While the difference between MMP series >>>>> is just the offset of the IRQ_NUM register, it could be calculated in >>>>> get_irqnr_preamble, which doesn't have to be executed each time >>>>> in the IRQ handling loop. See arch/arm/kernel/entry-armv.S >>>> >>>> get_irqnr_preamble can only carries one base parameter and one tmp >>>> parameter. base parameter is already used for interrupt control >>>> register. Only one tmp parameter is left for use. If we want to check >>>> chip id, one parameter is not enought. >>>> >>> >>> I'm a bit confused, but those are just registers that the macro is able to use, >>> not something as parameters?? >>> >> >> I could totaly use two parameters / registers. One is already used for >> irq controller. Loading the value from irq controller register needs >> one register. Storing camparison needs another register. At least, I >> need two registers except irq controller register. But there's no >> enough parameter / register in get_irqnr_preamble. >> > > Two solutions: > > 1. use the first two nibbles 0x58, 0x80, 0x84 to decide if it's mmp1 > or mmp2, e.g. > > ? ? ? ?.macro ?get_irqnr_preamble, base, tmp > ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID > ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 > ? ? ? ?and ? ? \tmp, \tmp, #0xff0 > ? ? ? ?cmp ? ? \tmp, #0x580 I have some concern on compressing 12-bit to 8-bit. Maybe it would not fit chips in future. > ? ? ? ?ldrne ? \base, =ICU_AP_IRQ_SEL_INT_NUM > ? ? ? ?ldreq ? \base, =ICU_MMP2_PJ4_IRQ_SEL > ? ? ? ?.endm > > ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 > ? ? ? ?.endm > > ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp > ? ? ? ?ldr ? ? \tmp, [\base] ? ? ? ? ? @ AP INT SEL register > ? ? ? ?and ? ? \irqnr, \tmp, #0x3f > ? ? ? ?tst ? ? \tmp, #(1 << 6) > ? ? ? ?.endm > It's wrong at here. Whatever we need to access ICU register in get_irqnr_and_base. .macro irq_handler get_irqnr_preamble r5, lr 1: get_irqnr_and_base r0, r6, r5, lr ... bne asm_do_IRQ If we just keep accessing ICU register in get_irqnr_preamble, we'll find lr register used in asm_do_IRQ. When the loop came back to get_irqnr_and_base(), we'll meet unpredicated value. So I think that there's some flaw on this optimization. Thanks Haojian ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 10:15 ` Haojian Zhuang @ 2010-01-04 10:27 ` Russell King - ARM Linux 2010-01-04 11:16 ` Haojian Zhuang 0 siblings, 1 reply; 15+ messages in thread From: Russell King - ARM Linux @ 2010-01-04 10:27 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 04, 2010 at 05:15:22AM -0500, Haojian Zhuang wrote: > On Sun, Jan 3, 2010 at 11:27 PM, Eric Miao <eric.y.miao@gmail.com> wrote: > > ? ? ? ?.macro ?get_irqnr_preamble, base, tmp > > ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID > > ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 > > ? ? ? ?and ? ? \tmp, \tmp, #0xff0 > > ? ? ? ?cmp ? ? \tmp, #0x580 > > I have some concern on compressing 12-bit to 8-bit. Maybe it would not > fit chips in future. The spec for immediate constants is: any 32-bit constant, which may be rotated by an even number of shift places to form a single 8-bit constant. So, 0x0ff00000 is legal (0xff rotated left 20). 0x07f800000 is not (0xff rotated left 19). 0xfc000003 is legal (0xff rotated left 26). This comes directly from the instruction coding, which is an 8-bit constant plus a 4-bit shift. > > ? ? ? ?ldrne ? \base, =ICU_AP_IRQ_SEL_INT_NUM > > ? ? ? ?ldreq ? \base, =ICU_MMP2_PJ4_IRQ_SEL > > ? ? ? ?.endm > > > > ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 > > ? ? ? ?.endm > > > > ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp > > ? ? ? ?ldr ? ? \tmp, [\base] ? ? ? ? ? @ AP INT SEL register > > ? ? ? ?and ? ? \irqnr, \tmp, #0x3f > > ? ? ? ?tst ? ? \tmp, #(1 << 6) > > ? ? ? ?.endm > > > It's wrong at here. Whatever we need to access ICU register in > get_irqnr_and_base. > > .macro irq_handler > get_irqnr_preamble r5, lr > 1: get_irqnr_and_base r0, r6, r5, lr > ... > bne asm_do_IRQ > > If we just keep accessing ICU register in get_irqnr_preamble, we'll > find lr register used in asm_do_IRQ. When the loop came back to > get_irqnr_and_base(), we'll meet unpredicated value. I don't see what you're getting at - Eric only uses the 'tmp' register as a local temporary in each macro. It's entirely local to that macro, and each macro doesn't care what value it had previously. The first thing that get_irqnr_and_base does is overwrite 'lr' with the value from the INT SEL register. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 10:27 ` Russell King - ARM Linux @ 2010-01-04 11:16 ` Haojian Zhuang 2010-01-05 7:30 ` Eric Miao 0 siblings, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2010-01-04 11:16 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 4, 2010 at 5:27 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Jan 04, 2010 at 05:15:22AM -0500, Haojian Zhuang wrote: >> On Sun, Jan 3, 2010 at 11:27 PM, Eric Miao <eric.y.miao@gmail.com> wrote: >> > ? ? ? ?.macro ?get_irqnr_preamble, base, tmp >> > ? ? ? ?mrc ? ? p15, 0, \tmp, c0, c0, 0 ? ? ? ? @ CPUID >> > ? ? ? ?mov ? ? \tmp, \tmp, lsr #4 >> > ? ? ? ?and ? ? \tmp, \tmp, #0xff0 >> > ? ? ? ?cmp ? ? \tmp, #0x580 >> >> I have some concern on compressing 12-bit to 8-bit. Maybe it would not >> fit chips in future. > > The spec for immediate constants is: any 32-bit constant, which may be > rotated by an even number of shift places to form a single 8-bit > constant. > > So, 0x0ff00000 is legal (0xff rotated left 20). ?0x07f800000 is not (0xff > rotated left 19). ?0xfc000003 is legal (0xff rotated left 26). > > This comes directly from the instruction coding, which is an 8-bit > constant plus a 4-bit shift. > >> > ? ? ? ?ldrne ? \base, =ICU_AP_IRQ_SEL_INT_NUM >> > ? ? ? ?ldreq ? \base, =ICU_MMP2_PJ4_IRQ_SEL >> > ? ? ? ?.endm >> > >> > ? ? ? ?.macro ?arch_ret_to_user, tmp1, tmp2 >> > ? ? ? ?.endm >> > >> > ? ? ? ?.macro ?get_irqnr_and_base, irqnr, irqstat, base, tmp >> > ? ? ? ?ldr ? ? \tmp, [\base] ? ? ? ? ? @ AP INT SEL register >> > ? ? ? ?and ? ? \irqnr, \tmp, #0x3f >> > ? ? ? ?tst ? ? \tmp, #(1 << 6) >> > ? ? ? ?.endm >> > >> It's wrong at here. Whatever we need to access ICU register in >> get_irqnr_and_base. >> >> ? ? ? ? .macro irq_handler >> ? ? ? ? get_irqnr_preamble ?r5, lr >> 1: ? ? get_irqnr_and_base ?r0, r6, r5, lr >> ? ? ? ? ... >> ? ? ? ? bne ?asm_do_IRQ >> >> If we just keep accessing ICU register in get_irqnr_preamble, we'll >> find lr register used in asm_do_IRQ. When the loop came back to >> get_irqnr_and_base(), we'll meet unpredicated value. > > I don't see what you're getting at - Eric only uses the 'tmp' register > as a local temporary in each macro. ?It's entirely local to that > macro, and each macro doesn't care what value it had previously. > > The first thing that get_irqnr_and_base does is overwrite 'lr' with > the value from the INT SEL register. > OK. Update the patches. -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001--ARM-mmp-support-marvell-MMP2.patch Type: text/x-patch Size: 21193 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100104/cfdd9ce8/attachment-0004.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0002--ARM-mmp-add-device-support-in-mmp2.patch Type: text/x-patch Size: 17402 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100104/cfdd9ce8/attachment-0005.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0003--ARM-mmp-support-flint-development-board.patch Type: text/x-patch Size: 3895 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100104/cfdd9ce8/attachment-0006.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0004--ARM-mmp-add-mmp2-configuration.patch Type: text/x-patch Size: 30784 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100104/cfdd9ce8/attachment-0007.bin> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 11:16 ` Haojian Zhuang @ 2010-01-05 7:30 ` Eric Miao 2010-01-05 9:13 ` Haojian Zhuang 0 siblings, 1 reply; 15+ messages in thread From: Eric Miao @ 2010-01-05 7:30 UTC (permalink / raw) To: linux-arm-kernel > OK. Update the patches. > Applied to branch 'mmp2' and made some changes as follows (please let me know your concerns and help test this): 1. irq-mmp2.c: separate the difference into different irq_chip and individual demux handler for those MUXed IRQs, naming is changed a bit as well. Having a single irq_chip and demux handler isn't performance friendly - having to go through a lengthy condition branches each time - and that's why irq_chip is invented. 2. merge patch 1, 2, 3 together so mmp2 can build (my build here complains about no machine_desc defined if flint support isn't there) And I'd like you to help co-maintaine MMP2 together with me, please kindly Ack if you're fine with the patch follows: commit 2004b15682f0f612b85088d730164e91fca2eec8 Author: Eric Miao <eric.y.miao@gmail.com> Date: Tue Jan 5 15:28:26 2010 +0800 MAINTAINERS: add maintainers for Marvell MMP2 (aka ARMADA610) support Cc: Haojian Zhuang <haojian.zhuang@marvell.com> Signed-off-by: Eric Miao <eric.y.miao@gmail.com> diff --git a/MAINTAINERS b/MAINTAINERS index 66f5f7d..33978ed 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4439,6 +4439,13 @@ L: linux-arm-kernel at lists.infradead.org (moderated for non-subscribers) T: git git://git.kernel.org/pub/scm/linux/kernel/git/ycmiao/pxa-linux-2.6.git S: Maintained +MMP2 SUPPORT (aka ARMADA610) +M: Haojian Zhuang <haojian.zhuang@marvell.com> +M: Eric Miao <eric.y.miao@gmail.com> +L: linux-arm-kernel at lists.infradead.org (moderated for non-subscribers) +T: git git://git.kernel.org/pub/scm/linux/kernel/git/ycmiao/pxa-linux-2.6.git +S: Maintained + PXA MMCI DRIVER S: Orphan ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-05 7:30 ` Eric Miao @ 2010-01-05 9:13 ` Haojian Zhuang 2010-01-05 9:34 ` Eric Miao 0 siblings, 1 reply; 15+ messages in thread From: Haojian Zhuang @ 2010-01-05 9:13 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 5, 2010 at 2:30 AM, Eric Miao <eric.y.miao@gmail.com> wrote: >> OK. Update the patches. >> > > Applied to branch 'mmp2' and made some changes as follows > (please let me know your concerns and help test this): > > 1. irq-mmp2.c: separate the difference into different irq_chip > and individual demux handler for those MUXed IRQs, naming > is changed a bit as well. Having a single irq_chip and demux > handler isn't performance friendly - having to go through a lengthy > condition branches each time - and that's why irq_chip is invented. > > 2. merge patch 1, 2, 3 together so mmp2 can build (my build > here complains about no machine_desc defined if flint support > isn't there) > > And I'd like you to help co-maintaine MMP2 together with me, > please kindly Ack if you're fine with the patch follows: > > commit 2004b15682f0f612b85088d730164e91fca2eec8 > Author: Eric Miao <eric.y.miao@gmail.com> > Date: ? Tue Jan 5 15:28:26 2010 +0800 > > ? ?MAINTAINERS: add maintainers for Marvell MMP2 (aka ARMADA610) support > > ? ?Cc: Haojian Zhuang <haojian.zhuang@marvell.com> > ? ?Signed-off-by: Eric Miao <eric.y.miao@gmail.com> > > diff --git a/MAINTAINERS b/MAINTAINERS > index 66f5f7d..33978ed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4439,6 +4439,13 @@ L: ? ? ? linux-arm-kernel at lists.infradead.org > (moderated for non-subscribers) > ?T: ? ? git git://git.kernel.org/pub/scm/linux/kernel/git/ycmiao/pxa-linux-2.6.git > ?S: ? ? Maintained > > +MMP2 SUPPORT (aka ARMADA610) > +M: ? ? Haojian Zhuang <haojian.zhuang@marvell.com> > +M: ? ? Eric Miao <eric.y.miao@gmail.com> > +L: ? ? linux-arm-kernel at lists.infradead.org (moderated for non-subscribers) > +T: ? ? git git://git.kernel.org/pub/scm/linux/kernel/git/ycmiao/pxa-linux-2.6.git > +S: ? ? Maintained > + > ?PXA MMCI DRIVER > ?S: ? ? Orphan > Acked. Best Regards Haojian ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-05 9:13 ` Haojian Zhuang @ 2010-01-05 9:34 ` Eric Miao 0 siblings, 0 replies; 15+ messages in thread From: Eric Miao @ 2010-01-05 9:34 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 5, 2010 at 5:13 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote: > On Tue, Jan 5, 2010 at 2:30 AM, Eric Miao <eric.y.miao@gmail.com> wrote: >>> OK. Update the patches. >>> >> >> Applied to branch 'mmp2' and made some changes as follows >> (please let me know your concerns and help test this): >> >> 1. irq-mmp2.c: separate the difference into different irq_chip >> and individual demux handler for those MUXed IRQs, naming >> is changed a bit as well. Having a single irq_chip and demux >> handler isn't performance friendly - having to go through a lengthy >> condition branches each time - and that's why irq_chip is invented. >> >> 2. merge patch 1, 2, 3 together so mmp2 can build (my build >> here complains about no machine_desc defined if flint support >> isn't there) >> >> And I'd like you to help co-maintaine MMP2 together with me, >> please kindly Ack if you're fine with the patch follows: >> >> commit 2004b15682f0f612b85088d730164e91fca2eec8 >> Author: Eric Miao <eric.y.miao@gmail.com> >> Date: ? Tue Jan 5 15:28:26 2010 +0800 >> >> ? ?MAINTAINERS: add maintainers for Marvell MMP2 (aka ARMADA610) support >> >> ? ?Cc: Haojian Zhuang <haojian.zhuang@marvell.com> >> ? ?Signed-off-by: Eric Miao <eric.y.miao@gmail.com> >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 66f5f7d..33978ed 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -4439,6 +4439,13 @@ L: ? ? ? linux-arm-kernel at lists.infradead.org >> (moderated for non-subscribers) >> ?T: ? ? git git://git.kernel.org/pub/scm/linux/kernel/git/ycmiao/pxa-linux-2.6.git >> ?S: ? ? Maintained >> >> +MMP2 SUPPORT (aka ARMADA610) >> +M: ? ? Haojian Zhuang <haojian.zhuang@marvell.com> >> +M: ? ? Eric Miao <eric.y.miao@gmail.com> >> +L: ? ? linux-arm-kernel at lists.infradead.org (moderated for non-subscribers) >> +T: ? ? git git://git.kernel.org/pub/scm/linux/kernel/git/ycmiao/pxa-linux-2.6.git >> +S: ? ? Maintained >> + >> ?PXA MMCI DRIVER >> ?S: ? ? Orphan >> > > Acked. > OK, thanks ^ permalink raw reply [flat|nested] 15+ messages in thread
* [patch 2/4] [ARM] mmp: support marvell ARMADA610 2010-01-04 2:49 ` Eric Miao 2010-01-04 3:29 ` Haojian Zhuang @ 2010-01-04 10:30 ` Russell King - ARM Linux 1 sibling, 0 replies; 15+ messages in thread From: Russell King - ARM Linux @ 2010-01-04 10:30 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 04, 2010 at 10:49:05AM +0800, Eric Miao wrote: > On Mon, Jan 4, 2010 at 10:27 AM, Haojian Zhuang > <haojian.zhuang@gmail.com> wrote: > > get_irqnr_preamble can only carries one base parameter and one tmp > > parameter. base parameter is already used for interrupt control > > register. Only one tmp parameter is left for use. If we want to check > > chip id, one parameter is not enought. > > I'm a bit confused, but those are just registers that the macro is able to use, > not something as parameters?? Correct - they're for sanity with the rest of the code. They're a way of saying "this macro can use these registers", and the rest of the code which calls the macros really doesn't care what you do with them in the macro. Imagine what the situation would be if we didn't do this, and everyone used their own sets of registers in these macros - it'd quickly become impossible to safely modify the surrounding code, since we wouldn't know what registers the macro used. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2010-01-05 9:34 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-07 6:19 [patch 2/4] [ARM] mmp: support marvell ARMADA610 Haojian Zhuang 2009-12-29 3:49 ` Eric Miao 2009-12-31 14:42 ` Haojian Zhuang 2010-01-01 3:46 ` Eric Miao 2010-01-04 2:27 ` Haojian Zhuang 2010-01-04 2:49 ` Eric Miao 2010-01-04 3:29 ` Haojian Zhuang 2010-01-04 4:27 ` Eric Miao 2010-01-04 10:15 ` Haojian Zhuang 2010-01-04 10:27 ` Russell King - ARM Linux 2010-01-04 11:16 ` Haojian Zhuang 2010-01-05 7:30 ` Eric Miao 2010-01-05 9:13 ` Haojian Zhuang 2010-01-05 9:34 ` Eric Miao 2010-01-04 10:30 ` Russell King - ARM Linux
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).