kernel 2.6.15: cpm_uart driver broken?

[Kenneth Poole]

Hello Dan:

>
>From: Dan Malek [mailto:dan at embeddedalley.com] 
>Sent: Wednesday, April 19, 2006 3:42 PM
>To: Kenneth Poole
>Cc: Vitaly Bordug; David Jander; linuxppc-embedded at ozlabs.org
>Subject: Re: kernel 2.6.15: cpm_uart driver broken?


>On Apr 19, 2006, at 3:24 PM, Kenneth Poole wrote:

>>
>>  		/* get pointer */
>> -		cp = cpm2cpu_addr(bdp->cbd_bufaddr);
>> +		cp = (unsigned char *)pinfo->mem_addr +
>> (bdp->cbd_bufaddr - pinfo->dma_addr);


>Ummm, no.  Keep the cpm2cpu_addr() and pass it
>some driver data structure pointer so it does the computes,
>

We did this to solve the problem mentioned earlier in this thread which
is that bus_to_virt() and virt_to_bus() don't work for memory allocated
with dma_alloc_coherent(). By saving the return value from
dma_alloc_coherent() in dma_addr, we avoid having to do those
conversions all of the time.

>or better, keep the phys/virt addresses in a handy data
>structure you can easily access and work with offsets within
>the different address spaces.

That's what we're doing by using dma_addr. We just compute the offset in
the physical address space and apply that offset in the virtual address
space. Some drivers keep both sets of pointers around for each buffer
allocated, and that would work as well.

>  The test of addr >= CPM_ADDR
>is critically important to early boot and kgdb support
>and can't be removed.

I assumed that this test was simply to exclude console ports from
conversion. But in cpm_uart_allocbuf() there already is a test (is_con).
For the console, we simply save the unconverted dma_addr in the port
structure once and use it normally. Our console port works just fine.
Testing addr >= CPM_ADDR forces very specific ordering onto the IMMR and
DMA address spaces, and I thought our approach was more general. 

>> -		bdp->cbd_bufaddr = cpu2cpm_addr(mem_addr);
>> +		bdp->cbd_bufaddr = dma_addr;

>This kind of initialization is broken, too.  You have to test
>that memory address and return the proper space.

>Thanks.

>	-- Dan