Re: Setup-sh7709.c

[Kristoffer E. <kri...@ho...>]

I'll be glad to try it out once I get back to uni on monday. Big thanks at 
cleaning up it abit, had a few braincell losses figuring it out. :)

Best wishes
Kristoffer


>From: tak...@op...
>To: "Kristoffer Ericson" <kri...@ho...>
>CC: lin...@li...
>Subject: Re: Setup-sh7709.c
>Date: Wed, 28 Mar 2007 02:46:25 +0900
>
>Hi, Kristoffer, and lists,
># This is not a patch commit.
>
>There is a issue whether external interrupts should be initialized in CPU 
>setup code, or not.
>I initialize them in board setup code (boards/shmin/setup.c) now.
>I don't know the answer, perhaps CPU do definition, and board should do all 
>of initialization, or
>ipr initialization and priority setting should be separated, or this is pin 
>mux issue, or ... I don't know.
>So, forget it now, please :) Here is another topic.
>
> > +	[33]            = { IPRC(1),     1 }, /* IRQ 1 */
> > ...
>Perhaps, what you want is like a patch at the end.
>Argument of IPR* macro is like "11-8".
>It's not a number, nor a mathematical notation(like 15-8=7), because of 
>"0*N*-1" in IPRx macro
>definition. I'm  sorry to have commited such a dain bramaged macros.
>
>I explain on this occasion.
>This was a kind of objection against the comment in asm-sh/irq.h. It says
>| * The shift value is now the number of bits to shift, not the number of
>| * bits/4. This is to make it easier to read the value directly from the
>| * datasheets. <snip>
>I don't think (12,8,4,0) is easier than (3,2,1,0). And what you read direct 
>from the
>datasheet is (15-12,11-8,7-4,3-0). Hence the weired math.
>But, this is ugry, sorry. I will fix the macro to (12,8,4,0) style. It 
>seems this is the majority.
>
>More important is later part.
>| <snip> The IPR address, addr, will be set from ipr_idx via the
>| * map_ipridx_to_addr function.
>I don't agree this method. Symbol reference back from core part is not 
>good.
>I don't understand what this is intended to.  If it is to abstract 
>hardware, it's not enough.
>
>IRQx macro provids the way to setup IPRs without map_ipridx_to_addr().
>I think .address  and .shift is a set, and is the hardware description.
>The macro packs them, at least for the eyes. The real one should come 
>later, perhaps when
>we introduce IRQ number abstraction.
>That's why I end whth this implementation.
>
>So, could you please test this patch on your target ? > Kristoffer.
>Here on my board(shmin/7706), it seems no problem, so far.
>
>Best regards,
>/yoshii
>
>diff --git a/arch/sh/kernel/cpu/sh3/setup-sh7709.c 
>b/arch/sh/kernel/cpu/sh3/setup-sh7709.c
>index dc9b211..47c7cf6 100644
>--- a/arch/sh/kernel/cpu/sh3/setup-sh7709.c
>+++ b/arch/sh/kernel/cpu/sh3/setup-sh7709.c
>@@ -52,20 +52,30 @@ static int __init sh7709_devices_setup(v
>  }
>  __initcall(sh7709_devices_setup);
>
>-#define IPRx(A,N) .addr=A, .shift=0*N*-1
>+#define IPRx(A,N)	.addr=A, .shift=N
>  #define IPRA(N)	IPRx(0xfffffee2UL,N)
>  #define IPRB(N)	IPRx(0xfffffee4UL,N)
>+#define IPRC(N)	IPRx(0xa4000016UL,N)
>+#define IPRD(N)	IPRx(0xa4000018UL,N)
>  #define IPRE(N)	IPRx(0xa400001aUL,N)
>
>  static struct ipr_data sh7709_ipr_map[] = {
>-	[16]		= { IPRA(15-12), 2 }, /* TMU TUNI0 */
>-	[17]		= { IPRA(11-8),  4 }, /* TMU TUNI1 */
>-	[22]		= { IPRA(3-0),   2 }, /* RTC CUI */
>-	[23 ... 26]	= { IPRB(7-4),   3 }, /* SCI */
>-	[27]		= { IPRB(15-12), 2 }, /* WDT ITI */
>-	[48 ... 51]	= { IPRE(15-12), 7 }, /* DMA */
>-	[52 ... 55]	= { IPRE(11-8),  3 }, /* IRDA */
>-	[56 ... 59]	= { IPRE(7-4),   3 }, /* SCIF */
>+	[16]		= { IPRA(12), 2 }, /* TMU TUNI0 */
>+	[17]		= { IPRA( 8), 4 }, /* TMU TUNI1 */
>+	[18 ... 19]	= { IPRA( 4), 1 }, /* TMU2 */
>+	[20 ... 22]	= { IPRA( 0), 2 }, /* RTC */
>+	[23 ... 26]	= { IPRB( 4), 3 }, /* SCI */
>+	[27]		= { IPRB(12), 2 }, /* WDT ITI */
>+	[28 ... 29]	= { IPRB( 8), 1 }, /* REF */
>+	[32]		= { IPRC( 0), 1 }, /* IRQ0 */
>+	[33]		= { IPRC( 4), 1 }, /* IRQ1 */
>+	[34]		= { IPRC( 8), 1 }, /* IRQ2 */
>+	[35]		= { IPRC(12), 1 }, /* IRQ3 */
>+	[36]		= { IPRD( 0), 1 }, /* IRQ4 */
>+	[37]		= { IPRD( 4), 1 }, /* IRQ5 */
>+	[48 ... 51]	= { IPRE(12), 7 }, /* DMA */
>+	[52 ... 55]	= { IPRE( 8), 3 }, /* IRDA */
>+	[56 ... 59]	= { IPRE( 4), 3 }, /* SCIF */
>  };
>
>  void __init init_IRQ_ipr()

_________________________________________________________________
FREE pop-up blocking with the new MSN Toolbar - get it now! 
http://toolbar.msn.click-url.com/go/onm00200415ave/direct/01/