LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* question : DMA of PCI bridge
From: Sauce.Cheng @ 2009-05-20  3:19 UTC (permalink / raw)
  To: linuxppc-dev


excuse me
I hate to bother everyone but I have a question about DMA of PCI bridge

Now I attempt to fetch data from peripheral device to SDRAM, and it has been
successed

but how the DMA controller know the data bandwidth of src and dest.

for example, if i get a 16bits data with a 32bits bus, and other 16bits will
be set high
and data will fetched into cache line of dma, then it will be wrote to
32bits SDRAM.

i guess the data will be wrong, isn't ?

please give me some advices. thank you

-- 
View this message in context: http://www.nabble.com/question-%3A-DMA-of-PCI-bridge-tp23628338p23628338.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* Re: Musings on PCI busses
From: Grant Likely @ 2009-05-20  3:17 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Roderick Colenbrander, John Linn
In-Reply-To: <1242788522.16901.134.camel@pasglop>

On Tue, May 19, 2009 at 9:02 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-05-19 at 09:25 -0700, Stephen Neuendorffer wrote:
>
>> I agree that something is called for... =A0The first might be slightly
>> simpler, since it would probably transparently deal with the presence
>> of more than one PLB->PCI bridge?
>
> The current code doesn't already ?

Current code doesn't exist in mainline.  :-)

And, no, the patch Roderick wrote doesn't handle more than one PHB.
No reason it couldn't though.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* RE: Musings on PCI busses
From: Benjamin Herrenschmidt @ 2009-05-20  3:02 UTC (permalink / raw)
  To: Stephen Neuendorffer; +Cc: linuxppc-dev, Roderick Colenbrander, John Linn
In-Reply-To: <20090519162511.7454D17E8058@mail19-dub.bigfish.com>

On Tue, 2009-05-19 at 09:25 -0700, Stephen Neuendorffer wrote:

> I agree that something is called for...  The first might be slightly
> simpler, since it would probably transparently deal with the presence
> of more than one PLB->PCI bridge?

The current code doesn't already ?

Cheers,
Ben.

^ permalink raw reply

* Re: Musings on PCI busses
From: Benjamin Herrenschmidt @ 2009-05-20  3:01 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, Roderick Colenbrander, John Linn
In-Reply-To: <fa686aa40905190828x251765dek586f5bf8e73e4cf7@mail.gmail.com>

On Tue, 2009-05-19 at 09:28 -0600, Grant Likely wrote:

> 1) Probe the host controller in an of_platform driver.  This has the
> advantage of simplicity.  The probe routine will get automatically
> called when the PCI host controller device tree node is registered
> with the of_platform bus.  The bus parenthood also gets reflected in
> the device model and sysfs.  The disadvantage is that it defers PCI
> bus probing until after the of_platform bus is probed (maybe this is
> okay; maybe this already happens anyway).

We are doing that on Cell as mentioned by Arnd. However, there are a
few issues with that approach. One is that it relies on PCI-hotplug
related bits that currently only exist in 64-bit land. Another is
that doing so tends to expose various hidden issues or subtle
dependencies to the PCI stuff being setup early and probed early.

For example, some of the OF parsing bits may fail to convert memory
addresses to IO addresses if the PCI host bridges have not been
discovered yet, potentially causing issues with matching of resources
between the early serial stuff and the generic serial driver (if you
use an IO driven UART, the PCI 8250 driver may not properly figure out
that what it's finding in its IO BARs is actually the same port as
what it gets from the platform code as the later will end up with
memory addresses rather than IO ones). That's one example.

At this stage, I much prefer if we stick to doing the PHB discovery
at setup_arch() time, especially since there's other WIP in the area
of PCI coming from Kumar and I don't want to mixup multiple source of
problems in that area at the same time.

We can revisit that later but we'll have to be very careful.

Cheers,
Ben.

^ permalink raw reply

* [PATCH] powerpc/mm: Fix some SMP issues with MMU context handling
From: Benjamin Herrenschmidt @ 2009-05-20  2:56 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Kumar Gala

This patch fixes a couple of issues that can happen as a result
of steal_context() dropping the context_lock when all possible
PIDs are ineligible for stealing (hopefully an extremely hard to
hit occurence).

This case exposes the possibility of a stale context_mm[] entry
to be seen since destroy_context() doesn't clear it and the free
map isn't re-tested. It also means steal_context() will not notice
a context freed while the lock was help, thus possibly trying to
steal a context when a free one was available.

This fixes it by always returning to the caller from steal_context
when it dropped the lock with a return value that causes the
caller to re-samble the number of free contexts, along with
properly clearing the context_mm[] array for destroyed contexts.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 arch/powerpc/mm/mmu_context_nohash.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

--- linux-work.orig/arch/powerpc/mm/mmu_context_nohash.c	2009-05-20 12:39:21.000000000 +1000
+++ linux-work/arch/powerpc/mm/mmu_context_nohash.c	2009-05-20 12:49:45.000000000 +1000
@@ -73,7 +73,6 @@ static unsigned int steal_context_smp(un
 	struct mm_struct *mm;
 	unsigned int cpu, max;
 
- again:
 	max = last_context - first_context;
 
 	/* Attempt to free next_context first and then loop until we manage */
@@ -108,7 +107,9 @@ static unsigned int steal_context_smp(un
 	spin_unlock(&context_lock);
 	cpu_relax();
 	spin_lock(&context_lock);
-	goto again;
+
+	/* This will cause the caller to try again */
+	return MMU_NO_CONTEXT;
 }
 #endif  /* CONFIG_SMP */
 
@@ -194,6 +195,8 @@ void switch_mmu_context(struct mm_struct
 		WARN_ON(prev->context.active < 1);
 		prev->context.active--;
 	}
+
+ again:
 #endif /* CONFIG_SMP */
 
 	/* If we already have a valid assigned context, skip all that */
@@ -212,7 +215,8 @@ void switch_mmu_context(struct mm_struct
 #ifdef CONFIG_SMP
 		if (num_online_cpus() > 1) {
 			id = steal_context_smp(id);
-			goto stolen;
+			if (id == MMU_NO_CONTEXT)
+				goto again;
 		}
 #endif /* CONFIG_SMP */
 		id = steal_context_up(id);
@@ -286,8 +290,8 @@ void destroy_context(struct mm_struct *m
 		mm->context.id = MMU_NO_CONTEXT;
 #ifdef DEBUG_MAP_CONSISTENCY
 		mm->context.active = 0;
-		context_mm[id] = NULL;
 #endif
+		context_mm[id] = NULL;
 		nr_free_contexts++;
 	}
 	spin_unlock(&context_lock);

^ permalink raw reply

* [PATCH] tape: beyond ARRAY_SIZE of viocd_diskinfo
From: Roel Kluin @ 2009-05-20  0:58 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sfr, Andrew Morton, lkml

Do not go beyond ARRAY_SIZE of tape_device and viotape_unitinfo

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
diff --git a/drivers/char/viotape.c b/drivers/char/viotape.c
index ffc9254..042c814 100644
--- a/drivers/char/viotape.c
+++ b/drivers/char/viotape.c
@@ -867,7 +867,7 @@ static int viotape_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	int j;
 	struct device_node *node = vdev->dev.archdata.of_node;
 
-	if (i > VIOTAPE_MAX_TAPE)
+	if (i >= VIOTAPE_MAX_TAPE)
 		return -ENODEV;
 	if (!node)
 		return -ENODEV;

^ permalink raw reply related

* Re: Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.
From: Hongjun Chen @ 2009-05-20  0:39 UTC (permalink / raw)
  To: Piotr Zięcik, John Rigby; +Cc: linuxppc-dev@ozlabs.org, Wolfgang Denk
In-Reply-To: <200905191003.42964.kosmo@semihalf.com>

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

Hi John,

Did you use DMA driver in our FEC driver before?

Best Regards,



Hongjun Chen
2009-05-20



发件人: Piotr Zięcik
发送时间: 2009-05-19 16:05:05
收件人: Hongjun Chen
抄送: Wolfgang Denk; linuxppc-dev@ozlabs.org
主题: Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.

Tuesday 19 May 2009 03:37:47 Hongjun Chen wrote:
> Since you have reuse most part of our BSP, why should you reinvent wheel
> for MPC512x DMA? We have one ready DMA driver, which has been used by AC97,
> ATA, VIU etc.
>

Answer is simple. The old one does not fit Linux DMA API. New one uses 
existing infrastructure which makes DMA engine aviable for existing consumers
in Linux kernel. For example network stack. Support for I/O  <- > memory 
transfers will be added when more consumers arrive.

-- 
Best Regards.
Piotr Zięcik

[-- Attachment #2: Type: text/html, Size: 3654 bytes --]

^ permalink raw reply

* [PATCH] powerpc/5200: beyond ARRAY_SIZE of mpc52xx_uart_{ports, nodes}
From: Roel Kluin @ 2009-05-20  0:28 UTC (permalink / raw)
  To: grant.likely; +Cc: linuxppc-dev, Andrew Morton

Do not go beyond ARRAY_SIZE of mpc52xx_uart_{ports,nodes}

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
index 7f72f8c..b3feb61 100644
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -988,7 +988,7 @@ mpc52xx_console_setup(struct console *co, char *options)
 	pr_debug("mpc52xx_console_setup co=%p, co->index=%i, options=%s\n",
 		 co, co->index, options);
 
-	if ((co->index < 0) || (co->index > MPC52xx_PSC_MAXNUM)) {
+	if ((co->index < 0) || (co->index >= MPC52xx_PSC_MAXNUM)) {
 		pr_debug("PSC%x out of range\n", co->index);
 		return -EINVAL;
 	}

^ permalink raw reply related

* Re: Initialize DBCR0 for PPC440 targets
From: David Gibson @ 2009-05-19 23:53 UTC (permalink / raw)
  To: srikanth krishnakar; +Cc: Linuxppc-dev, Linuxppc-embedded
In-Reply-To: <6213bc560905190608j73f4191fxbd94158a2b1740c0@mail.gmail.com>

On Tue, May 19, 2009 at 06:38:53PM +0530, srikanth krishnakar wrote:
> Hi,
> 
> kernel- 2.6.29
> Debug technique: KGDB
> 
> The PowerPC kernel does not initialize the PPC440 DBCR0 register. This
> prevents the use of software breakpoints in case of internal debug
> mode. Looking into head_fsl_booke.S for initialization of DBCR0 is
> used by boot-loaders.
> It seems head_44x.S lacks this step of DBCR0 register initialization.
> So fixing this with initializing the DBCR0 register as shown below :
> 
> Subject: [PATCH] powerpc: 44x: Initialize DBCR0 for targets not having
> bootloader
> 
> The kernel does not initialize the PPC440 DBCR0 register.
> This prevents (among other things) the use of software
> breakpoints with GDB. The boot loaders probably do initialize
> this but few targets run without a boot loader

Um.. how does this prevent the use of software breakpoints with gdb?
The trap instructions still work with IDM==0.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: mpc5200 fec error
From: Grant Likely @ 2009-05-19 23:36 UTC (permalink / raw)
  To: Eric Millbrandt; +Cc: linuxppc-dev
In-Reply-To: <A88094362DE0AE49A118AB9B4EB3612403F9DEBC@dekaexchange.deka.local>

On Tue, May 19, 2009 at 4:21 PM, Eric Millbrandt
<emillbrandt@dekaresearch.com> wrote:
> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang@pengutronix.de]
> Sent: Tuesday, May 19, 2009 16:57
> To: Robert Schwebel
> Cc: Eric Millbrandt; linuxppc-dev@ozlabs.org
> Subject: Re: mpc5200 fec error
>
> On Tue, May 19, 2009 at 10:36:45PM +0200, Robert Schwebel wrote:
>> Wolfram, have you seen this mail? You recently tested -rt on 2.6.29,
>> right? Did you only test that on the customer hardware or also on the
>> phyCORE-MPC5200B?
>
> So far, I tried only on customer hardware, and that was 2.6.29.2-rt11.
> With
> that setup, I could work with NFS flawlessly, no real stress-testing
> though.
> Testing latest .3-rt is on my todo, will check the phyCOREs, too.
>
> Regards,
>
> =A0 Wolfram
>
> --
> Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | Wo=
lfram Sang
> |
> Industrial Linux Solutions =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | http://www.p=
engutronix.de/
> |
>
> I am able to reproduce the error using 2.6.29.2-rt11. =A0I was able to
> mitigate the problem by raising the priority of the transmit irq.
> However when running an NFS server on the pcm030 under high cpu load I
> now get
>
> [ =A0132.477503] net eth0: FEC_IEVENT_RFIFO_ERROR
> [ =A0132.892329] net eth0: FEC_IEVENT_RFIFO_ERROR
> [ =A0133.884109] net eth0: FEC_IEVENT_RFIFO_ERROR
> [ =A0134.876059] net eth0: FEC_IEVENT_RFIFO_ERROR
>
> Raising the priority of the rx irq does not seem to fix this problem
> though.

Hi Eric,

This error has been seen before in non-rt kernels.  I haven't had the
chance to track it down and kill it yet.  I believe there are locking
issues associated with it.

g.

>
>
> _________________________________________________________________________=
________________
>
> This e-mail and the information, including any attachments, it contains a=
re intended to be a confidential communication only to the person or entity=
 to whom it is addressed and may contain information that is privileged. If=
 the reader of this message is not the intended recipient, you are hereby n=
otified that any dissemination, distribution or copying of this communicati=
on is strictly prohibited. If you have received this communication in error=
, please immediately notify the sender and destroy the original message.
>
> Thank you.
>
> Please consider the environment before printing this email.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* [PATCH] fsldma: fix "DMA halt timeout!" errors
From: Ira Snyder @ 2009-05-19 22:42 UTC (permalink / raw)
  To: linuxppc-dev, Li Yang, Dan Williams

When using the DMA controller from multiple threads at the same time, it is
possible to get lots of "DMA halt timeout!" errors printed to the kernel
log.

This occurs due to a race between fsl_dma_memcpy_issue_pending() and the
interrupt handler, fsl_dma_chan_do_interrupt(). Both call the
fsl_chan_xfer_ld_queue() function, which does not protect against
concurrent accesses to dma_halt() and dma_start().

The existing spinlock is moved to cover the dma_halt() and dma_start()
functions. Testing shows that the "DMA halt timeout!" errors disappear.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/dma/fsldma.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 465846c..8db3ffd 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -841,15 +841,16 @@ static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan)
 	dma_addr_t next_dest_addr;
 	unsigned long flags;
 
+	spin_lock_irqsave(&fsl_chan->desc_lock, flags);
+
 	if (!dma_is_idle(fsl_chan))
-		return;
+		goto out_unlock;
 
 	dma_halt(fsl_chan);
 
 	/* If there are some link descriptors
 	 * not transfered in queue. We need to start it.
 	 */
-	spin_lock_irqsave(&fsl_chan->desc_lock, flags);
 
 	/* Find the first un-transfer desciptor */
 	for (ld_node = fsl_chan->ld_queue.next;
@@ -860,8 +861,6 @@ static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan)
 				fsl_chan->common.cookie) == DMA_SUCCESS);
 		ld_node = ld_node->next);
 
-	spin_unlock_irqrestore(&fsl_chan->desc_lock, flags);
-
 	if (ld_node != &fsl_chan->ld_queue) {
 		/* Get the ld start address from ld_queue */
 		next_dest_addr = to_fsl_desc(ld_node)->async_tx.phys;
@@ -873,6 +872,9 @@ static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan)
 		set_cdar(fsl_chan, 0);
 		set_ndar(fsl_chan, 0);
 	}
+
+out_unlock:
+	spin_unlock_irqrestore(&fsl_chan->desc_lock, flags);
 }
 
 /**
-- 
1.5.4.3

^ permalink raw reply related

* RE: mpc5200 fec error
From: Eric Millbrandt @ 2009-05-19 22:21 UTC (permalink / raw)
  To: Wolfram Sang, Robert Schwebel; +Cc: linuxppc-dev
In-Reply-To: <20090519205706.GD27476@pengutronix.de>

-----Original Message-----
From: Wolfram Sang [mailto:w.sang@pengutronix.de]=20
Sent: Tuesday, May 19, 2009 16:57
To: Robert Schwebel
Cc: Eric Millbrandt; linuxppc-dev@ozlabs.org
Subject: Re: mpc5200 fec error

On Tue, May 19, 2009 at 10:36:45PM +0200, Robert Schwebel wrote:
> Wolfram, have you seen this mail? You recently tested -rt on 2.6.29,
> right? Did you only test that on the customer hardware or also on the
> phyCORE-MPC5200B?

So far, I tried only on customer hardware, and that was 2.6.29.2-rt11.
With
that setup, I could work with NFS flawlessly, no real stress-testing
though.
Testing latest .3-rt is on my todo, will check the phyCOREs, too.

Regards,

   Wolfram

--=20
Pengutronix e.K.                           | Wolfram Sang
|
Industrial Linux Solutions                 | http://www.pengutronix.de/
|

I am able to reproduce the error using 2.6.29.2-rt11.  I was able to
mitigate the problem by raising the priority of the transmit irq.
However when running an NFS server on the pcm030 under high cpu load I
now get

[  132.477503] net eth0: FEC_IEVENT_RFIFO_ERROR
[  132.892329] net eth0: FEC_IEVENT_RFIFO_ERROR
[  133.884109] net eth0: FEC_IEVENT_RFIFO_ERROR
[  134.876059] net eth0: FEC_IEVENT_RFIFO_ERROR

Raising the priority of the rx irq does not seem to fix this problem
though.


_________________________________________________________________________=
________________

This e-mail and the information, including any attachments, it contains =
are intended to be a confidential communication only to the person or =
entity to whom it is addressed and may contain information that is =
privileged. If the reader of this message is not the intended recipient, =
you are hereby notified that any dissemination, distribution or copying =
of this communication is strictly prohibited. If you have received this =
communication in error, please immediately notify the sender and destroy =
the original message.

Thank you.

Please consider the environment before printing this email.

^ permalink raw reply

* Re: [PATCH 02/12] fs_enet: Add MPC5121 FEC support.
From: Benjamin Herrenschmidt @ 2009-05-19 21:56 UTC (permalink / raw)
  To: Piotr Zięcik
  Cc: Becky Bruce, Wolfgang Denk, Detlev Zundel, John Rigby, netdev,
	linuxppc-dev, Scott Wood
In-Reply-To: <200905191326.29122.kosmo@semihalf.com>

On Tue, 2009-05-19 at 13:26 +0200, Piotr Zięcik wrote:
> Tuesday 19 May 2009 00:17:31 Benjamin Herrenschmidt wrote:
> >
> > We are close to the point where we can make this a runtime option
> > though, by just having a different set of dma_ops hooked in.
> >
> 
> Is somebody currently working on it? If yes, where we can see
> code ?

Well, the current upstream code has the dma ops support. Becky is
massaging that more for swiotlb support which has been submitted to the
linuxppc-dev list. We haven't yet added runtime support rather than
compile time for non-cache coherent platforms, but it's becoming
reasonably easy to add now (though still needs to be done).

Cheers,
Ben

^ permalink raw reply

* Re: [PATCH 2/2] qe: add polling timeout to qe_issue_cmd()
From: Timur Tabi @ 2009-05-19 21:09 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, smaclennan
In-Reply-To: <fa686aa40905191310h3ed0efdfn4bd4b4f4b9ca0b7d@mail.gmail.com>

Grant Likely wrote:

> Hmmm, the ret value is backwards from what most coders would expect
> (zero on success, non-zero on failure).  I'd personally recommend
> reversing the polarity in the macro.

The ret value is documented as being the value of the expression when the loop terminates.  The reason it appears backwards is because the expression is

	(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG) == 0

That is, the loop should spin until the QE_CR_FLG is zero.  So when the loop terminates normally, the expression (x) == 0 is true, which is equal to one.

I would expect that in most cases, the loop spins until some bit is *set*.  Let's pretend that QE_CR_FLG operates this way.  In that case, the call would look like this:

spin_event_timeout(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG, 100, 0, ret);

At loop termination, the result of the expression will be either QE_CR_FLG or zero.

> Otherwise, feel free to add my acked-by line to both patches.

Thanks.

Ben, would you please apply this to your 'next' branch?

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc: Add support for swiotlb on 32-bit
From: Becky Bruce @ 2009-05-19 20:57 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20090519142656T.fujita.tomonori@lab.ntt.co.jp>


On May 19, 2009, at 12:27 AM, FUJITA Tomonori wrote:

> CC'ed linux-kernel
>
> On Thu, 14 May 2009 17:42:28 -0500
> Becky Bruce <beckyb@kernel.crashing.org> wrote:

>
>
>> This patch includes the basic infrastructure to use swiotlb
>> bounce buffering on 32-bit powerpc.  It is not yet enabled on
>> any platforms.  Probably the most interesting bit is the
>> addition of addr_needs_map to dma_ops - we need this as
>> a dma_op because the decision of whether or not an addr
>> can be mapped by a device is device-specific.
>>
>> Signed-off-by: Becky Bruce <beckyb@kernel.crashing.org>

<snip>

>> +/*
>> + * Determine if an address needs bounce buffering via swiotlb.
>> + * Going forward I expect the swiotlb code to generalize on using
>> + * a dma_ops->addr_needs_map, and this function will move from  
>> here to the
>> + * generic swiotlb code.
>> + */
>> +int
>> +swiotlb_arch_address_needs_mapping(struct device *hwdev,  
>> dma_addr_t addr,
>> +				   size_t size)
>> +{
>> +	struct dma_mapping_ops *dma_ops = get_dma_ops(hwdev);
>> +
>> +	BUG_ON(!dma_ops);
>> +	return dma_ops->addr_needs_map(hwdev, addr, size);
>> +}
>> +
>> +/*
>> + * Determine if an address is reachable by a pci device, or if we  
>> must bounce.
>> + */
>> +static int
>> +swiotlb_pci_addr_needs_map(struct device *hwdev, dma_addr_t addr,  
>> size_t size)
>> +{
>> +	u64 mask = dma_get_mask(hwdev);
>> +	dma_addr_t max;
>> +	struct pci_controller *hose;
>> +	struct pci_dev *pdev = to_pci_dev(hwdev);
>> +
>> +	hose = pci_bus_to_host(pdev->bus);
>> +	max = hose->dma_window_base_cur + hose->dma_window_size;
>> +
>> +	/* check that we're within mapped pci window space */
>> +	if ((addr + size > max) | (addr < hose->dma_window_base_cur))
>> +		return 1;
>> +
>> +	return !is_buffer_dma_capable(mask, addr, size);
>> +}
>> +
>> +static int
>> +swiotlb_addr_needs_map(struct device *hwdev, dma_addr_t addr,  
>> size_t size)
>> +{
>> +	return !is_buffer_dma_capable(dma_get_mask(hwdev), addr, size);
>> +}
>
> I think that swiotlb_pci_addr_needs_map and swiotlb_addr_needs_map
> don't need swiotlb_arch_range_needs_mapping() since
>
> - swiotlb_arch_range_needs_mapping() is always used with
> swiotlb_arch_address_needs_mapping().

Yes.  I don't need range_needs_mapping() on ppc, so I let it default  
to the lib/swiotlb.c implementation, which just returns 0.  All the  
determination of whether an address needs mapping comes from  
swiotlb_arch_address_needs_mapping().

>
>
> - swiotlb_arch_address_needs_mapping() uses is_buffer_dma_capable()
> and powerpc doesn't overwrite swiotlb_arch_address_needs_mapping()

I *do* overwrite it.  But I still use is_buffer_dma_capable().  I just  
add some other checks to it in the pci case, because I need to know  
what the controller has mapped as it changes the point at which I must  
begin to bounce buffer.

>
>
>
> Do I miss something?

I don't think so.  But let me know if I've misunderstood you.

>
>
> Anyway, we need to fix swiotlb checking code to if an area is
> DMA-capable or not.
>
> swiotlb_arch_address_needs_mapping() calls is_buffer_dma_capable() in
> dma-mapping.h but it should not. It should live in an arch-specific
> place such as arch's dma-mapping.h or something since
> is_buffer_dma_capable() is arch-specific. I didn't know that
> is_buffer_dma_capable() is arch-specific when I added it to the
> generic place.

Errrr, is_buffer_dma_capable is generic right now, unless I'm missing  
something.  It's in include/linux/dma-mapping.h, unless I'm getting  
bitten by a slightly stale tree.

>
>
> If we have something like in arch/{x86|ia64|powerpc}/dma-mapping.h:
>
> static inline int is_buffer_dma_capable(struct device *dev,  
> dma_addr_t addr, size_t size)
>
> then we don't need two checking functions, address_needs_mapping and
> range_needs_mapping.

It's never been clear to me *why* we had both in the first place - if  
you can explain this, I'd be grateful :)

It actually looks like we want to remove the dma_op that I created for  
addr_needs_map because of the perf hit.... it gets called a ton of  
times, many times on addresses that don't actually require bounce  
buffering (and thus, are more sensitive to the minor performance  
hit).  We are looking instead at adding a per-device variable that is  
used to determine if we need to bounce (in addition to  
is_buffer_dma_capable) that would live inside archdata - see the other  
posts on this thread for details (we're still hashing out exactly how  
we want to do this).

Cheers,
Becky

^ permalink raw reply

* Re: mpc5200 fec error
From: Wolfram Sang @ 2009-05-19 20:57 UTC (permalink / raw)
  To: Robert Schwebel; +Cc: linuxppc-dev, Eric Millbrandt
In-Reply-To: <20090519203645.GW19843@pengutronix.de>

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Tue, May 19, 2009 at 10:36:45PM +0200, Robert Schwebel wrote:
> Wolfram, have you seen this mail? You recently tested -rt on 2.6.29,
> right? Did you only test that on the customer hardware or also on the
> phyCORE-MPC5200B?

So far, I tried only on customer hardware, and that was 2.6.29.2-rt11. With
that setup, I could work with NFS flawlessly, no real stress-testing though.
Testing latest .3-rt is on my todo, will check the phyCOREs, too.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] maps/mtd-ram: add an of-platform driver
From: Grant Likely @ 2009-05-19 20:36 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, linux-mtd, ben-linux
In-Reply-To: <20090519203014.GC27476@pengutronix.de>

On Tue, May 19, 2009 at 2:30 PM, Wolfram Sang <w.sang@pengutronix.de> wrote=
:
>> This name is too generic for an of_platform device. =A0'mtd' needs to be
>> in there somewhere.
>
> Will "of-mtd-ram" do? (I'd think so)

fine by me.

g

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: mpc5200 fec error
From: Robert Schwebel @ 2009-05-19 20:36 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Eric Millbrandt
In-Reply-To: <A88094362DE0AE49A118AB9B4EB3612403F9DEB6@dekaexchange.deka.local>

Wolfram, have you seen this mail? You recently tested -rt on 2.6.29,
right? Did you only test that on the customer hardware or also on the
phyCORE-MPC5200B?

rsc

On Mon, May 18, 2009 at 01:36:27PM -0400, Eric Millbrandt wrote:
> Hello all,
> 
>  
> 
> I am testing a 2.6.29.3 (with preempt_rt patches) kernel on a phytec pcm030 and
> am getting a kernel hang when testing the fec Ethernet controller.  The error
> only occurs when running the preempt-patched kernel, an unmodified kernel works
> fine.  Is anyone out there using preempt_rt on an MPC5200 successfully?
> 
>  
> 
> Eric
> 
>  
> 
> root@rudolph-ui:/root> iperf -c linux-5200bdevl01 -P 2 -i 1 -p 5001 -f k -t 600
> 
> ------------------------------------------------------------
> 
> Client connecting to linux-5200bdevl01, TCP port 5001
> 
> TCP window size: 36.2 KByte (default)
> 
> ------------------------------------------------------------
> 
> [  4] local 10.1.4.88 port 37872 connected with 10.1.5.234 port 5001
> 
> [  3] local 10.1.4.88 port 37871 connected with 10.1.5.234 port 5001
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  3]  0.0- 1.0 sec  3824 KBytes  31326 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  4]  0.0- 1.0 sec  3656 KBytes  29950 Kbits/sec
> 
> [SUM]  0.0- 1.0 sec  7480 KBytes  61276 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  4]  1.0- 2.0 sec  3760 KBytes  30802 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  3]  1.0- 2.0 sec  3752 KBytes  30736 Kbits/sec
> 
> [SUM]  1.0- 2.0 sec  7512 KBytes  61538 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  4]  2.0- 3.0 sec  3728 KBytes  30540 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  3]  2.0- 3.0 sec  3816 KBytes  31261 Kbits/sec
> 
> [SUM]  2.0- 3.0 sec  7544 KBytes  61800 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  3]  3.0- 4.0 sec  3712 KBytes  30409 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  4]  3.0- 4.0 sec  3824 KBytes  31326 Kbits/sec
> 
> [SUM]  3.0- 4.0 sec  7536 KBytes  61735 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  3]  4.0- 5.0 sec  3968 KBytes  32506 Kbits/sec
> 
> [ ID] Interval       Transfer     Bandwidth
> 
> [  4]  4.0- 5.0 sec  3624 KBytes  29688 Kbits/sec
> 
> [SUM]  4.0- 5.0 sec  7592 KBytes  62194 Kbits/sec
> 
> [ 5761.999175] net eth0: transmit queue overrun
> 
> [ 5762.003591] net eth0: transmit queue overrun
> 
> [ 5762.007948] net eth0: transmit queue overrun
> 
> [ 5762.012302] net eth0: transmit queue overrun
> 
> [ 5762.016658] net eth0: transmit queue overrun
> 
> [ 5762.021013] net eth0: transmit queue overrun
> 
> [ 5762.025381] net eth0: transmit queue overrun
> 
> [ 5762.029735] net eth0: transmit queue overrun
> 
> [ 5762.034090] net eth0: transmit queue overrun
> 
> [ 5762.038445] net eth0: transmit queue overrun
> 
> [ 5767.000928] net eth0: transmit queue overrun
> 
> [ 5767.005278] net eth0: transmit queue overrun
> 
> [ 5767.009634] net eth0: transmit queue overrun
> 
> [ 5767.013990] net eth0: transmit queue overrun
> 
> [ 5767.018345] net eth0: transmit queue overrun
> 
> [ 5767.022701] net eth0: transmit queue overrun
> 
> ?.
> 
> 
> 
> _____________________________________________________________________________
> 
> This e-mail and the information, including any attachments, it contains are
> intended to be a confidential communication only to the person or entity to
> whom it is addressed and may contain information that is privileged. If the
> reader of this message is not the intended recipient, you are hereby notified
> that any dissemination, distribution or copying of this communication is
> strictly prohibited. If you have received this communication in error, please
> immediately notify the sender and destroy the original message.
> 
> Thank you.        
> 
> 
> 
> Please consider the environment before printing this email.
> 

> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 2/2] maps/mtd-ram: add an of-platform driver
From: Wolfram Sang @ 2009-05-19 20:30 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, linux-mtd, ben-linux
In-Reply-To: <fa686aa40905190757w68b35333j6f0fa84ea253a486@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 6437 bytes --]

Hi Grant,

thanks a lot for the review!

On Tue, May 19, 2009 at 08:57:15AM -0600, Grant Likely wrote:
> On Wed, Jan 21, 2009 at 11:41 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> > Create an of-aware driver using the now exported generic functions from
> > plat-ram.c. A typical oftree snipplet for it looks like this:
> >
> > sram@04e00000 {
> >        compatible = "mtd-ram";
> >        reg = <0x04e00000 0x00200000>;
> >        bank-width = <2>;
> > };
> >
> > Partitions on this device are not yet supported.
> 
> This is a new device tree binding.  It must be documented in
> Documentation/powerpc/dts-bindings and posted to
> devicetree-discuss@ozlabs.org for review.

Yeah, noticed that myself. I wanted to start another try to mainline this
driver next week, then also asking devicetree-discuss; will do it happily this
week, too :)

> 
> > diff --git a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
> > index 6d9ba35..f8e3908 100644
> > --- a/drivers/mtd/maps/Makefile
> > +++ b/drivers/mtd/maps/Makefile
> > @@ -58,6 +58,7 @@ obj-$(CONFIG_MTD_WRSBC8260)   += wr_sbc82xx_flash.o
> >  obj-$(CONFIG_MTD_DMV182)       += dmv182.o
> >  obj-$(CONFIG_MTD_SHARP_SL)     += sharpsl-flash.o
> >  obj-$(CONFIG_MTD_PLATRAM)      += plat-ram.o
> > +obj-$(CONFIG_MTD_OFRAM)                += of-ram.o
> >  obj-$(CONFIG_MTD_OMAP_NOR)     += omap_nor.o
> >  obj-$(CONFIG_MTD_INTEL_VR_NOR) += intel_vr_nor.o
> >  obj-$(CONFIG_MTD_BFIN_ASYNC)   += bfin-async-flash.o
> > diff --git a/drivers/mtd/maps/of-ram.c b/drivers/mtd/maps/of-ram.c
> > new file mode 100644
> > index 0000000..4d334a9
> > --- /dev/null
> > +++ b/drivers/mtd/maps/of-ram.c
> > @@ -0,0 +1,120 @@
> > +/*
> > + * drivers/mtd/maps/of-ram.c
> > + *
> > + * Generic of device based RAM map
> > + *
> > + * Copyright (C) 2009 Wolfram Sang, Pengutronix
> > + *
> > + * Using plat-ram.c by Ben Dooks
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/mtd/plat-ram.h>
> > +
> > +static int of_ram_probe(struct of_device *op, const struct of_device_id *match)
> 
> __devinit

Oops, missed all of those.

> 
> > +{
> > +       struct platdata_mtd_ram *pdata;
> > +       struct resource *resource;
> > +       const char *name;
> > +       const u32 *bankwidth;
> > +       int ret = -ENOMEM;
> > +
> > +       pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> > +       if (!pdata)
> > +               goto bad0;
> > +       op->dev.platform_data = pdata;
> > +
> > +       name = of_get_property(op->node, "name", NULL);
> > +       if (!name) {
> > +               ret = -ENOENT;
> > +               dev_dbg(&op->dev, "could not get node name\n");
> > +               goto bad1;
> > +       }
> > +
> > +       resource = kzalloc(sizeof(struct resource), GFP_KERNEL);
> > +       if (!resource)
> > +               goto bad1;
> 
> Why isn't resource on the stack?

Point taken.

> 
> > +
> > +       if (of_address_to_resource(op->node, 0, resource)) {
> > +               ret = -ENXIO;
> > +               dev_dbg(&op->dev, "could not create resource\n");
> > +               goto bad2;
> > +       }
> > +
> > +       bankwidth = of_get_property(op->node, "bank-width", NULL);
> > +       if (*bankwidth == 0) {
> > +               ret = -ENOENT;
> > +               dev_dbg(&op->dev, "bank width not set\n");
> > +               goto bad2;
> > +       }
> > +       pdata->bankwidth = *bankwidth;
> > +
> > +       ret = __platram_probe(&op->dev, name, resource, pdata);
> > +       kfree(resource);
> > +
> > +       if (ret)
> > +               goto bad1;
> > +
> > +       return 0;
> > +
> > + bad2:
> > +       kfree(resource);
> > + bad1:
> > +       op->dev.platform_data = NULL;
> > +       kfree(pdata);
> > + bad0:
> > +       return ret;
> 
> I'd rather see more meaningful labels.  ie "err_kalloc:" and
> "err_platram_probe:".

Okay.

> 
> > +}
> > +
> > +static int of_ram_remove(struct of_device *op)
> 
> __devexit
> 
> > +{
> > +       int ret;
> > +       struct platdata_mtd_ram *pdata = op->dev.platform_data;
> > +
> > +       ret = __platram_remove(&op->dev);
> > +       op->dev.platform_data = NULL;
> > +       kfree(pdata);
> > +       return ret;
> > +}
> > +
> > +static struct of_device_id of_ram_match[] = {
> > +       { .compatible = "mtd-ram", },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(of, of_ram_match);
> > +
> > +static struct of_platform_driver of_ram_driver = {
> 
> __devinitdata
> 
> > +       .owner = THIS_MODULE,
> > +       .name = "of-ram",
> 
> This name is too generic for an of_platform device.  'mtd' needs to be
> in there somewhere.

Will "of-mtd-ram" do? (I'd think so)

> 
> > +       .match_table = of_ram_match,
> > +       .probe = of_ram_probe,
> > +       .remove = of_ram_remove,
> 
> __devexit_p()
> 
> > +};
> > +
> > +static int __init of_ram_init(void)
> > +{
> > +       return of_register_platform_driver(&of_ram_driver);
> > +}
> > +
> > +static void __exit of_ram_exit(void)
> > +{
> > +       of_unregister_platform_driver(&of_ram_driver);
> > +}
> > +
> > +module_init(of_ram_init);
> 
> Put the module_init() statement directly after the of_ram_init() definition.
> 
> > +module_exit(of_ram_exit);
> > +
> > +MODULE_AUTHOR("Wolfram Sang");
> > +MODULE_DESCRIPTION("MTD OF RAM map driver");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 1.5.6.5
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@ozlabs.org
> > https://ozlabs.org/mailman/listinfo/linuxppc-dev
> >
> 
> 
> 
> -- 
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.

Will send V2 after some tests tomorrow...

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] qe: add polling timeout to qe_issue_cmd()
From: Grant Likely @ 2009-05-19 20:10 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, smaclennan
In-Reply-To: <1242761199-17875-3-git-send-email-timur@freescale.com>

On Tue, May 19, 2009 at 1:26 PM, Timur Tabi <timur@freescale.com> wrote:
> The qe_issue_cmd() function (Freescale PowerPC QUICC Engine library) poll=
s on
> a register until a status bit changes, but does not include a timeout to
> handle the situation if the bit never changes. =A0Change the code to use =
the new
> spin_event_timeout() macro, which simplifies polling on a register withou=
t
> a timeout.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> This patch depends on my previous patch, "powerpc: introduce macro
> spin_event_timeout()".
>
> =A0arch/powerpc/sysdev/qe_lib/qe.c | =A0 =A09 ++++++---
> =A01 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib=
/qe.c
> index 01bce37..810e1df 100644
> --- a/arch/powerpc/sysdev/qe_lib/qe.c
> +++ b/arch/powerpc/sysdev/qe_lib/qe.c
> @@ -111,6 +111,7 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol=
, u32 cmd_input)
> =A0{
> =A0 =A0 =A0 =A0unsigned long flags;
> =A0 =A0 =A0 =A0u8 mcn_shift =3D 0, dev_shift =3D 0;
> + =A0 =A0 =A0 int ret;
>
> =A0 =A0 =A0 =A0spin_lock_irqsave(&qe_lock, flags);
> =A0 =A0 =A0 =A0if (cmd =3D=3D QE_RESET) {
> @@ -138,11 +139,13 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protoc=
ol, u32 cmd_input)
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0/* wait for the QE_CR_FLG to clear */
> - =A0 =A0 =A0 while(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 cpu_relax();
> + =A0 =A0 =A0 spin_event_timeout((in_be32(&qe_immr->cp.cecr) & QE_CR_FLG)=
 =3D=3D 0,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0100, 0, ret);
> + =A0 =A0 =A0 /* On timeout (e.g. failure), the expression will be false =
(ret =3D=3D 0),
> + =A0 =A0 =A0 =A0 =A0otherwise it will be true (ret =3D=3D 1). */
> =A0 =A0 =A0 =A0spin_unlock_irqrestore(&qe_lock, flags);
>
> - =A0 =A0 =A0 return 0;
> + =A0 =A0 =A0 return ret =3D=3D 1;

Hmmm, the ret value is backwards from what most coders would expect
(zero on success, non-zero on failure).  I'd personally recommend
reversing the polarity in the macro.

Otherwise, feel free to add my acked-by line to both patches.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc: Add support for swiotlb on 32-bit
From: Becky Bruce @ 2009-05-19 20:06 UTC (permalink / raw)
  To: Kumar Gala; +Cc: fujita.tomonori, linuxppc-dev
In-Reply-To: <13F18475-1D3E-4D9E-B10C-8BE48FBA9DF5@kernel.crashing.org>


On May 19, 2009, at 8:04 AM, Kumar Gala wrote:

>
> On May 18, 2009, at 4:49 PM, Benjamin Herrenschmidt wrote:
>
>> On Mon, 2009-05-18 at 08:25 -0500, Kumar Gala wrote:
>>>
>>> Part of this is how the generic swiotlb code works and part of it
>>> was
>>> our desire not to bloat dev archdata by adding such info that as you
>>> say is either bus specific or conveyed in the dma addr mask.
>>
>> Right but perf sucks :-)
>>
>> Maybe an option is to clamp the DMA mask when it's set by the  
>> driver to
>> limit it to the available inbound window ?
>
> Clamping the DMA mask is even worse than the additional indirection  
> for us.  We have valid scenarios in which we'd have 512M of outbound  
> PCI address space and 4G of mem and thus 3.5G of inbound PCI address  
> space.  With the DMA mask we'd be limited to 2G and bouncing from  
> 2..3.5G when we don't need to.

>
>
> I think our options are to change archdata as follows:
>
> Option 1 - just add a new data member to dev_archdata
>
> struct dev_archdata {
>        /* Optional pointer to an OF device node */
>        struct device_node      *of_node;
>
>        /* DMA operations on that device */
>        struct dma_mapping_ops  *dma_ops;
>        void 		        *dma_data;
> 	dma_addr_t		direct_dma_addr;
> };
>
> Option 2 - introduce a proper container for how we use dma_data.   
> This may just be moving the indirection from an indirection function  
> call to an indirection data reference:
>
> struct dma_data {
>        dma_addr_t      offset;
>        dma_addr_t      direct_dma_addr;
>        struct          iommu_table *iommu_table;
> };
>
> struct dev_archdata {
>        /* Optional pointer to an OF device node */
>        struct device_node      *of_node;
>
>        /* DMA operations on that device */
>        struct dma_mapping_ops  *dma_ops;
>        struct dma_data         *dma_data;
> };

Personally, I prefer this.  However, I understand Ben has some  
objection here.   At a minimum, I *do* need to change dma_data to be  
able to contain a 64-bit number on a 32-bit platform as part of the  
optimization for 64-bit PCI devices.   It should probably be a  
dma_addr_t for that, looking at how it's being used.  However, that is  
a bit of a philosophical break from the type of dma_data being generic  
void *.  I'm open to suggestions here.

>
>
> Option 3 - use dma_data to keep the addr at which we need to bounce  
> vs not for SWIOTLB - this has potential issues w/conflicting with  
> dma_data being used as the dma_offset. (need to think on that a bit  
> more).  Additionally this has the benefit in that we need dma_data  
> to be a 64-bit quantity on ppc32 w/>32-bit phys addr.
>
> struct dev_archdata {
>        /* Optional pointer to an OF device node */
>        struct device_node      *of_node;
>
>        /* DMA operations on that device */
>        struct dma_mapping_ops  *dma_ops;
>        dma_addr_t 	        dma_data;
> };
>

This won't work.  I need the dma offset that's currently stored there  
for 64-bit PCI devices.

It sounds like we want to do some combination of the above:

1) add a max_dma_addr field to archdata to indicate the boundary  
beyond which a device must bounce buffer (we currently don't have  
anything that requires a start/size type paradigm here - does anyone  
know of any cases that I don't know about here?)

2) change the type of dma_data so it can hold 64 bits on a 32-bit  
platform.

-Becky

^ permalink raw reply

* Re: Musings on PCI busses
From: Grant Likely @ 2009-05-19 20:02 UTC (permalink / raw)
  To: David Miller; +Cc: linuxppc-dev, thunderbird2k, John.Linn, arnd
In-Reply-To: <20090519.120514.26253637.davem@davemloft.net>

On Tue, May 19, 2009 at 1:05 PM, David Miller <davem@davemloft.net> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 19 May 2009 18:12:02 +0200
>
>> On Tuesday 19 May 2009, Grant Likely wrote:
>>> 1) Probe the host controller in an of_platform driver. =A0This has the
>>> advantage of simplicity. =A0The probe routine will get automatically
>>> called when the PCI host controller device tree node is registered
>>> with the of_platform bus. =A0The bus parenthood also gets reflected in
>>> the device model and sysfs. =A0The disadvantage is that it defers PCI
>>> bus probing until after the of_platform bus is probed (maybe this is
>>> okay; maybe this already happens anyway).
>>
>> This is also what we do on 64-bit systems (well, Cell at this time).
>> There are no fundamental problems with this, just some slightly
>> unexpected interactions e.g. when you have use a PCI serial port
>> as your console before probing PCI.
>
> This is also what sparc64 does :-)
>
> All of the individual sparc64 PCI controller types have a OF driver
> and then there is a common layer of OF PCI helper code to do most
> of the work.

Cool, well I have my solution then.  :-)

g.


--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* [PATCH 0/2] introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-05-19 19:26 UTC (permalink / raw)
  To: linuxppc-dev, benh, grant.likely, smaclennan


Introduce the spin_event_timeout() macro, and update the QE library to use it.

^ permalink raw reply

* [PATCH 2/2] qe: add polling timeout to qe_issue_cmd()
From: Timur Tabi @ 2009-05-19 19:26 UTC (permalink / raw)
  To: linuxppc-dev, benh, grant.likely, smaclennan
In-Reply-To: <1242761199-17875-2-git-send-email-timur@freescale.com>

The qe_issue_cmd() function (Freescale PowerPC QUICC Engine library) polls on
a register until a status bit changes, but does not include a timeout to
handle the situation if the bit never changes.  Change the code to use the new
spin_event_timeout() macro, which simplifies polling on a register without
a timeout.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch depends on my previous patch, "powerpc: introduce macro
spin_event_timeout()".

 arch/powerpc/sysdev/qe_lib/qe.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index 01bce37..810e1df 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -111,6 +111,7 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
 {
 	unsigned long flags;
 	u8 mcn_shift = 0, dev_shift = 0;
+	int ret;
 
 	spin_lock_irqsave(&qe_lock, flags);
 	if (cmd == QE_RESET) {
@@ -138,11 +139,13 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input)
 	}
 
 	/* wait for the QE_CR_FLG to clear */
-	while(in_be32(&qe_immr->cp.cecr) & QE_CR_FLG)
-		cpu_relax();
+	spin_event_timeout((in_be32(&qe_immr->cp.cecr) & QE_CR_FLG) == 0,
+			   100, 0, ret);
+	/* On timeout (e.g. failure), the expression will be false (ret == 0),
+	   otherwise it will be true (ret == 1). */
 	spin_unlock_irqrestore(&qe_lock, flags);
 
-	return 0;
+	return ret == 1;
 }
 EXPORT_SYMBOL(qe_issue_cmd);
 
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH 1/2 v8] powerpc: introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-05-19 19:26 UTC (permalink / raw)
  To: linuxppc-dev, benh, grant.likely, smaclennan
In-Reply-To: <1242761199-17875-1-git-send-email-timur@freescale.com>

The macro spin_event_timeout() takes a condition and timeout value
(in microseconds) as parameters.  It spins until either the condition is true
or the timeout expires.  It returns the result of the condition when the loop
was terminated.

This primary purpose of this macro is to poll on a hardware register until a
status bit changes.  The timeout ensures that the loop still terminates if the
bit doesn't change as expected.  This macro makes it easier for driver
developers to perform this kind of operation properly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

v8: added a copyright notice

 arch/powerpc/include/asm/delay.h |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..af4a270 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -2,8 +2,11 @@
 #define _ASM_POWERPC_DELAY_H
 #ifdef __KERNEL__
 
+#include <asm/time.h>
+
 /*
  * Copyright 1996, Paul Mackerras.
+ * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -30,5 +33,35 @@ extern void udelay(unsigned long usecs);
 #define mdelay(n)	udelay((n) * 1000)
 #endif
 
+/**
+ * spin_event_timeout - spin until a condition gets true or a timeout elapses
+ * @condition: a C expression to evalate
+ * @timeout: timeout, in microseconds
+ * @delay: the number of microseconds to delay between eache evaluation of
+ *         @condition
+ * @rc: the last value of the condition
+ *
+ * The process spins until the condition evaluates to true (non-zero) or the
+ * timeout elapses.  Upon exit, @rc contains the value of the condition. This
+ * allows you to test the condition without incurring any side effects.
+ *
+ * This primary purpose of this macro is to poll on a hardware register
+ * until a status bit changes.  The timeout ensures that the loop still
+ * terminates even if the bit never changes.  The delay is for devices that
+ * need a delay in between successive reads.
+ *
+ * gcc will optimize out the if-statement if @delay is a constant.
+ */
+#define spin_event_timeout(condition, timeout, delay, rc)                   \
+{                                                                           \
+	unsigned long __loops = tb_ticks_per_usec * timeout;                \
+	unsigned long __start = get_tbl();                                  \
+	while (!(rc = (condition)) && (tb_ticks_since(__start) <= __loops)) \
+		if (delay)                                                  \
+			udelay(delay);                                      \
+		else	                                                    \
+			cpu_relax();                                        \
+}
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DELAY_H */
-- 
1.6.0.6

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox