Ouch.
I would convert the warning printk() to a BUG(), since such an address
error
indicates deep trouble anyway...
And add a "likely()" indication around the range check.
Looks great otherwise!
On Tue, 2006-04-25 at 20:26 +0400, Vitaly Bordug wrote:
> Current address translation methods can produce wrong results, because
> virt_to_bus and vice versa may not produce correct offsets on dma-allocated
> memory. The right way is, while tracking both phys and virt address of the
> window that has been allocated for boffer descriptors, and use those
> numbers to compute the offset and make translation properly.
>
> Signed-off-by: Vitaly Bordug <vbordug at ru.mvista.com>
> ---
>
> drivers/serial/cpm_uart/cpm_uart.h | 33
> +++++++++++++++++++++++++++++++
> drivers/serial/cpm_uart/cpm_uart_core.c | 31 ++++++++---------------------
> drivers/serial/cpm_uart/cpm_uart_cpm1.c | 7 ++++---
> drivers/serial/cpm_uart/cpm_uart_cpm2.c | 5 ++++-
> 4 files changed, 50 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/serial/cpm_uart/cpm_uart.h
> b/drivers/serial/cpm_uart/cpm_uart.h
> index 17f2c7a..9db402c 100644
> --- a/drivers/serial/cpm_uart/cpm_uart.h
> +++ b/drivers/serial/cpm_uart/cpm_uart.h
> @@ -66,6 +66,7 @@ struct uart_cpm_port {
> uint dp_addr;
> void *mem_addr;
> dma_addr_t dma_addr;
> + u32 mem_size;
> /* helpers */
> int baud;
> int bits;
> @@ -92,4 +93,36 @@ void scc2_lineif(struct uart_cpm_port *p
> void scc3_lineif(struct uart_cpm_port *pinfo);
> void scc4_lineif(struct uart_cpm_port *pinfo);
>
> +/*
> + virtual to phys transtalion
> +*/
> +static inline unsigned long cpu2cpm_addr(void* addr, struct uart_cpm_port
> *pinfo)
> +{
> + int offset;
> + u32 val = (u32)addr;
> + /* sane check */
> + if ((val >= (u32)pinfo->mem_addr) &&
> + (val<((u32)pinfo->mem_addr + pinfo->mem_size))) {
> + offset = val - (u32)pinfo->mem_addr;
> + return pinfo->dma_addr+offset;
> + }
> + printk("%s(): address %x to translate out of range!\n", __FUNCTION__,
> val);
> + return 0;
> +}
> +
> +static inline void *cpm2cpu_addr(unsigned long addr, struct uart_cpm_port
> *pinfo)
> +{
> + int offset;
> + u32 val = addr;
> + /* sane check */
> + if ((val >= pinfo->dma_addr) &&
> + (val<(pinfo->dma_addr + pinfo->mem_size))) {
> + offset = val - (u32)pinfo->dma_addr;
> + return (void*)(pinfo->mem_addr+offset);
> + }
> + printk("%s(): address %x to translate out of range!\n", __FUNCTION__,
> val);
> + return 0;
> +}
> +
> +
> #endif /* CPM_UART_H */
> diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
> b/drivers/serial/cpm_uart/cpm_uart_core.c
> index 8f33815..3549073 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_core.c
> @@ -72,19 +72,6 @@ static void cpm_uart_initbd(struct uart_
>
> /**************************************************************/
>
> -static inline unsigned long cpu2cpm_addr(void *addr)
> -{
> - if ((unsigned long)addr >= CPM_ADDR)
> - return (unsigned long)addr;
> - return virt_to_bus(addr);
> -}
> -
> -static inline void *cpm2cpu_addr(unsigned long addr)
> -{
> - if (addr >= CPM_ADDR)
> - return (void *)addr;
> - return bus_to_virt(addr);
> -}
>
> /* Place-holder for board-specific stuff */
> struct platform_device* __attribute__ ((weak)) __init
> @@ -290,7 +277,7 @@ static void cpm_uart_int_rx(struct uart_
> }
>
> /* get pointer */
> - cp = cpm2cpu_addr(bdp->cbd_bufaddr);
> + cp = cpm2cpu_addr(bdp->cbd_bufaddr, pinfo);
>
> /* loop through the buffer */
> while (i-- > 0) {
> @@ -633,7 +620,7 @@ static int cpm_uart_tx_pump(struct uart_
> /* Pick next descriptor and fill from buffer */
> bdp = pinfo->tx_cur;
>
> - p = cpm2cpu_addr(bdp->cbd_bufaddr);
> + p = cpm2cpu_addr(bdp->cbd_bufaddr, pinfo);
>
> *p++ = port->x_char;
> bdp->cbd_datlen = 1;
> @@ -660,7 +647,7 @@ static int cpm_uart_tx_pump(struct uart_
>
> while (!(bdp->cbd_sc & BD_SC_READY) && (xmit->tail != xmit->head)) {
> count = 0;
> - p = cpm2cpu_addr(bdp->cbd_bufaddr);
> + p = cpm2cpu_addr(bdp->cbd_bufaddr, pinfo);
> while (count < pinfo->tx_fifosize) {
> *p++ = xmit->buf[xmit->tail];
> xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> @@ -709,12 +696,12 @@ static void cpm_uart_initbd(struct uart_
> mem_addr = pinfo->mem_addr;
> bdp = pinfo->rx_cur = pinfo->rx_bd_base;
> for (i = 0; i < (pinfo->rx_nrfifos - 1); i++, bdp++) {
> - bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr);
> + bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr, pinfo);
> bdp->cbd_sc = BD_SC_EMPTY | BD_SC_INTRPT;
> mem_addr += pinfo->rx_fifosize;
> }
>
> - bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr);
> + bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr, pinfo);
> bdp->cbd_sc = BD_SC_WRAP | BD_SC_EMPTY | BD_SC_INTRPT;
>
> /* Set the physical address of the host memory
> @@ -724,12 +711,12 @@ static void cpm_uart_initbd(struct uart_
> mem_addr = pinfo->mem_addr + L1_CACHE_ALIGN(pinfo->rx_nrfifos *
> pinfo->rx_fifosize);
> bdp = pinfo->tx_cur = pinfo->tx_bd_base;
> for (i = 0; i < (pinfo->tx_nrfifos - 1); i++, bdp++) {
> - bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr);
> + bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr, pinfo);
> bdp->cbd_sc = BD_SC_INTRPT;
> mem_addr += pinfo->tx_fifosize;
> }
>
> - bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr);
> + bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr, pinfo);
> bdp->cbd_sc = BD_SC_WRAP | BD_SC_INTRPT;
> }
>
> @@ -1099,7 +1086,7 @@ static void cpm_uart_console_write(struc
> * If the buffer address is in the CPM DPRAM, don't
> * convert it.
> */
> - cp = cpm2cpu_addr(bdp->cbd_bufaddr);
> + cp = cpm2cpu_addr(bdp->cbd_bufaddr, pinfo);
>
> *cp = *s;
>
> @@ -1116,7 +1103,7 @@ static void cpm_uart_console_write(struc
> while ((bdp->cbd_sc & BD_SC_READY) != 0)
> ;
>
> - cp = cpm2cpu_addr(bdp->cbd_bufaddr);
> + cp = cpm2cpu_addr(bdp->cbd_bufaddr, pinfo);
>
> *cp = 13;
> bdp->cbd_datlen = 1;
> diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> index 31223aa..a5a3062 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c
> @@ -144,7 +144,7 @@ int cpm_uart_allocbuf(struct uart_cpm_po
> /* was hostalloc but changed cause it blows away the */
> /* large tlb mapping when pinning the kernel area */
> mem_addr = (u8 *) cpm_dpram_addr(cpm_dpalloc(memsz, 8));
> - dma_addr = 0;
> + dma_addr = (u32)mem_addr;
> } else
> mem_addr = dma_alloc_coherent(NULL, memsz, &dma_addr,
> GFP_KERNEL);
> @@ -157,8 +157,9 @@ int cpm_uart_allocbuf(struct uart_cpm_po
> }
>
> pinfo->dp_addr = dp_offset;
> - pinfo->mem_addr = mem_addr;
> - pinfo->dma_addr = dma_addr;
> + pinfo->mem_addr = mem_addr; /* virtual address*/
> + pinfo->dma_addr = dma_addr; /* physical address*/
> + pinfo->mem_size = memsz;
>
> pinfo->rx_buf = mem_addr;
> pinfo->tx_buf = pinfo->rx_buf + L1_CACHE_ALIGN(pinfo->rx_nrfifos
> diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> index c9c3b1d..7c6b07a 100644
> --- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> +++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c
> @@ -209,8 +209,10 @@ int cpm_uart_allocbuf(struct uart_cpm_po
>
> memsz = L1_CACHE_ALIGN(pinfo->rx_nrfifos * pinfo->rx_fifosize) +
> L1_CACHE_ALIGN(pinfo->tx_nrfifos * pinfo->tx_fifosize);
> - if (is_con)
> + if (is_con) {
> mem_addr = alloc_bootmem(memsz);
> + dma_addr = mem_addr;
> + }
> else
> mem_addr = dma_alloc_coherent(NULL, memsz, &dma_addr,
> GFP_KERNEL);
> @@ -225,6 +227,7 @@ int cpm_uart_allocbuf(struct uart_cpm_po
> pinfo->dp_addr = dp_offset;
> pinfo->mem_addr = mem_addr;
> pinfo->dma_addr = dma_addr;
> + pinfo->mem_size = memsz;
>
> pinfo->rx_buf = mem_addr;
> pinfo->tx_buf = pinfo->rx_buf + L1_CACHE_ALIGN(pinfo->rx_nrfifos
