[Fwd: Re: [APPROVE?] Fix spurious interrupt on mpc8xx]

[Fwd: Re: [APPROVE?] Fix spurious interrupt on mpc8xx]

[Jonathan Larmour]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

a) A brief name by which we can all refer to the patch.

quicc.pat

b) A description of what the patch achieves or the problem which it
addresses. No more than a few sentences.

As discussed just now on ecos-patches as per attached, the attached patch 
fixes spurious interrupts with serial on all quicc based targets when 
debugging over serial via GDB.

c) A rationale for including the patch in the 2.0 final release.

quicc-based targets will otherwise not be able to debug over serial when 
asserts are enabled. It is limited to quiccs.

Jifl
-- 
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine

[-- Attachment #2: quicc.pat --]
[-- Type: text/plain, Size: 3500 bytes --]

Index: src/cpm.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/powerpc/quicc/current/src/cpm.c,v
retrieving revision 1.3.2.1
diff -u -5 -p -r1.3.2.1 cpm.c
--- src/cpm.c	20 Mar 2003 18:49:10 -0000	1.3.2.1
+++ src/cpm.c	31 Mar 2003 15:33:01 -0000
@@ -55,10 +55,11 @@
 
 #include <pkgconf/hal.h>
 #include <pkgconf/hal_powerpc_quicc.h>
 #include <cyg/infra/cyg_type.h>
 #include <cyg/hal/hal_arch.h>
+#include <string.h>           // memset
 
 #ifdef CYGPKG_HAL_POWERPC_MPC860
 // eCos headers decribing PowerQUICC:
 #include <cyg/hal/quicc/ppc8xx.h>
 
@@ -73,19 +74,19 @@ static short *nextBd = (short *)(CYGHWR_
 
 void
 _mpc8xx_reset_cpm(void)
 {
     EPPC *eppc = eppc_base();
-    int i;
     static int init_done = 0;
 
     if (init_done) return;
     init_done++;
 
     eppc->cp_cr = QUICC_CPM_CR_RESET | QUICC_CPM_CR_BUSY;
-    memset(eppc->pram, 0, 0x400);
-    for (i = 0; i < 100000; i++);
+    memset(eppc->pram, 0, sizeof(eppc->pram));
+    while (eppc->cp_cr & QUICC_CPM_CR_BUSY)
+        CYG_EMPTY_STATEMENT;
 
     *nextBd = QUICC_BD_BASE;
 }
 
 //
Index: src/quicc_smc1.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/powerpc/quicc/current/src/quicc_smc1.c,v
retrieving revision 1.23.2.1
diff -u -5 -p -r1.23.2.1 quicc_smc1.c
--- src/quicc_smc1.c	20 Mar 2003 18:49:10 -0000	1.23.2.1
+++ src/quicc_smc1.c	31 Mar 2003 15:33:01 -0000
@@ -71,10 +71,11 @@
 #include <cyg/hal/hal_stub.h>           // target_register_t
 #include <cyg/hal/hal_intr.h>           // HAL_INTERRUPT_UNMASK(...)
 #include <cyg/hal/hal_if.h>             // Calling interface definitions
 #include <cyg/hal/hal_misc.h>           // Helper functions
 #include <cyg/hal/drv_api.h>            // CYG_ISR_HANDLED
+#include <string.h>                     // memset
 
 #define UART_BIT_RATE(n) (((int)(CYGHWR_HAL_POWERPC_BOARD_SPEED*1000000)/16)/n)
 #define UART_BAUD_RATE CYGNUM_HAL_VIRTUAL_VECTOR_CONSOLE_CHANNEL_BAUD
 
 // Note: buffers will be placed just after descriptors
@@ -149,12 +150,10 @@ cyg_hal_smcx_init_channel(struct port_in
     struct cp_bufdesc *txbd, *rxbd;
 
     if (info->init) return;
     info->init = 1;
 
-    _mpc8xx_reset_cpm();
-
     switch (port) {
 #if CYGNUM_HAL_QUICC_SMC1 > 0
     case QUICC_CPM_SMC1:
         /*
          *  Set up the PortB pins for UART operation.
@@ -253,11 +252,11 @@ cyg_hal_smcx_init_channel(struct port_in
     /*
      *  Clear any previous events. Mask interrupts.
      *  (Section 16.15.7.14 and 16.15.7.15)
      */
     regs->smc_smce = 0xff;
-    regs->smc_smcm = 5;
+    regs->smc_smcm = 1; // RX interrupts only, for ctrl-c
 
     /*
      *  Set 8,n,1 characters, then also enable rx and tx.
      *  (Section 16.15.7.11)
      */
@@ -585,12 +584,10 @@ cyg_hal_sccx_init_channel(struct port_in
     struct cp_bufdesc *txbd, *rxbd;
 
     if (info->init) return;
     info->init = 1;
 
-    _mpc8xx_reset_cpm();
-
     /*
      *  Set up the Port pins for UART operation.
      */
     switch (port) {
 #if CYGNUM_HAL_QUICC_SCC1 > 0
@@ -732,12 +729,11 @@ cyg_hal_sccx_init_channel(struct port_in
     /*
      *  Clear any previous events. Mask interrupts.
      *  (Section 16.15.7.14 and 16.15.7.15)
      */
     regs->scc_scce = 0xffff;
-    regs->scc_sccm = 5;
-    regs->scc_sccm = 3;
+    regs->scc_sccm = 1; // RX interrupts only, for ctrl-c
 
     /*
      *  Set 8,n,1 characters
      */
     regs->scc_psmr = (3<<12);

[-- Attachment #3: Re: [APPROVE?] Fix spurious interrupt on mpc8xx --]
[-- Type: message/rfc822, Size: 3284 bytes --]

From: Gary Thomas <gary@mlbassoc.com>
To: Jonathan Larmour <jifl@eCosCentric.com>
Cc: eCos Patches List <ecos-patches@sources.redhat.com>
Subject: Re: [APPROVE?] Fix spurious interrupt on mpc8xx
Date: 31 Mar 2003 08:03:09 -0700
Message-ID: <1049122989.16212.661.camel@hermes.chez-thomas.org>

On Mon, 2003-03-31 at 07:49, Jonathan Larmour wrote:
> Gary Thomas wrote:
> > On Sun, 2003-03-30 at 23:14, Jonathan Larmour wrote:
> > 
> >>The attached patch fixes a spurious interrupt with asserts enabled when 
> >>using the serial port on viper. It also does some very minor cleanups.
> >>
> >>I haven't checked it in to the trunk, just in case there's some other 
> >>reasoning for the way things was that I've missed (although I doubt it), 
> >>but if it's okay, I would also like approval for the 2.0 branch.
> >>
> > 
> > 
> > I think that these are fine, although the removal of the call to 
> > CPM_reset is a no-op (it should be done by the variant code and
> > then only once, so it has no real effect except for documentation).
> 
> But if someone does CLAIM_COMMS in a RAM loaded app, these channels will 
> then be included, and it will run again, and have side effects on things 
> other than the channels you're using.
> 
> > BTW I've yet to see the spurious interrupt that you are chasing when 
> > using a newer RedBoot (probably even one built from the current branch).
> 
> Yep, was still doing it from the trunk as well as branch as at the end of 
> last week. From what was needed to fix, AFAIK you should just need asserts 
> enabled and to load and run over serial explicitly. The serial TX of 
> talking to GDB is what makes it generate the TX interrupt, which the isr 
> in the driver will (rightly) return 0 for.
> 
> Okay for 2.0 branch explicitly?

Actually, I'm fine with those changes on the trunk as well.  I was 
just bantering about them :-)

-- 
------------------------------------------------------------
Gary Thomas                 |
MLB Associates              |  Consulting for the
+1 (970) 229-1963           |    Embedded world
http://www.mlbassoc.com/    |
email: <gary@mlbassoc.com>  |
gpg: http://www.chez-thomas.org/gary/gpg_key.asc
------------------------------------------------------------

Re: [Fwd: Re: [APPROVE?] Fix spurious interrupt on mpc8xx]

[Jonathan Larmour]

Jonathan Larmour wrote:
> a) A brief name by which we can all refer to the patch.
> 
> quicc.pat

Applied after no objections.

Jifl
-- 
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine