arm926_dma_flush_range undefined!
Subsystems (2)
Hi all,
I am trying to compile a recent kernel (v2.6.34-rc6-201-g722154e) and I am having this kind of error:
ERROR: "arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!
As the arm926_dma_flush_range assembler function seems to exits in the kernel, I have difficulties to figure out what the issue comes from...
As I am maintainer of this driver, I would like to fix this very fast ;-)
This error shows up while I compile this driver as a module.
Do you have any hint?
Thanks, best regards,
Signature · 1 line
Nicolas Ferre
Quoted 5 lines · Nicolas Ferre
I am trying to compile a recent kernel
(v2.6.34-rc6-201-g722154e) and I am
having this kind of error:ERROR: "
arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!
The driver seems to use dmac_flush_range() directly. That's not part of the DMA API. Could you not use one of the supported DMA API functions?
Signature · 1 line
Catalin
Quoted 9 lines · Catalin Marinas
Quoted 5 lines · Nicolas Ferre
I am trying to compile a recent kernel
(v2.6.34-rc6-201-g722154e) and I am
having this kind of error:ERROR: "
arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!The driver seems to use
dmac_flush_range()directly. That's not part of the DMA API. Could you not use one of the supported DMA API functions?
Indeed; I've always said that I don't care about drivers directly using the internals of the DMA API, and drivers doing this will be constantly subjected to breakage.
I really do not regard the above to be a regression; it's a latent programming error. AT91 folk need to fix their driver(s) to use the proper interfaces rather than using internal functionality.
Le 06/05/2010 19:59, Russell King - ARM Linux :
Quoted 10 lines · Catalin Marinas
Quoted 9 lines · Nicolas Ferre
Quoted 5 lines
I am trying to compile a recent kernel
(v2.6.34-rc6-201-g722154e) and I am
having this kind of error:ERROR: "
arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!The driver seems to use
dmac_flush_range()directly. That's not part of the DMA API. Could you not use one of the supported DMA API functions?
The difficult part for me is to choose the proper one ;-)
And also, I am wondering if this call is needed as we do a kunmap_atomic() on the same scatterlist pointer just before. Does not kunmap_atomic() embed a cache flushing directive already?
Quoted 7 lines
Indeed; I've always said that I don't care about drivers directly using the internals of the DMA API, and drivers doing this will be constantly subjected to breakage.
I really do not regard the above to be a regression; it's a latent programming error. AT91 folk need to fix their driver(s) to use the proper interfaces rather than using internal functionality.
For sure, I will !
Thanks, best regards,
Signature · 1 line
Nicolas Ferre
Quoted 9 lines · Nicolas Ferre
Le 06/05/2010 19:59, Russell King - ARM Linux :
Quoted 8 lines · Catalin Marinas
Quoted 7 lines · Nicolas Ferre
Quoted 5 lines
I am trying to compile a recent kernel
(v2.6.34-rc6-201-g722154e) and I am
having this kind of error:ERROR: "
arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
The driver seems to use
dmac_flush_range()directly. That's not part of the DMA API. Could you not use one of the supported DMA API functions?The difficult part for me is to choose the proper one ;-)
And also, I am wondering if this call is needed as we do a
kunmap_atomic()on the same scatterlist pointer just before. Does notkunmap_atomic()embed a cache flushing directive already?
No it doesn't. What are you trying to do here?
Le 11/05/2010 11:44, Russell King - ARM Linux :
Quoted 13 lines · Nicolas Ferre
Quoted 12 lines
Le 06/05/2010 19:59, Russell King - ARM Linux :
Quoted 10 lines · Catalin Marinas
Quoted 9 lines · Nicolas Ferre
Quoted 5 lines
I am trying to compile a recent kernel
(v2.6.34-rc6-201-g722154e) and I am
having this kind of error:ERROR: "
arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!The driver seems to use
dmac_flush_range()directly. That's not part of the DMA API. Could you not use one of the supported DMA API functions?
Quoted 7 lines
Quoted 5 lines
The difficult part for me is to choose the proper one ;-)
And also, I am wondering if this call is needed as we do a
kunmap_atomic()on the same scatterlist pointer just before. Does notkunmap_atomic()embed a cache flushing directive already?No it doesn't. What are you trying to do here?
After a read with DMA from sd/mmc interface, a DMA buffer allocated by dma_alloc_coherent() is filled.
This buffer is then copied to the scatterlist provided by the mmc request (from drivers/mmc/card/block.c it seems that sg buffers are alocacted via kmalloc()).
For each buffer of the scatterlist, I do this dmac_flush_rage()...
As copy is done between a coherent and a kernel memory buffers, I guess that the flushing routine is not needed?
Best regards,
Signature · 1 line
Nicolas Ferre
Quoted 5 lines · Nicolas Ferre
After a read with DMA from sd/mmc interface, a DMA buffer allocated by
dma_alloc_coherent()is filled.
This buffer is then copied to the scatterlist provided by the mmc request (fromdrivers/mmc/card/block.cit seems that sg buffers are alocacted viakmalloc()).
I assume that this means you can't handle scatterlists with your DMA engine?
Quoted 1 line
For each buffer of the scatterlist, I do this
dmac_flush_rage()...
I think you need to look at how mmci.c does its flushing, and do something along those lines. You could use flush_kernel_dcache_page() instead of flush_dcache_page(), which would be slightly cheaper.
Quoted 2 lines
As copy is done between a coherent and a kernel memory buffers, I guess that the flushing routine is not needed?
The issue of coherency with "PIO" writes to block IO pages is going around the same old loops of discussion yet again at the moment; it's been a problem for ARM for about the last 10 years or so, and I'm not expecting there to suddenly be a major change of heart or political will to fix the issue.
So I think we're stuck with having to have the drivers we really care about and have our own control over do the flushing themselves.
PATCH 1/1 MMC: at91_mci: modify cache flush routines
As we were using an internal dma flushing routine, this patch changes to the DMA API flush_kernel_dcache_page(). Driver is able to compile now.
--- drivers/mmc/host/at91_mci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/at91_mci.c +1 -1
@@ -315,7 +315,7 @@ static void at91_mci_post_dma_read(struct at91mci_host *host) }kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ ); -dmac_flush_range ((void *)sgbuffer, ((void *)sgbuffer) + amount); +flush_kernel_dcache_page (sg_page (sg)); data->bytes_xfered += amount;if (size ==0 )break ;
Signature · 1 line
1.5.6.5
Re: [PATCH] MMC: at91_mci: modify cache flush routines
Quoted 13 lines · On Tue, 11 May 2010 19:09:53 +0200 Nicolas Ferre <[email protected]>…
As we were using an internal dma flushing routine, this patch changes to the DMA API
flush_kernel_dcache_page(). Driver is able to compile now.Signed-off-by Nicolas Ferre <[email protected]>--- drivers/mmc/host/at91_mci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/mmc/host/at91_mci.c +0
-0
drivers/mmc/host/at91_mci.c static void at91_mci_post_dma_read(struct at91mci_host *host) · line 321
Quoted 8 lines
drivers/mmc/host/at91_mci.c +1
-1@@ -315,7 +315,7 @@ static void at91_mci_post_dma_read(struct at91mci_host *host) }kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ ); -dmac_flush_range ((void *)sgbuffer, ((void *)sgbuffer) + amount); +flush_kernel_dcache_page (sg_page (sg)); data->bytes_xfered += amount;if (size ==0 )break ;
The flush_kernel_dcache_page() documentation specifically says that thou shalt run flush_kernel_dcache_page() _prior_ to kunmapping the page.
I don't know if that makes a difference in the real world, but heck why not:
drivers/mmc/host/at91_mci.c renamed from drivers/mmc/host/at91_mci.c~mmc-at91_mci-modify-cache-flush-routines-fix +1 -1
@@ -314,8 +314,8 @@ static void at91_mci_post_dma_read(struc dmabuf = (unsigned *)tmpv; } -kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ );flush_kernel_dcache_page (sg_page (sg)); +kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ ); data->bytes_xfered += amount;if (size ==0 )break ;
_
However, I'm wondering why you chose flush_kernel_dcache_page() instead of plain old flush_dcache_page(). Is this a pagecache or possibly direct-io page we're dealing with here?
Re: [PATCH] MMC: at91_mci: modify cache flush routines
Quoted 3 lines · Andrew Morton
The
flush_kernel_dcache_page()documentation specifically says that thou shalt runflush_kernel_dcache_page()_prior_ to kunmapping the page.
Hmm, interesting - I can't see why that would be, but it doesn't make any difference for ARM (and this is an ARM only driver).
Quoted 2 lines
I don't know if that makes a difference in the real world, but heck why not:
In the interests of stopping cut'n'paste bugs into other drivers, I'd say this is a good idea even if it makes no difference to ARM.
Quoted 3 lines
However, I'm wondering why you chose
flush_kernel_dcache_page()instead of plain oldflush_dcache_page(). Is this a pagecache or possibly direct-io page we're dealing with here?
It's whatever the block layers hand us - which would be page cache pages, and I'd assume also DIO pages (I'm not up on DIO stuff though.)
It's also my understanding that the preferred interface for drivers which write to page cache pages is flush_kernel_dcache_page() rather than flush_dcache_page().
Re: [PATCH] MMC: at91_mci: modify cache flush routines
In reply to Andrew MortonLe 12/05/2010 23:19, Andrew Morton :
Quoted 44 lines
Quoted 13 lines · On Tue, 11 May 2010 19:09:53 +0200 Nicolas Ferre <[email protected]>…
As we were using an internal dma flushing routine, this patch changes to the DMA API
flush_kernel_dcache_page(). Driver is able to compile now.Signed-off-by Nicolas Ferre <[email protected]>--- drivers/mmc/host/at91_mci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)drivers/mmc/host/at91_mci.c +0
-0drivers/mmc/host/at91_mci.c static void at91_mci_post_dma_read(struct at91mci_host *host) · line 321
Quoted 8 lines
drivers/mmc/host/at91_mci.c +1
-1@@ -315,7 +315,7 @@ static void at91_mci_post_dma_read(struct at91mci_host *host) }kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ ); -dmac_flush_range ((void *)sgbuffer, ((void *)sgbuffer) + amount); +flush_kernel_dcache_page (sg_page (sg)); data->bytes_xfered += amount;if (size ==0 )break ;The
flush_kernel_dcache_page()documentation specifically says that thou shalt runflush_kernel_dcache_page()_prior_ to kunmapping the page.I don't know if that makes a difference in the real world, but heck why not:
drivers/mmc/host/at91_mci.c renamed from drivers/mmc/host/at91_mci.c~mmc-at91_mci-modify-cache-flush-routines-fix +1
-1@@ -314,8 +314,8 @@ static void at91_mci_post_dma_read(struc dmabuf = (unsigned *)tmpv; } -kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ );flush_kernel_dcache_page (sg_page (sg)); +kunmap_atomic (sgbuffer,KM_BIO_SRC_IRQ ); data->bytes_xfered += amount;if (size ==0 )break ;
Quoted 1 line
_
Andrew, thanks a lot for folding this in my patch.
I had no access to my emails last days and it was good to learn that this patch went to 2.6.34-final.
Best regards,
Signature · 1 line
Nicolas Ferre
Quoted 4 lines · Russell King - ARM Linux
Quoted 2 lines · Nicolas Ferre
As copy is done between a coherent and a kernel memory buffers, I guess that the flushing routine is not needed?
Quoted 8 lines
The issue of coherency with "PIO" writes to block IO pages is going around the same old loops of discussion yet again at the moment; it's been a problem for ARM for about the last 10 years or so, and I'm not expecting there to suddenly be a major change of heart or political will to fix the issue.
So I think we're stuck with having to have the drivers we really care about and have our own control over do the flushing themselves.
For non-aliasing VIPT hardware we still have the option of doing what the IA-64 and PowerPC guys have done (flushing in set_pte_at with PG_arch_1 meaning "clean"). It may even work for VIVT caches if we take care to also flush the kernel alias in set_pte_at.
If you are ok with this approach, I'll prepare some patches (it seems that trying to get an agreement with other architectures isn't very productive).
Signature · 1 line
Catalin
Quoted 4 lines · Catalin Marinas
For non-aliasing VIPT hardware we still have the option of doing what the IA-64 and PowerPC guys have done (flushing in
set_pte_atwithPG_arch_1meaning "clean"). It may even work for VIVT caches if we take care to also flush the kernel alias inset_pte_at.
It should work for VIVT as well, so lets just go ahead and negate the PG_arch_1 meaning. I was rather expecting this to be done a month or so ago while we still had plenty of time before the next merge window - and given where we are now (maybe 2 weeks away) I think it's too risky to push into the upcoming window.
I'm already planning on scheduling the ioremap and LMB changes for the following window, and I think this should be scheduled likewise.
Quoted 3 lines
If you are ok with this approach, I'll prepare some patches (it seems that trying to get an agreement with other architectures isn't very productive).
That's what I've always found with this issue; whenever I've talked with the mainline community about PIO cache coherency, the result has always been non-productive. Hence why it's always remained a problem for us.
The issue is that too many people have their own strong ideas about how "we" should fix the problem - and each idea is different, and they don't accept each other's ideas. They won't even talk to each other and discuss their differences to resolve it either, presumably because they see that it's not their problem to resolve.
I'm rather heartened by the outcome of your attempts to discuss this; it has illustrated that there's no possibility of an agreement being reached, which although sad, merely confirms my conclusion formed over the past years.
However, thanks for making the effort; it's a shame it wasn't more productive in the end.
Quoted 11 lines · Russell King - ARM Linux
Quoted 4 lines · Catalin Marinas
For non-aliasing VIPT hardware we still have the option of doing what the IA-64 and PowerPC guys have done (flushing in
set_pte_atwithPG_arch_1meaning "clean"). It may even work for VIVT caches if we take care to also flush the kernel alias inset_pte_at.It should work for VIVT as well, so lets just go ahead and negate the
PG_arch_1meaning. I was rather expecting this to be done a month or so ago while we still had plenty of time before the next merge window - and given where we are now (maybe 2 weeks away) I think it's too risky to push into the upcoming window.
Well, I started and then I had to go on holiday (and drive back ~1500 miles because of the volcanic ash).
Quoted 2 lines
I'm already planning on scheduling the ioremap and LMB changes for the following window, and I think this should be scheduled likewise.
The cache flushing change would need some wider testing, so maybe 6-8 weeks should be enough. I'll try to get some rfc patches by the end of this week.
Quoted 8 lines
Quoted 3 lines
If you are ok with this approach, I'll prepare some patches (it seems that trying to get an agreement with other architectures isn't very productive).
That's what I've always found with this issue; whenever I've talked with the mainline community about PIO cache coherency, the result has always been non-productive. Hence why it's always remained a problem for us.
The good thing is that I found out how PowerPC deals with it :)
Signature · 1 line
Catalin
Hi,
Am Dienstag, 11. Mai 2010 schrieb Nicolas Ferre:
Quoted 24 lines
Le 11/05/2010 11:44, Russell King - ARM Linux :
Quoted 15 lines · Nicolas Ferre
Quoted 14 lines
Le 06/05/2010 19:59, Russell King - ARM Linux :
Quoted 12 lines · Catalin Marinas
Quoted 11 lines · Nicolas Ferre
Quoted 6 lines
I am trying to compile a recent kernel
(v2.6.34-rc6-201-g722154e) and I am
having this kind of error:ERROR: "
arm926_dma_flush_range" [drivers/mmc/host/at91_mci.ko] undefined!The driver seems to use
dmac_flush_range()directly. That's not part of the DMA API. Could you not use one of the supported DMA API functions?Quoted 7 lines
Quoted 5 lines
The difficult part for me is to choose the proper one ;-)
And also, I am wondering if this call is needed as we do a
kunmap_atomic()on the same scatterlist pointer just before. Does notkunmap_atomic()embed a cache flushing directive already?No it doesn't. What are you trying to do here?
Quoted 7 lines
After a read with DMA from sd/mmc interface, a DMA buffer allocated by
dma_alloc_coherent()is filled.
This buffer is then copied to the scatterlist provided by the mmc request (fromdrivers/mmc/card/block.cit seems that sg buffers are alocacted viakmalloc()).For each buffer of the scatterlist, I do this
dmac_flush_rage()...
Yepp!
Quoted 2 lines
As copy is done between a coherent and a kernel memory buffers, I guess that the flushing routine is not needed?
Where do you get this wisdom?
The coherent buffer does not need a cache flush, never.
But the target buffer does need a flush(). If the loaded data from SD card is an executable or library, the target buffer may be mapped into the text segment of a process and be executed. And this will fail if parts of the data are only inside the data cache (especially on ARM architectures where data and instruction cache are separate).
The basic assumption is that the scatterlist buffer is filled directly with a DMA controller: data is in memory, not in the cache.
If you fill the scatterlist buffer with the help of the CPU, you need a data cache flush afterwards to fullfill this assumption.
It's a shame that the function to do so is not available on all platforms. IMHO it should.
best regards
i. A. Wolfgang M?es
Signature · 12 lines
Auerswald Gesellschaft f?r Datensysteme mbH Hardware Development Telefon: +49 (0)5306 9219 562 Telefax: +49 (0)5306 9219 94 E-Mail: Wolfgang.Mues at Auerswald.de Web: http://www.auerswald.de -------------------------------------------------------------- Auerswald Gesellschaft f?r Datensysteme mbH Vor den Grash?fen 1, 38162 Cremlingen Registriert beim AG Braunschweig HRB 7499 Gesch?ftsf?hrer: Dipl-Ing. Gerhard Auerswald