From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Larmour To: Daniel Lind Cc: eCos Disuss Subject: Re: [ECOS] SMC serial driver, code in eCos wrong, PART II ? Made some changes and now it works fine..... Date: Tue, 05 Dec 2000 19:17:00 -0000 Message-id: <3A2DAFE0.C9BFD9EC@cygnus.co.uk> References: <3A096EEE.25FA8B1F@bluelabs.se> X-SW-Source: 2000-12/msg00062.html [ 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 + + * 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 * 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)