Re: CVS commit: src/sys/arch/evbarm/iq80310

[Ryo Shimizu <ryo%nerv.org@localhost>]

>> > On Mon, Sep 17, 2012 at 03:40:12AM +0900, Izumi Tsutsui wrote:
>> > > chs@ wrote:
>> > > 
>> > > > Module Name:   src
>> > > > Committed By:  chs
>> > > > Date:          Tue Aug 14 15:46:21 UTC 2012
>> > > > 
>> > > > Modified Files:
>> > > >        src/sys/arch/evbarm/iq80310: iq80310_intr.c
>> > > > 
>> > > > Log Message:
>> > > > move evcnt_attach_dynamic() calls later to avoid assertions.
>> > > 
>> > > 
>> > > - what happens if intr_establish function is called more than once
>> > >   against the same irq?
>> > > 
>> > > - shouldn't disestablish function have evcnt_detach()?
>> > > 
>> > > - isn't it better to defer intr_init() calls after evcnt_init(),
>> > >   rather than calling evcnt_attach_dynamic() in intr_establish() ?
>> > > 
>> > > 
>> > > It looks many other arm intr_init functions
>> > > (arm/ep93xx/ep93xx_intr.c, arm/xscale/ixp425_intr.c etc)
>> > > have the same problem, though.
>> > > (on the other hand marvell variants don't have evcnt at all)
>>  :
>> > are you suggesting to undo my change and instead move the call to
>> > iq80310_intr_init() from initarm() to somewhere else?  if so, where?
>> > 
>> > or are you suggesting to move the calls to evcnt_attach_dynamic()
>> > from iq80310_intr_init() to a separate function which could be called
>> > later?  if so, where would be a good place to call that other function?
>> 
>> The former is okay because interrupts won't be enabled before
>> at least cpu_configure(9) and I moved such initializations
>> into cpu_configure(9) on several mips and m68k ports.
>> 
>> But on evbarm cpu_configure(9) is shared among all boards
>> so we need some hook to handle it? Or call it mainbus_attach()?
>> Or just disable intr evcnt(9) for now?
>
>Now I notice that matt@ already fixed some boards using the latter one
>(split out evcnt_attach_dynamic() int a separate function and call it
> from board dependent mainbus_attach) so following it would be consistent.
>
>http://mail-index.NetBSD.org/source-changes/2012/02/12/msg031698.html
>http://www.nerv.org/~ryo/netbsd/netbsd/?q=id:20120212T163102Z.cef48d9fbe70f5c792f90fc3a83a35e9ec1d6880
>http://cvsweb.NetBSD.org/bsdweb.cgi/src/sys/arch/arm/xscale/i80321_icu.c.diff?r1=1.22&r2=1.23&f=h
>http://cvsweb.NetBSD.org/bsdweb.cgi/src/sys/arch/evbarm/hdl_g/i80321_mainbus.c.diff?r1=1.2&r2=1.3&f=h


I fixed evbarm/{ixdp425,nslu2} in the same way as matt@ did.
ok to commit?

--
ryo shimizu


Index: sys/arch/arm/xscale/ixp425_intr.c
===================================================================
RCS file: /cvsroot/src/sys/arch/arm/xscale/ixp425_intr.c,v
retrieving revision 1.23
diff -U 4 -r1.23 ixp425_intr.c
--- sys/arch/arm/xscale/ixp425_intr.c   1 Jul 2011 20:32:51 -0000       1.23
+++ sys/arch/arm/xscale/ixp425_intr.c   26 Sep 2012 11:19:58 -0000
@@ -307,18 +307,29 @@
                iq = &intrq[i];
                TAILQ_INIT(&iq->iq_list);
 
                sprintf(iq->iq_name, "irq %d", i);
-               evcnt_attach_dynamic(&iq->iq_ev, EVCNT_TYPE_INTR,
-                                    NULL, "ixp425", iq->iq_name);
        }
 
        ixp425_intr_calculate_masks();
 
        /* Enable IRQs (don't yet use FIQs). */
        enable_interrupts(I32_bit);
 }
 
+void
+ixp425_intr_evcnt_attach(void)
+{
+       struct intrq *iq;
+       int i;
+
+       for (i = 0; i < NIRQ; i++) {
+               iq = &intrq[i];
+               evcnt_attach_dynamic(&iq->iq_ev, EVCNT_TYPE_INTR,
+                   NULL, "ixp425", iq->iq_name);
+       }
+}
+
 void *
 ixp425_intr_establish(int irq, int ipl, int (*func)(void *), void *arg)
 {
        struct intrq *iq;
Index: sys/arch/arm/xscale/ixp425var.h
===================================================================
RCS file: /cvsroot/src/sys/arch/arm/xscale/ixp425var.h,v
retrieving revision 1.13
diff -U 4 -r1.13 ixp425var.h
--- sys/arch/arm/xscale/ixp425var.h     1 Jul 2011 20:32:51 -0000       1.13
+++ sys/arch/arm/xscale/ixp425var.h     26 Sep 2012 11:19:58 -0000
@@ -145,8 +145,9 @@
 void   ixp425_attach(struct ixp425_softc *);
 void   ixp425_icu_init(void);
 void   ixp425_clk_bootstrap(bus_space_tag_t);
 void   ixp425_intr_init(void);
+void   ixp425_intr_evcnt_attach(void);
 void   *ixp425_intr_establish(int, int, int (*)(void *), void *);
 void    ixp425_intr_disestablish(void *);
 
 uint32_t ixp425_sdram_size(void);
Index: sys/arch/evbarm/nslu2/nslu2_mainbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/evbarm/nslu2/nslu2_mainbus.c,v
retrieving revision 1.2
diff -U 4 -r1.2 nslu2_mainbus.c
--- sys/arch/evbarm/nslu2/nslu2_mainbus.c       28 Apr 2008 20:23:17 -0000      
1.2
+++ sys/arch/evbarm/nslu2/nslu2_mainbus.c       26 Sep 2012 11:19:58 -0000
@@ -51,8 +51,9 @@
 static void
 ixp425_mainbus_attach(struct device *parent, struct device *self, void *arg)
 {
 
+       ixp425_intr_evcnt_attach();
        ixp425_attach((struct ixp425_softc *) self);
 }
 
 CFATTACH_DECL(ixpio_mainbus, sizeof(struct ixp425_softc),
Index: sys/arch/evbarm/ixdp425/ixdp425_mainbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/evbarm/ixdp425/ixdp425_mainbus.c,v
retrieving revision 1.7
diff -U 4 -r1.7 ixdp425_mainbus.c
--- sys/arch/evbarm/ixdp425/ixdp425_mainbus.c   1 Jul 2011 20:42:36 -0000       
1.7
+++ sys/arch/evbarm/ixdp425/ixdp425_mainbus.c   26 Sep 2012 11:19:58 -0000
@@ -64,6 +64,7 @@
 ixp425_mainbus_attach(struct device *parent, struct device *self, void *aux)
 {
        struct ixp425_softc *sc = (void *) self;
 
+       ixp425_intr_evcnt_attach();
        ixp425_attach(sc);
 }