Re: Setup-sh7709.c

[<tak...@op...>]

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