cpm_uart: Made non-console uart work
PATCH 1/1 cpm_uart: Made non-console uart work
Kumar, Pantelis,
This patch makes non-console UART work on both 8xx and 82xx. Various issues are resolved:
- removed unnecessary STOP_TX commands in shutdown - no need to completely stop CPM TX and reinit BDs each time the port is closing;
- when mem_addr has been allocated via dma_coherent_alloc, virt_to_bus on it will not tell the truth in most cases
- SCC UART needs to wait several character times even after all the BDs have READY bit cleared
- adds needed board-specific bits for 86xADS
Tested on 8272ADS, 885ADS and 866ADS development boards.
---------------------------------
Signature · 1 line
Sincerely, Vitaly
drivers/serial/cpm_uart/cpm_uart.h +4 -0
@@ -40,6 +40,8 @@#define TX_NUM_FIFO 4#define TX_BUF_SIZE 32 +#define SCC_WAIT_CLOSING 100 +struct uart_cpm_port {struct uart_port port;u16 rx_nrfifos;
@@ -67,6 +69,8 @@ struct uart_cpm_port {int bits;/* Keep track of 'odd' SMC2 wirings */ int is_portb; +/* wait on close if needed */ +int wait_closing; };extern int cpm_uart_port_map[UART_NR ];
drivers/serial/cpm_uart/cpm_uart_core.c +78 -21
264 lines not on this page · raw message
drivers/serial/cpm_uart/cpm_uart_cpm1.c +23 -23
70 lines not on this page · raw message
I want to test this out on the 8560 ADS to see if works there as well before pushing upstream.
- kumar
Quoted 24 lines · Vitaly Bordug
Kumar, Pantelis,
This patch makes non-console UART work on both 8xx and 82xx. Various issues are resolved:
- removed unnecessarySTOP_TXcommands in shutdown - no need to completely stop CPM TX and reinit BDs each time the port is closing;
- whenmem_addrhas been allocated viadma_coherent_alloc,virt_to_buson it will not tell the truth in most cases
- SCC UART needs to wait several character times even after all the BDs
have READY bit cleared
- adds needed board-specific bits for 86xADSTested on 8272ADS, 885ADS and 866ADS development boards.
---------------------------------Signed-off-by Vitaly Bordug <[email protected]>Signature · 5 lines
Sincerely, Vitaly <cpm_uart_fixes.patch>
Quoted 12 lines · Vitaly Bordug
Kumar, Pantelis,
This patch makes non-console UART work on both 8xx and 82xx. Various issues are resolved:
- removed unnecessarySTOP_TXcommands in shutdown - no need to completely stop CPM TX and reinit BDs each time the port is closing;
- whenmem_addrhas been allocated viadma_coherent_alloc,virt_to_buson it will not tell the truth in most cases
- SCC UART needs to wait several character times even after all the BDs have READY bit cleared
- adds needed board-specific bits for 86xADS
Quoted 3 lines
Tested on 8272ADS, 885ADS and 866ADS development boards.
---------------------------------Signed-off-by Vitaly Bordug <[email protected]>
Vitaly, this patch is clearly in conflict with mine.
Mind taking a look at my patch and include the changes I've made?
You'll only have to change the cpm2cpu & cpu2cpm functions...
I would also argue that the STOP_TX command is needed at shutdown, since this is the canonical way serial ports operate in linux. Also you might want to change the protocol running on an SCC port after shutdown.
The rest are fine...
Regards
Pantelis
Quoted 2 lines · Vitaly Bordug
Kumar, Pantelis,
[snip]
Some more comments.
Review
Quoted 1 line
(quoted diff) +0
-0
b/drivers/serial/cpm_uart/cpm_uart.h
Review
Quoted 21 lines
drivers/serial/cpm_uart/cpm_uart.h +4
-0@@ -40,6 +40,8 @@#define TX_NUM_FIFO 4#define TX_BUF_SIZE 32 +#define SCC_WAIT_CLOSING 100 +struct uart_cpm_port {struct uart_port port;u16 rx_nrfifos;@@ -67,6 +69,8 @@ struct uart_cpm_port {int bits;/* Keep track of 'odd' SMC2 wirings */ int is_portb; +/* wait on close if needed */ +int wait_closing; };extern int cpm_uart_port_map[UART_NR ];(quoted diff) +0
-0
b/drivers/serial/cpm_uart/cpm_uart_core.c
drivers/serial/cpm_uart/cpm_uart_core.c line 15
Quoted 7 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-0@@ -12,6 +12,7 @@* * Copyright (C) 2004 Freescale Semiconductor, Inc. * (C) 2004 Intracom, S.A. +* (C) 2005 MontaVista Software, Inc. by Vitaly Bordug
Quoted 6 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0* * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -143,10 +144,13 @@ static void cpm_uart_start_tx(struct uar }
drivers/serial/cpm_uart/cpm_uart_core.c static void cpm_uart_start_tx(struct uar · line 153
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c +5
-2if (cpm_uart_tx_pump (port) !=0 ) { -if (IS_SMC (pinfo)) +if (IS_SMC (pinfo)) { smcp->smc_smcm |=SMCM_TX ; -else + smcp->smc_smcmr |=SMCMR_TEN ; + }else { sccp->scc_sccm |=UART_SCCM_TX ; + pinfo->sccp->scc_gsmrl |=SCC_GSMRL_ENT ; + }
Why the need to mess with the global SCC transmit enable here? It's dubious IMO.
Quoted 11 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-1} }@@ -265,13 +269,15 @@ static void cpm_uart_int_rx(struct uart_ }/* End while (i--) */ /* This BD is ready to be used again. Clear status. get next */ - bdp->cbd_sc &= ~(BD_SC_BR |BD_SC_FR |BD_SC_PR |BD_SC_OV ); + bdp->cbd_sc &= ~(BD_SC_BR |BD_SC_FR |BD_SC_PR |BD_SC_OV |BD_SC_ID ); bdp->cbd_sc |=BD_SC_EMPTY ;
drivers/serial/cpm_uart/cpm_uart_core.c static void cpm_uart_int_rx(struct uart_ · line 280
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c +6
-4-if (bdp->cbd_sc &BD_SC_WRAP ) - bdp = pinfo->rx_bd_base; -else - bdp++; +if (bdp->cbd_datlen) { +if (bdp->cbd_sc &BD_SC_WRAP ) + bdp = pinfo->rx_bd_base; +else + bdp++; + }
Why is that? Where ever we queue a buffer descriptor with zero length. If we ever do that we're screwed in more ways than that.
Quoted 15 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-1}/* End for (;;) */ /* Write back buffer pointer */ @@ -336,22 +342,22 @@ static irqreturn_t cpm_uart_int(int irq,if (IS_SMC (pinfo)) { events = smcp->smc_smce; + smcp->smc_smce = events;if (events &SMCM_BRKE )uart_handle_break (port);if (events &SMCM_RX )cpm_uart_int_rx (port, regs);if (events &SMCM_TX )cpm_uart_int_tx (port, regs); - smcp->smc_smce = events;
drivers/serial/cpm_uart/cpm_uart_core.c static irqreturn_t cpm_uart_int(int irq, · line 354
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-1}else { events = sccp->scc_scce; + sccp->scc_scce = events;if (events &UART_SCCM_BRKE )uart_handle_break (port);if (events &UART_SCCM_RX )cpm_uart_int_rx (port, regs);if (events &UART_SCCM_TX )cpm_uart_int_tx (port, regs); - sccp->scc_scce = events;
This is a good catch...
Quoted 12 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-0}return (events) ?IRQ_HANDLED :IRQ_NONE ; }@@ -360,6 +366,7 @@ static int cpm_uart_startup(struct uart_ {int retval;struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port; +int line = pinfo - cpm_uart_ports;pr_debug ("CPM uart[%d]:startup\n" , port->line);
drivers/serial/cpm_uart/cpm_uart_core.c static int cpm_uart_startup(struct uart_ · line 384
Quoted 4 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-0@@ -374,18 +381,30 @@ static int cpm_uart_startup(struct uart_ pinfo->smcp->smc_smcmr |=SMCMR_REN ; }else { pinfo->sccp->scc_sccm |=UART_SCCM_RX ; + pinfo->sccp->scc_gsmrl |=SCC_GSMRL_ENR ;
dido as above.
Quoted 7 lines
drivers/serial/cpm_uart/cpm_uart_core.c +2
-0} +cpm_line_cr_cmd (line,CPM_CR_RESTART_TX );return 0 ; } +inline void cpm_uart_wait_until_send (struct uart_cpm_port *pinfo)
drivers/serial/cpm_uart/cpm_uart_core.c static int cpm_uart_startup(struct uart_ · line 401
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c +10
-0+{ +unsigned long orig_jiffies = jiffies; +while (1 ) + { +schedule_timeout (2 ); +if (time_after (jiffies, orig_jiffies + pinfo->wait_closing)) +break ; + } +} +
perhaps, more like...
unsigned long target_jiffies = jiffies + pinfo->wait_closing; while (!time_after(jiffies, target_jiffies))
schedule();
Quoted 34 lines
drivers/serial/cpm_uart/cpm_uart_core.c +6
-4/* * Shutdown the uart */ static void cpm_uart_shutdown (struct uart_port *port) {struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port; -int line = pinfo - cpm_uart_ports;pr_debug ("CPM uart[%d]:shutdown\n" , port->line);@@ -394,6 +413,12 @@ static void cpm_uart_shutdown(struct uar/* If the port is not the console, disable Rx and Tx. */ if (!(pinfo->flags &FLAG_CONSOLE )) { +/* Wait for all the BDs marked sent */ +while (!cpm_uart_tx_empty (port)) +schedule_timeout (2 ); +if (pinfo->wait_closing) +cpm_uart_wait_until_send (pinfo); +/* Stop uarts */ if (IS_SMC (pinfo)) {volatile smc_t *smcp = pinfo->smcp;@@ -405,9 +430,6 @@ static void cpm_uart_shutdown(struct uar sccp->scc_sccm &= ~(UART_SCCM_TX |UART_SCCM_RX ); } -/* Shut them really down and reinit buffer descriptors */ -cpm_line_cr_cmd (line,CPM_CR_STOP_TX ); -cpm_uart_initbd (pinfo); } }
drivers/serial/cpm_uart/cpm_uart_core.c static int cpm_uart_tx_pump(struct uart_ · line 597
Quoted 8 lines
drivers/serial/cpm_uart/cpm_uart_core.c +4
-1@@ -569,7 +591,10 @@ static int cpm_uart_tx_pump(struct uart_/* Pick next descriptor and fill from buffer */ bdp = pinfo->tx_cur; - p =bus_to_virt (bdp->cbd_bufaddr); +if (pinfo->dma_addr) + p=(u8 *)((ulong)(pinfo->mem_addr) + bdp->cbd_bufaddr - pinfo->dma_addr); +else + p =bus_to_virt (bdp->cbd_bufaddr);
this looks bogus to me...
Quoted 59 lines
drivers/serial/cpm_uart/cpm_uart_core.c +18
-4*p++ = xmit->buf[xmit->tail]; bdp->cbd_datlen =1 ; bdp->cbd_sc |=BD_SC_READY ;@@ -595,7 +620,10 @@ static int cpm_uart_tx_pump(struct uart_while (!(bdp->cbd_sc &BD_SC_READY ) && (xmit->tail != xmit->head)) { count =0 ; - p =bus_to_virt (bdp->cbd_bufaddr); +if (pinfo->dma_addr) + p=(u8 *)((ulong)(pinfo->mem_addr) + bdp->cbd_bufaddr - pinfo->dma_addr); +else + p =bus_to_virt (bdp->cbd_bufaddr);while (count < pinfo->tx_fifosize) { *p++ = xmit->buf[xmit->tail]; xmit->tail = (xmit->tail +1 ) & (UART_XMIT_SIZE -1 );@@ -606,6 +634,7 @@ static int cpm_uart_tx_pump(struct uart_ } bdp->cbd_datlen = count; bdp->cbd_sc |=BD_SC_READY ; + __asm__("eieio" );/* Get next BD. */ if (bdp->cbd_sc &BD_SC_WRAP ) bdp = pinfo->tx_bd_base;@@ -632,6 +661,7 @@ static void cpm_uart_initbd(struct uart_ {int i;u8 *mem_addr; +u8 * dma_addr;volatile cbd_t *bdp;pr_debug ("CPM uart[%d]:initbd\n" , pinfo->port.line);@@ -641,14 +671,23 @@ static void cpm_uart_initbd(struct uart_* virtual address for us to work with. */ mem_addr = pinfo->mem_addr; + dma_addr = (u8 *)(pinfo->dma_addr); bdp = pinfo->rx_cur = pinfo->rx_bd_base;for (i =0 ; i < (pinfo->rx_nrfifos -1 ); i++, bdp++) { - bdp->cbd_bufaddr =virt_to_bus (mem_addr); +if (pinfo->dma_addr) + bdp->cbd_bufaddr = (ulong )dma_addr; +else + bdp->cbd_bufaddr =virt_to_bus (mem_addr); bdp->cbd_sc =BD_SC_EMPTY |BD_SC_INTRPT ; + bdp->cbd_datlen =0 ; mem_addr += pinfo->rx_fifosize; + dma_addr += pinfo->rx_fifosize; } - - bdp->cbd_bufaddr =virt_to_bus (mem_addr); +if (pinfo->dma_addr) + bdp->cbd_bufaddr = (ulong )dma_addr; +else + bdp->cbd_bufaddr =virt_to_bus (mem_addr); + bdp->cbd_datlen =0 ; bdp->cbd_sc =BD_SC_WRAP |BD_SC_EMPTY |BD_SC_INTRPT ;/* Set the physical address of the host memory
drivers/serial/cpm_uart/cpm_uart_core.c static void cpm_uart_initbd(struct uart_ · line 697
Quoted 3 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0@@ -656,14 +695,21 @@ static void cpm_uart_initbd(struct uart_* virtual address for us to work with. */ mem_addr = pinfo->mem_addr + L1_CACHE_ALIGN(pinfo->rx_nrfifos *
pinfo->rx_fifosize);
drivers/serial/cpm_uart/cpm_uart_core.c static void cpm_uart_initbd(struct uart_ · line 698
Quoted 1 line
drivers/serial/cpm_uart/cpm_uart_core.c +1
-0+ dma_addr = (u8*)(pinfo->dma_addr + L1_CACHE_ALIGN(pinfo->rx_nrfifos *
pinfo->rx_fifosize));
Review
Quoted 81 lines
drivers/serial/cpm_uart/cpm_uart_core.c +20
-3bdp = pinfo->tx_cur = pinfo->tx_bd_base;for (i =0 ; i < (pinfo->tx_nrfifos -1 ); i++, bdp++) { - bdp->cbd_bufaddr =virt_to_bus (mem_addr); +if (pinfo->dma_addr) + bdp->cbd_bufaddr = (ulong )dma_addr; +else + bdp->cbd_bufaddr =virt_to_bus (mem_addr); bdp->cbd_sc =BD_SC_INTRPT ; mem_addr += pinfo->tx_fifosize; + dma_addr += pinfo->tx_fifosize; } - - bdp->cbd_bufaddr =virt_to_bus (mem_addr); +if (pinfo->dma_addr) + bdp->cbd_bufaddr = (ulong )dma_addr; +else + bdp->cbd_bufaddr =virt_to_bus (mem_addr); bdp->cbd_sc =BD_SC_WRAP |BD_SC_INTRPT ; }@@ -763,6 +809,8 @@ static void cpm_uart_init_smc(struct uar/* Using idle charater time requires some additional tuning. */ up->smc_mrblr = pinfo->rx_fifosize; up->smc_maxidl = pinfo->rx_fifosize; + up->smc_brklen =0 ; + up->smc_brkec =0 ; up->smc_brkcr =1 ;cpm_line_cr_cmd (line,CPM_CR_INIT_TRX );@@ -815,6 +863,10 @@ static int cpm_uart_request_port(structreturn ret;cpm_uart_initbd (pinfo); +if (IS_SMC (pinfo)) +cpm_uart_init_smc (pinfo); +else +cpm_uart_init_scc (pinfo);return 0 ; }@@ -902,6 +954,7 @@ struct uart_cpm_port cpm_uart_ports[UART .rx_nrfifos = RX_NUM_FIFO, .rx_fifosize = RX_BUF_SIZE, .set_lineif = scc1_lineif, + .wait_closing = SCC_WAIT_CLOSING, }, [UART_SCC2] = { .port = {@@ -915,6 +968,7 @@ struct uart_cpm_port cpm_uart_ports[UART .rx_nrfifos = RX_NUM_FIFO, .rx_fifosize = RX_BUF_SIZE, .set_lineif = scc2_lineif, + .wait_closing = SCC_WAIT_CLOSING, }, [UART_SCC3] = { .port = {@@ -928,6 +982,7 @@ struct uart_cpm_port cpm_uart_ports[UART .rx_nrfifos = RX_NUM_FIFO, .rx_fifosize = RX_BUF_SIZE, .set_lineif = scc3_lineif, + .wait_closing = SCC_WAIT_CLOSING, }, [UART_SCC4] = { .port = {@@ -941,6 +996,7 @@ struct uart_cpm_port cpm_uart_ports[UART .rx_nrfifos = RX_NUM_FIFO, .rx_fifosize = RX_BUF_SIZE, .set_lineif = scc4_lineif, + .wait_closing = SCC_WAIT_CLOSING, }, };@@ -1081,6 +1137,7 @@ static int __init cpm_uart_console_setupreturn ret;cpm_uart_initbd (pinfo); +cpm_uart_init_scc (pinfo);if (IS_SMC (pinfo))cpm_uart_init_smc (pinfo);(quoted diff) +0
-0
b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
Quoted 75 lines
drivers/serial/cpm_uart/cpm_uart_cpm1.c +23
-23@@ -82,6 +82,16 @@ void cpm_line_cr_cmd(int line, int cmd)void smc1_lineif (struct uart_cpm_port *pinfo) {volatile cpm8xx_t *cp = cpmp; + +#if defined (CONFIG_MPC885ADS ) +/* Enable SMC1 transceivers */ + { + cp->cp_pepar |=0x000000c0 ; + cp->cp_pedir &= ~0x000000c0 ; + cp->cp_peso &= ~0x00000040 ; + cp->cp_peso |=0x00000080 ; + } +#elif defined (CONFIG_MPC86XADS )unsigned int iobits =0x000000c0 ;if (!pinfo->is_portb) {@@ -93,41 +103,31 @@ void smc1_lineif(struct uart_cpm_port *p ((immap_t *)IMAP_ADDR )->im_ioport.iop_padir &= ~iobits; ((immap_t *)IMAP_ADDR )->im_ioport.iop_paodr &= ~iobits; } - -#ifdef CONFIG_MPC885ADS -/* Enable SMC1 transceivers */ - { -volatile uint __iomem *bcsr1 =ioremap (BCSR1 ,4 ); -uint tmp; - - tmp =in_be32 (bcsr1); - tmp &= ~BCSR1_RS232EN_1 ; -out_be32 (bcsr1, tmp); -iounmap (bcsr1); - }#endif - pinfo->brg =1 ; }void smc2_lineif (struct uart_cpm_port *pinfo) { -#ifdef CONFIG_MPC885ADS volatile cpm8xx_t *cp = cpmp; -volatile uint __iomem *bcsr1; -uint tmp; - +#if defined (CONFIG_MPC885ADS ) cp->cp_pepar |=0x00000c00 ; cp->cp_pedir &= ~0x00000c00 ; cp->cp_peso &= ~0x00000400 ; cp->cp_peso |=0x00000800 ; +#elif defined (CONFIG_MPC86XADS ) +unsigned int iobits =0x00000c00 ; + +if (!pinfo->is_portb) { + cp->cp_pbpar |= iobits; + cp->cp_pbdir &= ~iobits; + cp->cp_pbodr &= ~iobits; + }else { + ((immap_t *)IMAP_ADDR )->im_ioport.iop_papar |= iobits; + ((immap_t *)IMAP_ADDR )->im_ioport.iop_padir &= ~iobits; + ((immap_t *)IMAP_ADDR )->im_ioport.iop_paodr &= ~iobits; + } -/* Enable SMC2 transceivers */ - bcsr1 =ioremap (BCSR1 ,4 ); - tmp =in_be32 (bcsr1); - tmp &= ~BCSR1_RS232EN_2 ; -out_be32 (bcsr1, tmp); -iounmap (bcsr1);#endif pinfo->brg =2 ;
Panto, Kumar,
Thank you for review.
My comments:
Quoted 64 lines · Vitaly Bordug
Quoted 6 lines
Kumar, Pantelis,
[snip]
Some more comments.
Review
Quoted 3 lines
(quoted diff) +0
-0
b/drivers/serial/cpm_uart/cpm_uart.hReview
Quoted 23 lines
drivers/serial/cpm_uart/cpm_uart.h +4
-0@@ -40,6 +40,8 @@#define TX_NUM_FIFO 4#define TX_BUF_SIZE 32 +#define SCC_WAIT_CLOSING 100 +struct uart_cpm_port {struct uart_port port;u16 rx_nrfifos;@@ -67,6 +69,8 @@ struct uart_cpm_port {int bits;/* Keep track of 'odd' SMC2 wirings */ int is_portb; +/* wait on close if needed */ +int wait_closing; };extern int cpm_uart_port_map[UART_NR ];(quoted diff) +0
-0
b/drivers/serial/cpm_uart/cpm_uart_core.cdrivers/serial/cpm_uart/cpm_uart_core.c line 17
Quoted 9 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-0@@ -12,6 +12,7 @@* * Copyright (C) 2004 Freescale Semiconductor, Inc. * (C) 2004 Intracom, S.A. +* (C) 2005 MontaVista Software, Inc. by Vitaly Bordug Quoted 6 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0* * This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published bydrivers/serial/cpm_uart/cpm_uart_core.c +0
-0@@ -143,10 +144,13 @@ static void cpm_uart_start_tx(struct uar }
Quoted 17 lines
drivers/serial/cpm_uart/cpm_uart_core.c static void cpm_uart_start_tx(struct uar
Quoted 12 lines
drivers/serial/cpm_uart/cpm_uart_core.c +5
-2if (cpm_uart_tx_pump (port) !=0 ) { -if (IS_SMC (pinfo)) +if (IS_SMC (pinfo)) { smcp->smc_smcm |=SMCM_TX ; -else + smcp->smc_smcmr |=SMCMR_TEN ; + }else { sccp->scc_sccm |=UART_SCCM_TX ; + pinfo->sccp->scc_gsmrl |=SCC_GSMRL_ENT ; + }Why the need to mess with the global SCC transmit enable here? It's dubious IMO.
But without it (at least my boards) will have TX disabled. Just look - we have enabled this bit in pinfo->sccp->scc_gsmrl within ..._init_scc. Then all will
be fine until shutdown which will clear it and related in sccp->scc_sccm as well. The latter will be restored in start_tx, but scc_gsmrl will not. This results in the only first successful transmission.
Quoted 11 lines
Quoted 11 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0}}
drivers/serial/cpm_uart/cpm_uart_core.c +1
-1@@ -265,13 +269,15 @@ static void cpm_uart_int_rx(struct uart_ }/* End while (i--) */ /* This BD is ready to be used again. Clear status. get next */ - bdp->cbd_sc &= ~(BD_SC_BR |BD_SC_FR |BD_SC_PR |BD_SC_OV ); + bdp->cbd_sc &= ~(BD_SC_BR |BD_SC_FR |BD_SC_PR |BD_SC_OV |BD_SC_ID ); bdp->cbd_sc |=BD_SC_EMPTY ;
Quoted 18 lines
drivers/serial/cpm_uart/cpm_uart_core.c static void cpm_uart_int_rx(struct uart_
Quoted 12 lines
drivers/serial/cpm_uart/cpm_uart_core.c +6
-4-if (bdp->cbd_sc &BD_SC_WRAP ) - bdp = pinfo->rx_bd_base; -else - bdp++; +if (bdp->cbd_datlen) { +if (bdp->cbd_sc &BD_SC_WRAP ) + bdp = pinfo->rx_bd_base; +else + bdp++; + }Why is that? Where ever we queue a buffer descriptor with zero length. If we ever do that we're screwed in more ways than that.
I'm inclined to agree.
Quoted 43 lines
Quoted 17 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0}/* End for (;;) */ /* Write back buffer pointer */drivers/serial/cpm_uart/cpm_uart_core.c +1
-1@@ -336,22 +342,22 @@ static irqreturn_t cpm_uart_int(int irq,if (IS_SMC (pinfo)) { events = smcp->smc_smce; + smcp->smc_smce = events;if (events &SMCM_BRKE )uart_handle_break (port);if (events &SMCM_RX )cpm_uart_int_rx (port, regs);if (events &SMCM_TX )cpm_uart_int_tx (port, regs); - smcp->smc_smce = events; }else { events = sccp->scc_scce;drivers/serial/cpm_uart/cpm_uart_core.c static irqreturn_t cpm_uart_int(int irq, · line 362
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-1+ sccp->scc_scce = events;if (events &UART_SCCM_BRKE )uart_handle_break (port);if (events &UART_SCCM_RX )cpm_uart_int_rx (port, regs);if (events &UART_SCCM_TX )cpm_uart_int_tx (port, regs); - sccp->scc_scce = events;This is a good catch...
Quoted 11 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0}return (events) ? IRQ_HANDLED : IRQ_NONE; }drivers/serial/cpm_uart/cpm_uart_core.c +1
-0@@ -360,6 +366,7 @@ static int cpm_uart_startup(struct uart_ {int retval;struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port; +int line = pinfo - cpm_uart_ports;pr_debug ("CPM uart[%d]:startup\n" , port->line);
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c static int cpm_uart_startup(struct uart_ · line 386
Quoted 7 lines
drivers/serial/cpm_uart/cpm_uart_core.c +1
-0@@ -374,18 +381,30 @@ static int cpm_uart_startup(struct uart_ pinfo->smcp->smc_smcmr |=SMCMR_REN ; }else { pinfo->sccp->scc_sccm |=UART_SCCM_RX ; + pinfo->sccp->scc_gsmrl |=SCC_GSMRL_ENR ;dido as above.
Yeah, this is superfluous as it has been done above.
Quoted 21 lines
Quoted 9 lines
drivers/serial/cpm_uart/cpm_uart_core.c +4
-0} +cpm_line_cr_cmd (line,CPM_CR_RESTART_TX );return 0 ; } +inline void cpm_uart_wait_until_send (struct uart_cpm_port *pinfo) +{ +unsigned long orig_jiffies = jiffies;drivers/serial/cpm_uart/cpm_uart_core.c static int cpm_uart_startup(struct uart_ · line 410
Quoted 10 lines
drivers/serial/cpm_uart/cpm_uart_core.c +8
-0+while (1 ) + { +schedule_timeout (2 ); +if (time_after (jiffies, orig_jiffies + pinfo->wait_closing)) +break ; + } +} +perhaps, more like...
Quoted 7 lines
unsigned long target_jiffies = jiffies + pinfo->wait_closing; while (!time_after(jiffies, target_jiffies))
schedule();
No objections, I'll try this one.
Quoted 37 lines
Quoted 37 lines
drivers/serial/cpm_uart/cpm_uart_core.c +0
-0/* * Shutdown the uart */ static void cpm_uart_shutdown (struct uart_port *port) {struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port;drivers/serial/cpm_uart/cpm_uart_core.c +0
-1-int line = pinfo - cpm_uart_ports;pr_debug("CPM uart[%d]:shutdown\n", port->line);drivers/serial/cpm_uart/cpm_uart_core.c +6
-3@@ -394,6 +413,12 @@ static void cpm_uart_shutdown(struct uar/* If the port is not the console, disable Rx and Tx. */ if (!(pinfo->flags &FLAG_CONSOLE )) { +/* Wait for all the BDs marked sent */ +while (!cpm_uart_tx_empty (port)) +schedule_timeout (2 ); +if (pinfo->wait_closing) +cpm_uart_wait_until_send (pinfo); +/* Stop uarts */ if (IS_SMC (pinfo)) {volatile smc_t *smcp = pinfo->smcp;@@ -405,9 +430,6 @@ static void cpm_uart_shutdown(struct uar sccp->scc_sccm &= ~(UART_SCCM_TX |UART_SCCM_RX ); } -/* Shut them really down and reinit buffer descriptors */ -cpm_line_cr_cmd (line,CPM_CR_STOP_TX ); -cpm_uart_initbd (pinfo); } }@@ -569,7 +591,10 @@ static int cpm_uart_tx_pump(struct uart_/* Pick next descriptor and fill from buffer */ bdp = pinfo->tx_cur;
Quoted 12 lines
drivers/serial/cpm_uart/cpm_uart_core.c static int cpm_uart_tx_pump(struct uart_
Quoted 7 lines
drivers/serial/cpm_uart/cpm_uart_core.c +4
-1- p =bus_to_virt (bdp->cbd_bufaddr); +if (pinfo->dma_addr) + p=(u8 *)((ulong)(pinfo->mem_addr) + bdp->cbd_bufaddr - pinfo->dma_addr); +else + p =bus_to_virt (bdp->cbd_bufaddr);this looks bogus to me...
Well, all the stuff works on 8272 even without this and likewise stuff, but don't on 866ADS, where bus_to_virt returns value not equal to where we allocated DMA. I didn't dig too deep to track why this happens, since if we're using DMA, we should remember addresses upon allocation and avoid using bus_to_virt.
Signature · 1 line
Sincerely, Vitaly
Quoted 4 lines · you
Panto, Kumar,
Thank you for review.
[major snipage]
Quoted 5 lines
Well, all the stuff works on 8272 even without this and likewise stuff, but don't on 866ADS, where
bus_to_virtreturns value not equal to where we allocated DMA. I didn't dig too deep to track why this happens, since if we're using DMA, we should remember addresses upon allocation and avoid usingbus_to_virt.
Well, this is weird cause I've tested my latest patch on 8xx and had no problems...
I'll try to test your patch, some time later today.
However, what is important now is to get the fs_enet driver posted. Please test the latest patch I've send you on a linus tree and report if you have any problems...
Regards
Pantelis
Quoted 9 lines
Quoted 8 lines
Well, all the stuff works on 8272 even without this and likewise stuff, but don't on 866ADS, where
bus_to_virtreturns value not equal to where we allocated DMA. I didn't dig too deep to track why this happens, since if we're using DMA, we should remember addresses upon allocation and avoid usingbus_to_virt.
Quoted 4 lines
Well, this is weird cause I've tested my latest patch on 8xx and had no problems...
Since currently I have no ability to reproduce this issue, I'm inclined to drop those hunks. BTW, why those bus_to_virt and virt_to_bus are used? Isn't it more clear to use pure addresses if they are dma-allocated?
Quoted 8 lines
I'll try to test your patch, some time later today.
However, what is important now is to get the
fs_enetdriver posted. Please test the latest patch I've send you on a linus tree and report if you have any problems...
I'll try to test it today/tomorrow as time permits... But do you think it can proceed with those mii issues? Though they are almost harmless, the driver will complain that no PHY found but all seem to work fine subsequently. As far as Andy's phy will likely to be in rc14, maybe we can try to push this upstream (as what we have currently for 82xx and especially for 8xx is IMO worse than even incomplete implementation). I'll try to investigate mii troubles, but it's hard to estimate efforts...
Quoted 7 lines
Regards
Pantelis
Signature · 1 line
Sincerely, Vitaly