LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
@ 2008-02-11 16:23 Roel Kluin
  2008-02-11 16:42 ` Matthew Wilcox
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Roel Kluin @ 2008-02-11 16:23 UTC (permalink / raw)
  To: kyle, Matthew Wilcox, grundler; +Cc: linux-parisc, lkml

duplicate pa11_dma_alloc_consistent; more appropriate appears
pa11_dma_alloc_noncoherent here. 

Not tested, please confirm that this fix is correct
---
fix noncoherent allocation

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..0e8b71f 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -549,7 +549,7 @@ static void pa11_dma_sync_sg_for_device(struct device *dev, struct scatterlist *
 struct hppa_dma_ops pcxl_dma_ops = {
 	.dma_supported =	pa11_dma_supported,
 	.alloc_consistent =	pa11_dma_alloc_consistent,
-	.alloc_noncoherent =	pa11_dma_alloc_consistent,
+	.alloc_noncoherent =	pa11_dma_alloc_noncoherent,
 	.free_consistent =	pa11_dma_free_consistent,
 	.map_single =		pa11_dma_map_single,
 	.unmap_single =		pa11_dma_unmap_single,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
  2008-02-11 16:23 [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Roel Kluin
@ 2008-02-11 16:42 ` Matthew Wilcox
  2008-02-11 16:45   ` Roel Kluin
  2008-02-11 16:55   ` James Bottomley
  2008-02-11 16:45 ` James Bottomley
  2008-02-11 19:10 ` [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Kyle McMartin
  2 siblings, 2 replies; 10+ messages in thread
From: Matthew Wilcox @ 2008-02-11 16:42 UTC (permalink / raw)
  To: Roel Kluin; +Cc: kyle, grundler, linux-parisc, lkml

On Mon, Feb 11, 2008 at 05:23:33PM +0100, Roel Kluin wrote:
> duplicate pa11_dma_alloc_consistent; more appropriate appears
> pa11_dma_alloc_noncoherent here. 
> 
> Not tested, please confirm that this fix is correct

I don't think it is.  The memories are fading, so I don't recall why it
is we do it this way, but I'm pretty sure it's correct the way it is.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
  2008-02-11 16:23 [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Roel Kluin
  2008-02-11 16:42 ` Matthew Wilcox
@ 2008-02-11 16:45 ` James Bottomley
  2008-02-11 16:57   ` Roel Kluin
  2008-02-11 19:10 ` [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Kyle McMartin
  2 siblings, 1 reply; 10+ messages in thread
From: James Bottomley @ 2008-02-11 16:45 UTC (permalink / raw)
  To: Roel Kluin; +Cc: kyle, Matthew Wilcox, grundler, linux-parisc, lkml


On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
> duplicate pa11_dma_alloc_consistent; more appropriate appears
> pa11_dma_alloc_noncoherent here. 
> 
> Not tested, please confirm that this fix is correct

No, it looks completely incorrect to me.  What makes you think a pcxl
box has a problem with coherency?

James



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
  2008-02-11 16:42 ` Matthew Wilcox
@ 2008-02-11 16:45   ` Roel Kluin
  2008-02-11 16:55   ` James Bottomley
  1 sibling, 0 replies; 10+ messages in thread
From: Roel Kluin @ 2008-02-11 16:45 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: kyle, grundler, linux-parisc, lkml

Matthew Wilcox wrote:
> On Mon, Feb 11, 2008 at 05:23:33PM +0100, Roel Kluin wrote:
>> duplicate pa11_dma_alloc_consistent; more appropriate appears
>> pa11_dma_alloc_noncoherent here. 
>>
>> Not tested, please confirm that this fix is correct
> 
> I don't think it is.  The memories are fading, so I don't recall why it
> is we do it this way, but I'm pretty sure it's correct the way it is.
> 
Maybe this helps a bit: later I found that something similar occurs in 
drivers/parisc/{ccio-dma.c, sba_iommu.c}:

1010: .alloc_consistent =     ccio_alloc_consistent,
      .alloc_noncoherent =    ccio_alloc_consistent,

1036: .alloc_consistent =     sba_alloc_consistent,
      .alloc_noncoherent =    sba_alloc_consistent,

However in these files the functions {ccio_alloc, sba_alloc}_noncoherent
do not exist.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
  2008-02-11 16:42 ` Matthew Wilcox
  2008-02-11 16:45   ` Roel Kluin
@ 2008-02-11 16:55   ` James Bottomley
  1 sibling, 0 replies; 10+ messages in thread
From: James Bottomley @ 2008-02-11 16:55 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Roel Kluin, kyle, grundler, linux-parisc, lkml


On Mon, 2008-02-11 at 09:42 -0700, Matthew Wilcox wrote:
> On Mon, Feb 11, 2008 at 05:23:33PM +0100, Roel Kluin wrote:
> > duplicate pa11_dma_alloc_consistent; more appropriate appears
> > pa11_dma_alloc_noncoherent here. 
> > 
> > Not tested, please confirm that this fix is correct
> 
> I don't think it is.  The memories are fading, so I don't recall why it
> is we do it this way, but I'm pretty sure it's correct the way it is.

dma_alloc_noncoherent is a fallback for boxes with coherency problems
which cannot allocate coherent memory.  The API asks for coherent memory
but the driver promises to behave correctly even if an incoherent area
is returned.  dma_alloc_coherent() requires only coherent memory.  The
pcx boxes (PA7200 and below) can't do the uncached trick for coherent
memory, so the API is designed for them.  The PCXL can, so we allocate
coherent memory even for dma_alloc_noncoherent() in that case (and nop
out the coherency handlers the driver uses).

James



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
  2008-02-11 16:45 ` James Bottomley
@ 2008-02-11 16:57   ` Roel Kluin
  2008-02-11 17:08     ` [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent John David Anglin
  0 siblings, 1 reply; 10+ messages in thread
From: Roel Kluin @ 2008-02-11 16:57 UTC (permalink / raw)
  To: James Bottomley; +Cc: kyle, Matthew Wilcox, grundler, linux-parisc, lkml

James Bottomley wrote:
> On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
>> duplicate pa11_dma_alloc_consistent; more appropriate appears
>> pa11_dma_alloc_noncoherent here. 
>>
>> Not tested, please confirm that this fix is correct
> 
> No, it looks completely incorrect to me.  What makes you think a pcxl
> box has a problem with coherency?

Ok, please ignore the patch then. It just appeared suspicious to me
that the function did exist, but the names assigned were different.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c]      pcxl_dma_ops.alloc_noncoherent
  2008-02-11 16:57   ` Roel Kluin
@ 2008-02-11 17:08     ` John David Anglin
  2008-02-11 18:56       ` Roel Kluin
  0 siblings, 1 reply; 10+ messages in thread
From: John David Anglin @ 2008-02-11 17:08 UTC (permalink / raw)
  To: Roel Kluin
  Cc: James.Bottomley, kyle, matthew, grundler, linux-parisc, linux-kernel

> James Bottomley wrote:
> > On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
> >> duplicate pa11_dma_alloc_consistent; more appropriate appears
> >> pa11_dma_alloc_noncoherent here. 
> >>
> >> Not tested, please confirm that this fix is correct
> > 
> > No, it looks completely incorrect to me.  What makes you think a pcxl
> > box has a problem with coherency?
> 
> Ok, please ignore the patch then. It just appeared suspicious to me
> that the function did exist, but the names assigned were different.

How about a comment?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c]      pcxl_dma_ops.alloc_noncoherent
  2008-02-11 17:08     ` [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent John David Anglin
@ 2008-02-11 18:56       ` Roel Kluin
  2008-02-11 18:58         ` Kyle McMartin
  0 siblings, 1 reply; 10+ messages in thread
From: Roel Kluin @ 2008-02-11 18:56 UTC (permalink / raw)
  To: John David Anglin
  Cc: James.Bottomley, kyle, matthew, grundler, linux-parisc, linux-kernel

John David Anglin wrote:
>> James Bottomley wrote:
>>> On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
>>>> duplicate pa11_dma_alloc_consistent; more appropriate appears
>>>> pa11_dma_alloc_noncoherent here. 
>>>>
>>>> Not tested, please confirm that this fix is correct
>>> No, it looks completely incorrect to me.  What makes you think a pcxl
>>> box has a problem with coherency?
>> Ok, please ignore the patch then. It just appeared suspicious to me
>> that the function did exist, but the names assigned were different.
> 
> How about a comment?

Based on James Bottomley's explanation maybe a comment like this?
---
Explain why dma_alloc_noncoherent is only used for boxes PA7200 and below

Signed-off-by: Roel Kluin <12o3l@tiscali.nl>
---
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..fc3325a 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -567,6 +567,10 @@ static void *fail_alloc_consistent(struct device *dev, size_t size,
 	return NULL;
 }
 
+/*
+ * dma_alloc_noncoherent is a fallback for boxes PA7200 and below which
+ * cannot allocate coherent memory.
+ */
 static void *pa11_dma_alloc_noncoherent(struct device *dev, size_t size,
 					  dma_addr_t *dma_handle, gfp_t flag)
 {
@@ -586,6 +590,10 @@ static void pa11_dma_free_noncoherent(struct device *dev, size_t size,
 	return;
 }
 
+/*
+ * PCXL allocates coherent memory even for dma_alloc_noncoherent() due to the
+ * uncached trick for coherent memory.
+ */
 struct hppa_dma_ops pcx_dma_ops = {
 	.dma_supported =	pa11_dma_supported,
 	.alloc_consistent =	fail_alloc_consistent,


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent
  2008-02-11 18:56       ` Roel Kluin
@ 2008-02-11 18:58         ` Kyle McMartin
  0 siblings, 0 replies; 10+ messages in thread
From: Kyle McMartin @ 2008-02-11 18:58 UTC (permalink / raw)
  To: Roel Kluin
  Cc: John David Anglin, James.Bottomley, kyle, matthew, grundler,
	linux-parisc, linux-kernel

On Mon, Feb 11, 2008 at 07:56:10PM +0100, Roel Kluin wrote:
> +/*
> + * dma_alloc_noncoherent is a fallback for boxes PA7200 and below which
> + * cannot allocate coherent memory.
> + */
>  static void *pa11_dma_alloc_noncoherent(struct device *dev, size_t size,
>  					  dma_addr_t *dma_handle, gfp_t flag)
>  {

Why? This is located below pcxl_dma_ops so its obviously only relevant
for pcx.

> @@ -586,6 +590,10 @@ static void pa11_dma_free_noncoherent(struct device *dev, size_t size,
>  	return;
>  }
>  
> +/*
> + * PCXL allocates coherent memory even for dma_alloc_noncoherent() due to the
> + * uncached trick for coherent memory.
> + */

This isn't correct either.

>  struct hppa_dma_ops pcx_dma_ops = {
>  	.dma_supported =	pa11_dma_supported,
>  	.alloc_consistent =	fail_alloc_consistent,
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?
  2008-02-11 16:23 [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Roel Kluin
  2008-02-11 16:42 ` Matthew Wilcox
  2008-02-11 16:45 ` James Bottomley
@ 2008-02-11 19:10 ` Kyle McMartin
  2 siblings, 0 replies; 10+ messages in thread
From: Kyle McMartin @ 2008-02-11 19:10 UTC (permalink / raw)
  To: Roel Kluin; +Cc: kyle, Matthew Wilcox, grundler, linux-parisc, lkml

On Mon, Feb 11, 2008 at 05:23:33PM +0100, Roel Kluin wrote:
>

How about just doing something like this:

diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..63f9b7f 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -409,7 +409,7 @@ pcxl_dma_init(void)
 
 __initcall(pcxl_dma_init);
 
-static void * pa11_dma_alloc_consistent (struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t flag)
+static void * pcxl_dma_alloc_consistent (struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t flag)
 {
 	unsigned long vaddr;
 	unsigned long paddr;
@@ -435,7 +435,7 @@ static void * pa11_dma_alloc_consistent (struct device *dev, size_t size, dma_ad
 	return (void *)vaddr;
 }
 
-static void pa11_dma_free_consistent (struct device *dev, size_t size, void *vaddr, dma_addr_t dma_handle)
+static void pcxl_dma_free_consistent (struct device *dev, size_t size, void *vaddr, dma_addr_t dma_handle)
 {
 	int order;
 
@@ -548,9 +548,9 @@ static void pa11_dma_sync_sg_for_device(struct device *dev, struct scatterlist *
 
 struct hppa_dma_ops pcxl_dma_ops = {
 	.dma_supported =	pa11_dma_supported,
-	.alloc_consistent =	pa11_dma_alloc_consistent,
-	.alloc_noncoherent =	pa11_dma_alloc_consistent,
-	.free_consistent =	pa11_dma_free_consistent,
+	.alloc_consistent =	pcxl_dma_alloc_consistent,
+	.alloc_noncoherent =	pcxl_dma_alloc_consistent,
+	.free_consistent =	pcxl_dma_free_consistent,
 	.map_single =		pa11_dma_map_single,
 	.unmap_single =		pa11_dma_unmap_single,
 	.map_sg =		pa11_dma_map_sg,
@@ -567,7 +567,7 @@ static void *fail_alloc_consistent(struct device *dev, size_t size,
 	return NULL;
 }
 
-static void *pa11_dma_alloc_noncoherent(struct device *dev, size_t size,
+static void *pcx_dma_alloc_noncoherent(struct device *dev, size_t size,
 					  dma_addr_t *dma_handle, gfp_t flag)
 {
 	void *addr;
@@ -579,7 +579,7 @@ static void *pa11_dma_alloc_noncoherent(struct device *dev, size_t size,
 	return addr;
 }
 
-static void pa11_dma_free_noncoherent(struct device *dev, size_t size,
+static void pcx_dma_free_noncoherent(struct device *dev, size_t size,
 					void *vaddr, dma_addr_t iova)
 {
 	free_pages((unsigned long)vaddr, get_order(size));
@@ -589,8 +589,8 @@ static void pa11_dma_free_noncoherent(struct device *dev, size_t size,
 struct hppa_dma_ops pcx_dma_ops = {
 	.dma_supported =	pa11_dma_supported,
 	.alloc_consistent =	fail_alloc_consistent,
-	.alloc_noncoherent =	pa11_dma_alloc_noncoherent,
-	.free_consistent =	pa11_dma_free_noncoherent,
+	.alloc_noncoherent =	pcx_dma_alloc_noncoherent,
+	.free_consistent =	pcx_dma_free_noncoherent,
 	.map_single =		pa11_dma_map_single,
 	.unmap_single =		pa11_dma_unmap_single,
 	.map_sg =		pa11_dma_map_sg,

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2008-02-11 19:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-11 16:23 [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Roel Kluin
2008-02-11 16:42 ` Matthew Wilcox
2008-02-11 16:45   ` Roel Kluin
2008-02-11 16:55   ` James Bottomley
2008-02-11 16:45 ` James Bottomley
2008-02-11 16:57   ` Roel Kluin
2008-02-11 17:08     ` [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent John David Anglin
2008-02-11 18:56       ` Roel Kluin
2008-02-11 18:58         ` Kyle McMartin
2008-02-11 19:10 ` [PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent? Kyle McMartin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).