public inbox for ecos-discuss@sourceware.org
 help / color / mirror / Atom feed
* [ECOS] SMC serial driver, code in eCos wrong, PART II ? Made some changes and now it works fine.....
@ 2000-11-08  7:17 Daniel Lind
  2000-12-05 19:17 ` Jonathan Larmour
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Lind @ 2000-11-08  7:17 UTC (permalink / raw)
  To: eCos Disuss

Hello,
I'm glad if someone who works with eCos takes a look at the changes I
have done in quicc_smc_serial.c. I'm using a MBX860 PowerPC. I wrote a
testprogram and found out a rather funny thing. If i first used
cyg_io_write( ) and after this tried to receive with the function
cyg_io_read( ) everything just worked fine. But after this I tried first
to send out with the function cyg_io_read( ) and after this to send out
with  cyg_io_write( ). NOW NOTHING WORKED!!!

The reason why this later alternative don't work is that in the
initialization of the interrupts of the SMC2 are masked. Shouldn't it be
unmasked?
However, if it's right that the SMC2 interrupt is going to be masked
after initialization I can't see how the receiving of  bytes is going to
work! The functions that take care of receiving characters works
together with the interrupt routine quicc_smc_serial_DSR(cyg_vector_t
vector, cyg_ucount32 count, cyg_addrword_t data). If the interrupts are
not unmasked you will never come to the interrupt routine and thus never
take the characters out of the receive-buffers. If you dont believe me
test by your self.

Ok, so now I have changed the initialization and can run cyg_io_read( )
without first using cyg_io_write( ). But I made one more change. In the
function quicc_smc_serial_start_xmit(serial_channel *chan). In the
beginning of this function I mask the interrupts. I don't know if this
is necessary but my thought was that I don't want anything to play
around with the txbd-buffers when the interrupts are unmasked.

After a lot of hours of debugging and testing I have made the following
changes to the eCos code.

In file quicc_smc_serial.c

static bool
quicc_smc_serial_init(struct cyg_devtab_entry *tab)
{
.
 cyg_drv_interrupt_create(smc_chan->int_num,
                                 CYGARC_SIU_PRIORITY_HIGH, // Priority -
unused (but asserted)
                                 (cyg_addrword_t)chan,   //  Data item
passed to interrupt handler
                                 quicc_smc_serial_ISR,
                                 quicc_smc_serial_DSR,
                                 &smc_chan->serial_interrupt_handle,
                                 &smc_chan->serial_interrupt);
        cyg_drv_interrupt_attach(smc_chan->serial_interrupt_handle);
  #ifdef CYGDBG_IO_INIT
   diag_printf("QUICC_SMC_2 init - interrupt number %d , calls
cyg_drv_interrupt_mask( ) \n", smc_chan->int_num);
   //diag_printf("QUICC_SMC_2 init, calls cyg_drv_interrupt_mask( )
\n");
  #endif
        //cyg_drv_interrupt_mask(smc_chan->int_num);
  cyg_drv_interrupt_unmask(smc_chan->int_num); //Change by Daniel Lind
!?!?!??!?!?!?!?!?HERE IS A CHANGE!!!!!!!!!!!!!!!!!!!!!!!!
        smc_chan->tx_enabled = false;
.
}



static void
quicc_smc_serial_start_xmit(serial_channel *chan)
{
    quicc_smc_serial_info *smc_chan = (quicc_smc_serial_info
*)chan->dev_priv;
 cyg_drv_interrupt_mask(smc_chan->int_num); //Change by Daniel Lind
!?!??!?!?!?!?!??!?!?!?!?!HERE IS A CHANGE!?!?!?!?!?!?!?!?!?!?
    if (smc_chan->txbd->length == 0) {
        // See if there is anything to put in this buffer, just to get
it going
        cyg_drv_dsr_lock();
        (chan->callbacks->xmt_char)(chan);
        cyg_drv_dsr_unlock();
    }
    if (smc_chan->txbd->length != 0) {
        // Make sure it gets started
        quicc_smc_serial_flush(smc_chan);
    }
    smc_chan->tx_enabled = true;
    cyg_drv_interrupt_unmask(smc_chan->int_num);
}

/Daniel Lind

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [ECOS] SMC serial driver, code in eCos wrong, PART II ? Made some changes and now it works fine.....
  2000-11-08  7:17 [ECOS] SMC serial driver, code in eCos wrong, PART II ? Made some changes and now it works fine Daniel Lind
@ 2000-12-05 19:17 ` Jonathan Larmour
  0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Larmour @ 2000-12-05 19:17 UTC (permalink / raw)
  To: Daniel Lind; +Cc: eCos Disuss

[ Catching up on some old messages no-one ever replied to ]

Daniel Lind wrote:
> 
> The reason why this later alternative don't work is that in the
> initialization of the interrupts of the SMC2 are masked. Shouldn't it be
> unmasked?

I agree. I can't see how it can have behaved well before since when tx'ing
finishes, it is masked again.

> After a lot of hours of debugging and testing I have made the following
> changes to the eCos code.
> 
> In file quicc_smc_serial.c
> 
> static bool
> quicc_smc_serial_init(struct cyg_devtab_entry *tab)
> {
> .
>  cyg_drv_interrupt_create(smc_chan->int_num,
>                                  CYGARC_SIU_PRIORITY_HIGH, // Priority -
> unused (but asserted)
>                                  (cyg_addrword_t)chan,   //  Data item
> passed to interrupt handler
>                                  quicc_smc_serial_ISR,
>                                  quicc_smc_serial_DSR,
>                                  &smc_chan->serial_interrupt_handle,
>                                  &smc_chan->serial_interrupt);
>         cyg_drv_interrupt_attach(smc_chan->serial_interrupt_handle);
>   #ifdef CYGDBG_IO_INIT
>    diag_printf("QUICC_SMC_2 init - interrupt number %d , calls
> cyg_drv_interrupt_mask( ) \n", smc_chan->int_num);
>    //diag_printf("QUICC_SMC_2 init, calls cyg_drv_interrupt_mask( )
> \n");
>   #endif
>         //cyg_drv_interrupt_mask(smc_chan->int_num);
>   cyg_drv_interrupt_unmask(smc_chan->int_num); //Change by Daniel Lind
> !?!?!??!?!?!?!?!?HERE IS A CHANGE!!!!!!!!!!!!!!!!!!!!!!!!
>         smc_chan->tx_enabled = false;
> .
> }
> 
> static void
> quicc_smc_serial_start_xmit(serial_channel *chan)
> {
>     quicc_smc_serial_info *smc_chan = (quicc_smc_serial_info
> *)chan->dev_priv;
>  cyg_drv_interrupt_mask(smc_chan->int_num); //Change by Daniel Lind
> !?!??!?!?!?!?!??!?!?!?!?!HERE IS A CHANGE!?!?!?!?!?!?!?!?!?!?
>     if (smc_chan->txbd->length == 0) {
>         // See if there is anything to put in this buffer, just to get
> it going
>         cyg_drv_dsr_lock();
>         (chan->callbacks->xmt_char)(chan);
>         cyg_drv_dsr_unlock();
>     }
>     if (smc_chan->txbd->length != 0) {
>         // Make sure it gets started
>         quicc_smc_serial_flush(smc_chan);
>     }
>     smc_chan->tx_enabled = true;
>     cyg_drv_interrupt_unmask(smc_chan->int_num);
> }

I agree in principle with your changes, but I've applied a slightly
different patch (which I've attached). Please let us know how you get on
with it as I haven't tested it!

Jifl
-- 
Red Hat, 35 Cambridge Place, Cambridge, UK. CB2 1NS  Tel: +44 (1223) 728762
"Plan to be spontaneous tomorrow."  ||  These opinions are all my own fault
Index: ChangeLog
===================================================================
RCS file: /home/cvs/ecc/ecc/devs/serial/powerpc/quicc/current/ChangeLog,v
retrieving revision 1.7
diff -u -5 -p -r1.7 ChangeLog
--- ChangeLog	2000/10/24 13:46:11	1.7
+++ ChangeLog	2000/12/06 03:15:34
@@ -1,5 +1,12 @@
+2000-12-06  Jonathan Larmour  <jlarmour@redhat.com>
+
+	* src/quicc_smc_serial.c: Remove unread tx_enabled variable from
+	quicc_smc_serial_info
+	Ensure quicc serial interrupt is unmasked in general so that rx works!
+	(quicc_smc_serial_start_xmit): Protect better from DSR interruption
+
 2000-10-24  Jonathan Larmour  <jlarmour@redhat.com>
 
 	* src/quicc_smc_serial.c (quicc_smc_serial_ISR): Return with
 	CYG_ISR_HANDLED (reported by Daniel Lind)
 
Index: src/quicc_smc_serial.c
===================================================================
RCS file: /home/cvs/ecc/ecc/devs/serial/powerpc/quicc/current/src/quicc_smc_serial.c,v
retrieving revision 1.3
diff -u -5 -p -r1.3 quicc_smc_serial.c
--- src/quicc_smc_serial.c	2000/10/24 13:46:12	1.3
+++ src/quicc_smc_serial.c	2000/12/06 03:15:34
@@ -95,11 +95,10 @@ typedef struct quicc_smc_serial_info {
     volatile struct cp_bufdesc     *txbd, *rxbd;     // Next Tx,Rx descriptor to use
     struct cp_bufdesc     *tbase, *rbase;            // First Tx,Rx descriptor
     int                   txsize, rxsize;            // Length of individual buffers
     cyg_interrupt         serial_interrupt;
     cyg_handle_t          serial_interrupt_handle;
-    bool                  tx_enabled;
 } quicc_smc_serial_info;
 
 static bool quicc_smc_serial_init(struct cyg_devtab_entry *tab);
 static bool quicc_smc_serial_putc(serial_channel *chan, unsigned char c);
 static Cyg_ErrNo quicc_smc_serial_lookup(struct cyg_devtab_entry **tab, 
@@ -450,12 +449,11 @@ quicc_smc_serial_init(struct cyg_devtab_
                                  quicc_smc_serial_ISR,
                                  quicc_smc_serial_DSR,
                                  &smc_chan->serial_interrupt_handle,
                                  &smc_chan->serial_interrupt);
         cyg_drv_interrupt_attach(smc_chan->serial_interrupt_handle);
-        cyg_drv_interrupt_mask(smc_chan->int_num);
-        smc_chan->tx_enabled = false;
+        cyg_drv_interrupt_unmask(smc_chan->int_num);
     }
     quicc_smc_serial_config_port(chan, &chan->config, true);
     if (cache_state)
         HAL_DCACHE_ENABLE();
     return true;
@@ -575,37 +573,31 @@ quicc_smc_serial_set_config(serial_chann
 // Enable the transmitter (interrupt) on the device
 static void
 quicc_smc_serial_start_xmit(serial_channel *chan)
 {
     quicc_smc_serial_info *smc_chan = (quicc_smc_serial_info *)chan->dev_priv;
+    cyg_drv_dsr_lock();
     if (smc_chan->txbd->length == 0) {
         // See if there is anything to put in this buffer, just to get it going
-        cyg_drv_dsr_lock();
         (chan->callbacks->xmt_char)(chan);
-        cyg_drv_dsr_unlock();
     }
     if (smc_chan->txbd->length != 0) {
         // Make sure it gets started
         quicc_smc_serial_flush(smc_chan);
     }
-    smc_chan->tx_enabled = true;
-    cyg_drv_interrupt_unmask(smc_chan->int_num);
+    cyg_drv_dsr_unlock();
 }
 
 // Disable the transmitter on the device
 static void 
 quicc_smc_serial_stop_xmit(serial_channel *chan)
 {
     quicc_smc_serial_info *smc_chan = (quicc_smc_serial_info *)chan->dev_priv;
     // If anything is in the last buffer, need to get it started
     if (smc_chan->txbd->length != 0) {
         quicc_smc_serial_flush(smc_chan);
-        // Note: interrupt will get masked after this buffer finishes
-    } else {
-        cyg_drv_interrupt_mask(smc_chan->int_num);
     }
-    smc_chan->tx_enabled = false;
 }
 
 // Serial I/O - low level interrupt handler (ISR)
 static cyg_uint32 
 quicc_smc_serial_ISR(cyg_vector_t vector, cyg_addrword_t data)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2000-12-05 19:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2000-11-08  7:17 [ECOS] SMC serial driver, code in eCos wrong, PART II ? Made some changes and now it works fine Daniel Lind
2000-12-05 19:17 ` Jonathan Larmour

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 read-only IMAP folder(s) and NNTP newsgroup(s).