LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [00/15] swiotlb cleanup
From: Ian Campbell @ 2009-07-13  9:40 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: jeremy@goop.org, tony.luck@intel.com, linux-ia64@vger.kernel.org,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, joerg.roedel@amd.com, mingo@elte.hu
In-Reply-To: <20090713131859N.fujita.tomonori@lab.ntt.co.jp>

On Mon, 2009-07-13 at 05:20 +0100, FUJITA Tomonori wrote:
> On Fri, 10 Jul 2009 15:02:00 +0100
> Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Fri, 2009-07-10 at 14:35 +0900, FUJITA Tomonori wrote:
> > > I don't think that we need to take account of dom0 support; we don't
> > > have a clear idea about an acceptable dom0 design (it needs to use
> > > swiotlb code? I don't know yet), we don't even know we will have dom0
> > > support in mainline. That's why I didn't CC this patchset to Xen
> > > camp.
> > 
> > The core domain 0 patches which were the subject of the discussions a
> > few week back are completely orthogonal to the swiotlb side of things
> 
> ? If we don't merge dom0 patch, we don't need dom0 changes to
> swiotlb. We don't know we would have dom0 support in mainline. Or I
> overlooked something?
[...]
> As far as I know, you have not posted anything about changes to
> swiotlb for domU. I can't discuss it. If you want, please send
> patches.

There are no separate domU swiotlb patches -- the exact the same patches
as we have already been discussing are useful and necessary for both
domU and dom0.

Ian.

^ permalink raw reply

* Re: [00/15] swiotlb cleanup
From: FUJITA Tomonori @ 2009-07-13  9:53 UTC (permalink / raw)
  To: Ian.Campbell
  Cc: jeremy, tony.luck, linux-ia64, x86, linux-kernel, fujita.tomonori,
	linuxppc-dev, joerg.roedel, mingo
In-Reply-To: <1247478029.11668.10.camel@zakaz.uk.xensource.com>

On Mon, 13 Jul 2009 10:40:29 +0100
Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:

> On Mon, 2009-07-13 at 05:20 +0100, FUJITA Tomonori wrote:
> > On Fri, 10 Jul 2009 15:02:00 +0100
> > Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > 
> > > On Fri, 2009-07-10 at 14:35 +0900, FUJITA Tomonori wrote:
> > > > I don't think that we need to take account of dom0 support; we don't
> > > > have a clear idea about an acceptable dom0 design (it needs to use
> > > > swiotlb code? I don't know yet), we don't even know we will have dom0
> > > > support in mainline. That's why I didn't CC this patchset to Xen
> > > > camp.
> > > 
> > > The core domain 0 patches which were the subject of the discussions a
> > > few week back are completely orthogonal to the swiotlb side of things
> > 
> > ? If we don't merge dom0 patch, we don't need dom0 changes to
> > swiotlb. We don't know we would have dom0 support in mainline. Or I
> > overlooked something?
> [...]
> > As far as I know, you have not posted anything about changes to
> > swiotlb for domU. I can't discuss it. If you want, please send
> > patches.
> 
> There are no separate domU swiotlb patches -- the exact the same patches
> as we have already been discussing are useful and necessary for both
> domU and dom0.

Hmm, you guys introduced the swiotlb hooks by saying that it's for
only dom0.

=
http://lkml.org/lkml/2008/12/16/351

Hi Ingo,

Here's some patches to clean up swiotlb to prepare for some Xen dom0
patches.  These have been posted before, but undergone a round of
cleanups to address various comments.

=

I don't see any comments like 'this is useful to dom0 too'. I'm still
not sure what exactly part is useful to domU.

^ permalink raw reply

* Re: [00/15] swiotlb cleanup
From: Ian Campbell @ 2009-07-13 10:05 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: jeremy, tony.luck, linux-ia64, x86, linux-kernel, linuxppc-dev,
	joerg.roedel, mingo
In-Reply-To: <20090713185247W.fujita.tomonori@lab.ntt.co.jp>

On Mon, 2009-07-13 at 18:53 +0900, FUJITA Tomonori wrote:
> On Mon, 13 Jul 2009 10:40:29 +0100
> Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> 
> > On Mon, 2009-07-13 at 05:20 +0100, FUJITA Tomonori wrote:
> > > On Fri, 10 Jul 2009 15:02:00 +0100
> > > Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > > 
> > > > On Fri, 2009-07-10 at 14:35 +0900, FUJITA Tomonori wrote:
> > > > > I don't think that we need to take account of dom0 support; we don't
> > > > > have a clear idea about an acceptable dom0 design (it needs to use
> > > > > swiotlb code? I don't know yet), we don't even know we will have dom0
> > > > > support in mainline. That's why I didn't CC this patchset to Xen
> > > > > camp.
> > > > 
> > > > The core domain 0 patches which were the subject of the discussions a
> > > > few week back are completely orthogonal to the swiotlb side of things
> > > 
> > > ? If we don't merge dom0 patch, we don't need dom0 changes to
> > > swiotlb. We don't know we would have dom0 support in mainline. Or I
> > > overlooked something?
> > [...]
> > > As far as I know, you have not posted anything about changes to
> > > swiotlb for domU. I can't discuss it. If you want, please send
> > > patches.
> > 
> > There are no separate domU swiotlb patches -- the exact the same patches
> > as we have already been discussing are useful and necessary for both
> > domU and dom0.
> 
> Hmm, you guys introduced the swiotlb hooks by saying that it's for
> only dom0.

That was just sloppy wording on our part. domain 0 is the major usecase
today so there is a tendency to think in those terms but the changes are
actually relevant to any domain with access to a physical device that
can do DMA, this includes domU via PCI passthrough.

> I don't see any comments like 'this is useful to dom0 too'. I'm still
                                                      ^U?
> not sure what exactly part is useful to domU.

All of it...

Ian.

^ permalink raw reply

* Re: mmap() problem in own driver
From: Arnd Bergmann @ 2009-07-13 14:30 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sauce.Cheng
In-Reply-To: <24457031.post@talk.nabble.com>

On Monday 13 July 2009, Sauce.Cheng wrote:

> I want to get data from kernel space to user space indirectly using mmap()
> but i dont know how I can do , anyone can give me some advices ?
> 
> firstly, fetch data by DMA to a memory allocated by "kmalloc"
> then i want to mmap it to user space and save the data as a file 

You first need to allocate full pages, e.g. using alloc_pages() instead
of kmalloc, which may not be aligned. To get a streaming mapping on
that memory, use dma_map_single(), which returns a DMA address you 
can pass to the device.
For user space access, the easiest is to have a character device,
which uses an mmap() file operation that calls remap_pfn_range()
on the page_to_pfn(pages).

	Arnd <><

^ permalink raw reply

* SPI driver on mpc5121
From: Canella Matteo @ 2009-07-13 14:33 UTC (permalink / raw)
  To: linuxppc-dev@ozlabs.org

Hi,
I'm on a mpc5121 platform, kernel 2.6.24.6, and I'm using the mpc512x_psc_s=
pi driver to communicate =A0with a touch screen controller.
I'm sending commands and reading data from the controller using 24-bit pack=
et (3 bytes).
The speed of the SPI is set to 640 KHz.
I thought that I would need 8/640K =3D 12,5 us each byte I send, but this i=
s not true.
I'm verifying the time needed by mpc512x_psc_spi_transfer_rxtx function in =
order to send one byte toggling a digital output when it sets the TX/RX fla=
gs (after it copies data in the transmission buffer) and toggling the outpu=
t again when the empty buffer interrupt is triggered.
I see that the actual 12,5us data transfer is preceded by a 20us delay and =
followed by another 15,6us delay. So the transfer is done in almost 50 us.

Is this a normal behavior? Does the PSC controller need this overhead in th=
e transfer time?
DSCKI and DTL delays are set to zero.

Another doubt I have:

In mpc512x_psc_spi_transfer_rxtx, Mode Register 2 of the PSC is set to zero=
. This is done (as manual says) by access to MR1 (by a read) and then write=
 a zero on the same address.:

in_8(&psc->mode);
mdelay(1);
out_8(&psc->mode, 0x0);
mdelay(1);

I don't understand why a 1ms delay is needed. The manual doesn't say anythi=
ng about this.
I tried to comment out these delays and anyway it seems to work correctly.

Best regards,
Matteo Canella

^ permalink raw reply

* Re: Soft Reset for PPC44x Virtex 5 hangs saying Restarting System
From: Grant Likely @ 2009-07-13 15:02 UTC (permalink / raw)
  To: srikanth krishnakar; +Cc: Linuxppc-dev
In-Reply-To: <6213bc560907130016n2f2d5066t3dc88032200da8ef@mail.gmail.com>

On Mon, Jul 13, 2009 at 1:16 AM, srikanth
krishnakar<skrishnakar@gmail.com> wrote:
> Hi all,
>
> Kernel : Linux-2.6.29
> Arch: Powerpc (ppc44x)
> Target: Xilinx ML507 Virtex5
>
> I have an issue in "Reset System" of Xilinx ML507 target board. I am using
> Compact Flash to boot the target ( using system ACE file to boot the
> target), during the process reset or reboot command on the target, I am not
> able to reboot the target completely, here is the snapshot:

Where is your boot code located?  In BRAM?  or SDRAM?  If it is in
RAM, then it is likely that your boot code gets overwritten when the
Linux kernel boots and so soft resetting the processor will result in
a hung system (because it doesn't have any boot code to run).

> -----------------------------------------------------------------------------------------------------
>
> The target again doesn't provide me the boot options as obtained when done
> hard reset :

What boot options are you referring to?  SystemACE boot configuration?
 The current system ace driver doesn't have any support for either
setting the boot options or using the systemace to reboot the system
by reconfiguring the FPGA.  It shouldn't be hard to do, it just hasn't
been written.

g.

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

^ permalink raw reply

* [PATCH 0/2] Setting GPIOs simultaneously
From: Anton Vorontsov @ 2009-07-13 15:19 UTC (permalink / raw)
  To: David Brownell; +Cc: linuxppc-dev, linux-kernel

Hi all,

I've been sitting on these patches for some time, but now it appears
that the set_sync() feature is needed elsewhere. So here are the
patches.

Joakim, I think this is what you need.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* [PATCH 1/2] gpiolib: Implement gpio_set_values_sync()
From: Anton Vorontsov @ 2009-07-13 15:19 UTC (permalink / raw)
  To: David Brownell; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20090713151911.GA28114@oksana.dev.rtsoft.ru>

Sometimes it's necessary to set GPIO values synchronously, e.g.
when CPU's GPIOs go to a FPGA and the FPGA has no latch-enable line,
so intermediate values on GPIO lines may trigger unnecessary actions
by the FPGA.

Another usage is chip-select muxes, e.g. using X GPIOs as address lines
to mux 2^X chip-selects, and setting some amount of GPIOs at once may
be a huge win.

Note that it's not always possible to set up GPIOs synchronously, so
gpio_can_set_values_sync() call is implemented. Currently it checks
for two things:

1. Whether gpio_chip has .set_sync() callback;
2. Whether all passed GPIOs belong to one particular gpio_chip.

Someday we may want to implement chip-specific .can_set_sync() check,
but currently this isn't needed.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/gpio/gpiolib.c     |   57 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    8 ++++++
 include/linux/gpio.h       |   14 ++++++++++
 3 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 51a8d41..cf714ad 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1067,6 +1067,53 @@ void __gpio_set_value(unsigned gpio, int value)
 EXPORT_SYMBOL_GPL(__gpio_set_value);
 
 /**
+ * __gpio_can_set_values_sync() - check if gpios can be set synchronously
+ * @num: amount of gpios
+ * @gpios: gpios in question
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_can_set_values_sync().
+ * It returns nonzero if an array of gpios can be set simultaneously.
+ */
+int __gpio_can_set_values_sync(unsigned num, unsigned *gpios)
+{
+	struct gpio_chip *chip;
+	unsigned i;
+
+	chip = gpio_to_chip(gpios[0]);
+	if (!chip->set_sync)
+		return 0;
+
+	for (i = 1; i < num; i++) {
+		if (chip != gpio_to_chip(gpios[i]))
+			return 0;
+	}
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(__gpio_can_set_values_sync);
+
+/**
+ * __gpio_set_values_sync() - assign gpios' values synchronously
+ * @num: amount of gpios
+ * @gpios: gpios whose value will be assigned
+ * @values: value to assign
+ * Context: any
+ *
+ * This is used directly or indirectly to implement gpio_set_values_sync().
+ * It invokes the associated gpio_chip.set_sync() method.
+ */
+void __gpio_set_values_sync(unsigned num, unsigned *gpios, int *values)
+{
+	struct gpio_chip *chip;
+
+	chip = gpio_to_chip(gpios[0]);
+	WARN_ON(extra_checks && chip->can_sleep);
+	chip->set_sync(chip, num, gpios, values);
+}
+EXPORT_SYMBOL_GPL(__gpio_set_values_sync);
+
+/**
  * __gpio_cansleep() - report whether gpio value access will sleep
  * @gpio: gpio in question
  * Context: any
@@ -1129,6 +1176,16 @@ void gpio_set_value_cansleep(unsigned gpio, int value)
 }
 EXPORT_SYMBOL_GPL(gpio_set_value_cansleep);
 
+void gpio_set_values_sync_cansleep(unsigned num, unsigned *gpios, int *values)
+{
+	struct gpio_chip	*chip;
+
+	might_sleep_if(extra_checks);
+	chip = gpio_to_chip(gpios[0]);
+	chip->set_sync(chip, num, gpios, values);
+}
+EXPORT_SYMBOL_GPL(gpio_set_values_sync_cansleep);
+
 
 #ifdef CONFIG_DEBUG_FS
 
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6c379d..c181623 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -88,6 +88,10 @@ struct gpio_chip {
 						unsigned offset, int value);
 	void			(*set)(struct gpio_chip *chip,
 						unsigned offset, int value);
+	/* NOTE: _sync() version accepts gpios, not offsets. */
+	void			(*set_sync)(struct gpio_chip *chip,
+						unsigned num, unsigned *gpios,
+						int *values);
 
 	int			(*to_irq)(struct gpio_chip *chip,
 						unsigned offset);
@@ -121,6 +125,8 @@ extern int gpio_direction_output(unsigned gpio, int value);
 
 extern int gpio_get_value_cansleep(unsigned gpio);
 extern void gpio_set_value_cansleep(unsigned gpio, int value);
+extern void gpio_set_values_sync_cansleep(unsigned num, unsigned *gpios,
+					  int *values);
 
 
 /* A platform's <asm/gpio.h> code may want to inline the I/O calls when
@@ -129,6 +135,8 @@ extern void gpio_set_value_cansleep(unsigned gpio, int value);
  */
 extern int __gpio_get_value(unsigned gpio);
 extern void __gpio_set_value(unsigned gpio, int value);
+extern int __gpio_can_set_values_sync(unsigned num, unsigned *gpios);
+extern void __gpio_set_values_sync(unsigned num, unsigned *gpios, int *values);
 
 extern int __gpio_cansleep(unsigned gpio);
 
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index e10c49a..ff62c1e 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -62,6 +62,20 @@ static inline void gpio_set_value(unsigned gpio, int value)
 	WARN_ON(1);
 }
 
+static inline int gpio_can_set_values_sync(unsigned num, unsigned *gpios)
+{
+	/* GPIO can never have been requested or set as output */
+	WARN_ON(1);
+	return 0;
+}
+
+static inline void gpio_set_values_sync(unsigned num, unsigned *gpios,
+					int *values)
+{
+	/* GPIO can never have been requested or set as output */
+	WARN_ON(1);
+}
+
 static inline int gpio_cansleep(unsigned gpio)
 {
 	/* GPIO can never have been requested or set as {in,out}put */
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH 2/2] powerpc/qe: Implement set_sync() callback for QE GPIOs
From: Anton Vorontsov @ 2009-07-13 15:19 UTC (permalink / raw)
  To: David Brownell; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20090713151911.GA28114@oksana.dev.rtsoft.ru>

This is needed to set GPIO's values synchronously.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/include/asm/gpio.h   |   11 +++++++++++
 arch/powerpc/sysdev/qe_lib/gpio.c |   27 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/gpio.h b/arch/powerpc/include/asm/gpio.h
index ea04632..899f365 100644
--- a/arch/powerpc/include/asm/gpio.h
+++ b/arch/powerpc/include/asm/gpio.h
@@ -33,6 +33,17 @@ static inline void gpio_set_value(unsigned int gpio, int value)
 	__gpio_set_value(gpio, value);
 }
 
+static inline int gpio_can_set_values_sync(unsigned num, unsigned *gpios)
+{
+	return __gpio_can_set_values_sync(num, gpios);
+}
+
+static inline void gpio_set_values_sync(unsigned num, unsigned *gpios,
+					int *values)
+{
+	__gpio_set_values_sync(num, gpios, values);
+}
+
 static inline int gpio_cansleep(unsigned int gpio)
 {
 	return __gpio_cansleep(gpio);
diff --git a/arch/powerpc/sysdev/qe_lib/gpio.c b/arch/powerpc/sysdev/qe_lib/gpio.c
index 3485288..6cfe784 100644
--- a/arch/powerpc/sysdev/qe_lib/gpio.c
+++ b/arch/powerpc/sysdev/qe_lib/gpio.c
@@ -84,6 +84,32 @@ static void qe_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	spin_unlock_irqrestore(&qe_gc->lock, flags);
 }
 
+static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
+			     unsigned int *gpios, int *vals)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct qe_gpio_chip *qe_gc = to_qe_gpio_chip(mm_gc);
+	struct qe_pio_regs __iomem *regs = mm_gc->regs;
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&qe_gc->lock, flags);
+
+	for (i = 0; i < num; i++) {
+		unsigned int gpio = gpios[i] - gc->base;
+		u32 pin_mask = 1 << (QE_PIO_PINS - 1 - gpio);
+
+		if (vals[i])
+			qe_gc->cpdata |= pin_mask;
+		else
+			qe_gc->cpdata &= ~pin_mask;
+	}
+
+	out_be32(&regs->cpdata, qe_gc->cpdata);
+
+	spin_unlock_irqrestore(&qe_gc->lock, flags);
+}
+
 static int qe_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
@@ -328,6 +354,7 @@ static int __init qe_add_gpiochips(void)
 		gc->direction_output = qe_gpio_dir_out;
 		gc->get = qe_gpio_get;
 		gc->set = qe_gpio_set;
+		gc->set_sync = qe_gpio_set_sync;
 
 		ret = of_mm_gpiochip_add(np, mm_gc);
 		if (ret)
-- 
1.6.3.3

^ permalink raw reply related

* Re: Soft Reset for PPC44x Virtex 5 hangs saying Restarting System
From: srikanth krishnakar @ 2009-07-13 15:39 UTC (permalink / raw)
  To: Grant Likely; +Cc: Linuxppc-dev
In-Reply-To: <fa686aa40907130802h3a6875f7gf21d41974974a7e4@mail.gmail.com>

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

On Mon, Jul 13, 2009 at 8:32 PM, Grant Likely <grant.likely@secretlab.ca>wrote:

> On Mon, Jul 13, 2009 at 1:16 AM, srikanth
> krishnakar<skrishnakar@gmail.com> wrote:
> > Hi all,
> >
> > Kernel : Linux-2.6.29
> > Arch: Powerpc (ppc44x)
> > Target: Xilinx ML507 Virtex5
> >
> > I have an issue in "Reset System" of Xilinx ML507 target board. I am
> using
> > Compact Flash to boot the target ( using system ACE file to boot the
> > target), during the process reset or reboot command on the target, I am
> not
> > able to reboot the target completely, here is the snapshot:
>
> Where is your boot code located?  In BRAM?  or SDRAM?


It is located in BRAM.


>  If it is in
> RAM, then it is likely that your boot code gets overwritten when the
> Linux kernel boots and so soft resetting the processor will result in
> a hung system (because it doesn't have any boot code to run).
>
> >
> -----------------------------------------------------------------------------------------------------
> >
> > The target again doesn't provide me the boot options as obtained when
> done
> > hard reset :
>
> What boot options are you referring to?


Boot options: console=ttyS0,9600 ip=bootp root=/dev/nfs rw


>  SystemACE boot configuration?


Yes. I have created SystemACE file using XMD to boot the target, I place
system.ace file in compact flash with rootfs NFS,

I see current ppc4xx_reset_system has been set to : DBCR_RST_SYSTEM  so that
is system reset but leaves the FPGA still programmed, I tried writing
DBCR_RST_CORE to DBCR0 that leads to segmentation fault as ;

void ppc4xx_reset_system(char *cmd)
{
        mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) | DBCR0_RST_SYSTEM);  << Tried
using DBCR0_RST_CORE
        while (1)
                ;       /* Just in case the reset doesn't work */
}

--------------------------------------------------------------

root@xilinx-ml507:~# reboot

INIT: Sending processes the TERM signal
INIT: Stopping OpenBSD Secure Shell server: sshdstopped /usr/sbin/sshd (pid
105)
.
Stopping Vixie-cron.
Stopping network benchmark server: netserverstopped /usr/sbin/netserver (pid
10)
.
Stopping syslogd/klogd: stopped syslogd (pid 1060)
stopped klogd (pid 1062)
done
NOT deconfiguring network interfaces: / is an NFS mount
Sending all processes the TERM signal...
Sending all processes the KILL signal...
hwclock: can't open '/dev/misc/rtc': No such file or directory
Unmounting remote filesystems...
Deactivating swap...
/etc/rc6.d/S40umountfs: line 9: swapoff: not found
Unmounting local filesystems...
umount2: Device or resource busy
umount: none busy - remounted read-only
Rebooting... Restarting system.
Oops: Exception in kernel mode, sig: 11 [#1]
PREEMPT LTT NESTING LEVEL : 0
Xilinx Virtex440
Modules linked in: nls_iso8859_1 ipv6
NIP: fffffffc LR: c000d14c CTR: c0018b64
REGS: cf373d50 TRAP: 0700   Not tainted  (2.6.29.6)
MSR: 00000000 <>  CR: 22444428  XER: 20000001
TASK = cf9acbf0[1101] 'reboot' THREAD: cf372000
GPR00: 50000000 cf373e00 cf9acbf0 00000000 000018ef ffffffff c026493c
00004000
GPR08: c04a5c8c c04a0000 00003fff 000018ef 22444422 1001a5a0 00008000
ffffffff
GPR16: 00000000 00000000 10000000 00000004 00000001 00000000 00000000
00000001
GPR24: 01230000 00000001 00000000 00000000 00000000 01234567 28121969
00000000
NIP [fffffffc] 0xfffffffc
LR [c000d14c] machine_restart+0x34/0x48
Call Trace:
[cf373e00] [c000d130] machine_restart+0x18/0x48 (unreliable)
[cf373e10] [c0046530] kernel_restart+0x34/0x5c
[cf373e20] [c0046684] sys_reboot+0x124/0x1a4
[cf373f40] [c000e164] ret_from_syscall+0x0/0x3c
Instruction dump:
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
---[ end trace 51d087d0d6d3d0e5 ]---
Segmentation fault
----------------------------------------------------------------------------------

How can I reboot the system, while resetting the FPGA core completely ?


> The current system ace driver doesn't have any support for either
> setting the boot options or using the systemace to reboot the system
> by reconfiguring the FPGA.  It shouldn't be hard to do, it just hasn't
> been written.
>
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>

Thanks for you patience !

-Srikanth


-- 
"The Good You Do, The Best You GET"

Regards
Srikanth Krishnakar
**********************

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

^ permalink raw reply

* Re: [PATCH 0/2] Setting GPIOs simultaneously
From: Joakim Tjernlund @ 2009-07-13 16:01 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev, David Brownell, linux-kernel
In-Reply-To: <20090713151911.GA28114@oksana.dev.rtsoft.ru>


Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
>
> Hi all,
>
> I've been sitting on these patches for some time, but now it appears
> that the set_sync() feature is needed elsewhere. So here are the
> patches.
>
> Joakim, I think this is what you need.

Yes, it sure looks so :) I will have to look closer later as
I will be traveling the next few days.

Question though, have you considered using a bitmask instead of
an array:
static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
	 		      unsigned int gpio_mask, unsigned int vals)
If you want to set bit 0, 3 and 8 you would set positions 0, 3 and 8 in gpio_mask
to ones. Similarly in vals, set bit positions 0, 3 and 8 to requested value.

While being at it, the reason for me needing this is that the spi_mpc83xx driver
was recently converted to a OF only driver so I have no way of defining my own
CS function anymore. While OF is good I don't feel that OF drivers should block the native
method, OF should be a layer on top of the native methods.

 Jocke

^ permalink raw reply

* Re: Soft Reset for PPC44x Virtex 5 hangs saying Restarting System
From: Grant Likely @ 2009-07-13 16:01 UTC (permalink / raw)
  To: srikanth krishnakar; +Cc: Linuxppc-dev
In-Reply-To: <6213bc560907130839t36e28d5fr34191ccb290c9cf8@mail.gmail.com>

On Mon, Jul 13, 2009 at 9:39 AM, srikanth
krishnakar<skrishnakar@gmail.com> wrote:
>
>
> On Mon, Jul 13, 2009 at 8:32 PM, Grant Likely <grant.likely@secretlab.ca>
> wrote:
>>
>> On Mon, Jul 13, 2009 at 1:16 AM, srikanth
>> krishnakar<skrishnakar@gmail.com> wrote:
>> > Hi all,
>> >
>> > Kernel : Linux-2.6.29
>> > Arch: Powerpc (ppc44x)
>> > Target: Xilinx ML507 Virtex5
>> >
>> > I have an issue in "Reset System" of Xilinx ML507 target board. I am
>> > using
>> > Compact Flash to boot the target ( using system ACE file to boot the
>> > target), during the process reset or reboot command on the target, I a=
m
>> > not
>> > able to reboot the target completely, here is the snapshot:
>>
>> Where is your boot code located? =A0In BRAM? =A0or SDRAM?
>
> It is located in BRAM.
>

Then most likely the process of booting modifies the initial data in
BRAM such that it will not be able to reboot the system.

> How can I reboot the system, while resetting the FPGA core completely ?

To reset and reconfigure the FPGA, you need to modify the systemace
driver to provide a system reset routine.  See the systemace user
manual for details on how to do this.

g.

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

^ permalink raw reply

* Re: Soft Reset for PPC44x Virtex 5 hangs saying Restarting System
From: srikanth krishnakar @ 2009-07-13 16:07 UTC (permalink / raw)
  To: Grant Likely; +Cc: Linuxppc-dev
In-Reply-To: <fa686aa40907130901r181fea87k3f8da151a50edb75@mail.gmail.com>

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

On Mon, Jul 13, 2009 at 9:31 PM, Grant Likely <grant.likely@secretlab.ca>wrote:

> On Mon, Jul 13, 2009 at 9:39 AM, srikanth
> krishnakar<skrishnakar@gmail.com> wrote:
> >
> >
> > On Mon, Jul 13, 2009 at 8:32 PM, Grant Likely <grant.likely@secretlab.ca
> >
> > wrote:
> >>
> >> On Mon, Jul 13, 2009 at 1:16 AM, srikanth
> >> krishnakar<skrishnakar@gmail.com> wrote:
> >> > Hi all,
> >> >
> >> > Kernel : Linux-2.6.29
> >> > Arch: Powerpc (ppc44x)
> >> > Target: Xilinx ML507 Virtex5
> >> >
> >> > I have an issue in "Reset System" of Xilinx ML507 target board. I am
> >> > using
> >> > Compact Flash to boot the target ( using system ACE file to boot the
> >> > target), during the process reset or reboot command on the target, I
> am
> >> > not
> >> > able to reboot the target completely, here is the snapshot:
> >>
> >> Where is your boot code located?  In BRAM?  or SDRAM?
> >
> > It is located in BRAM.
> >
>
> Then most likely the process of booting modifies the initial data in
> BRAM such that it will not be able to reboot the system.
>
> > How can I reboot the system, while resetting the FPGA core completely ?
>
> To reset and reconfigure the FPGA, you need to modify the systemace
> driver to provide a system reset routine.  See the systemace user
> manual for details on how to do this.
>
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>

Thank You very much.

Will look into it.

Regards
Srikanth Krishnakar
**********************

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

^ permalink raw reply

* Re: [PATCH 0/2] Setting GPIOs simultaneously
From: Joakim Tjernlund @ 2009-07-13 17:17 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev, David Brownell, linux-kernel
In-Reply-To: <OFF188389D.68E638A2-ONC12575F2.00568C5B-C12575F2.0057FC95@LocalDomain>

Joakim Tjernlund/Transmode wrote on 13/07/2009 18:01:02:
>
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
> >
> > Hi all,
> >
> > I've been sitting on these patches for some time, but now it appears
> > that the set_sync() feature is needed elsewhere. So here are the
> > patches.
> >
> > Joakim, I think this is what you need.

> Yes, it sure looks so :) I will have to look closer later as
> I will be traveling the next few days.

hmm, one must define a new OF syntax (and update the spi_mpc83xx driver) to
use this(I think). Have you given this any thought?

 Jocke

^ permalink raw reply

* Re: [PATCH 0/2] Setting GPIOs simultaneously
From: Anton Vorontsov @ 2009-07-13 17:34 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, David Brownell, linux-kernel
In-Reply-To: <OFF188389D.68E638A2-ONC12575F2.00568C5B-C12575F2.0057FC95@transmode.se>

On Mon, Jul 13, 2009 at 06:01:02PM +0200, Joakim Tjernlund wrote:
> 
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
> >
> > Hi all,
> >
> > I've been sitting on these patches for some time, but now it appears
> > that the set_sync() feature is needed elsewhere. So here are the
> > patches.
> >
> > Joakim, I think this is what you need.
> 
> Yes, it sure looks so :) I will have to look closer later as
> I will be traveling the next few days.
> 
> Question though, have you considered using a bitmask instead of
> an array:
> static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
> 	 		      unsigned int gpio_mask, unsigned int vals)
> If you want to set bit 0, 3 and 8 you would set positions 0, 3 and 8 in gpio_mask
> to ones. Similarly in vals, set bit positions 0, 3 and 8 to requested value.

Yeah, I thought about it. We could do the u64 masks (to handle up
to 64 bits parallel IO buses).

It's all easy with dumb memory-mapped GPIO controllers, because
we have a 8/16/32/64 bits registers with linear bit<->gpio mapping.

But some gpio controllers aren't that easy. I know at least one
(FPGA-based) gpio controller that won't change any GPIO lines
for real unless changes are "commited". The controller has several
banks (registers) of PIOs (total count > 64 bits), but you can commit
all the changes to the banks at once (synchronously). This isn't
because the controller is uber-cool, it's just the controller has
sequential IO. So with masks approach you won't able to use _sync()
calls that easily for all GPIOs range.

But OK, if we throw away the special cases, I can't imagine any
clear api for this approach, all I can think of is something
along these lines:

int num = 3;
u32 gpios[3];
u64 shifts[3];

/* this implies checks whether we can use _sync() */
if (!gpio_get_shifts(num, gpios, shifts))
	return -EINVAL;

gpio_set_values_sync(chip, 1 << shifts[0] | 1 << shifts[1],
		     val0 << shifts[0] | val1 << shifts[1]).

We can implement it, if that's acceptable. But that's a bit
ugly, I think.

> While being at it, the reason for me needing this is that the spi_mpc83xx driver
> was recently converted to a OF only driver so I have no way of defining my own
> CS function anymore. While OF is good I don't feel that OF drivers should block the native
> method, OF should be a layer on top of the native methods.

Um, I don't get it. You have a mux, which is a sort of GPIO controller.
All you need to do is to write "of-gpio-mux" driver, that will get all
the needed gpios from the underlaying GPIO controller.

In the device tree it'll look like this:

muxed_gpio: gpio-controller {
	#gpio-cells = <2>;
	compatible = "board-gpio-mux", "generic-gpio-mux";
	gpios = <&qe_pio_d  2 0   /* AD0 */
		 &qe_pio_d 17 0   /* AD1 */
		 &qe_pio_d  5 0>; /* AD2 */
	gpio-controller;
};

spi-controller {
	gpios = <&muxed_gpio 0 0
		 &muxed_gpio 1 0
		 &muxed_gpio 2 0
		 &muxed_gpio 3 0
		 &muxed_gpio 4 0
		 &muxed_gpio 5 0
		 &muxed_gpio 6 0
		 &muxed_gpio 7 0>;

	spi-device@0 {
		reg = <0>;
	};
	...
	spi-device@7 {
		reg = <0>;
	};
};

So you don't have to modify the spi driver.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: DMA on AMCC ppc440epx
From: Mikhail Zolotaryov @ 2009-07-13 18:44 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <406A31B117F2734987636D6CCC93EE3C0252933D@ehost011-3.exch011.intermedia.net>

Hi Leonid,

probably, the following article on PPC DMA could help:
http://lebon.org.ua/?p=5


Best regards,
Mikhail Zolotaryov

^ permalink raw reply

* vmalloc issue on mpc8378
From: Robert Benea @ 2009-07-13 19:00 UTC (permalink / raw)
  To: linuxppc-dev

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

I have an mpc8378 system with 512MB ram and 256MB flash (NOR). When I start the system I get the infamous message vmalloc failure. If I do cat /proc/meminfo I can see that I have only 212MB of VMALLOC space. 

I tried the advanced options and change the KERNEL_START to 0xb0000000 (instead of 0xc0000000), however after doing that the system became unstable with crashes left and right. 

The current work around is to set the command line mem=400M, however I like to use the whole RAM. What are the possible culprits that made my system unstable when making the KERNEL address space larger ?. Can it be because of PCIEs devices, I see that the PCIE memory is mapped to 0xa000000/0xc0000000.

Regards,
R.




      

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

^ permalink raw reply

* Re: Delay on intialization ide subsystem(most likely)
From: Andrey Gusev @ 2009-07-13 19:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-ide, petkovbb, David Miller, Bartlomiej Zolnierkiewicz,
	linuxppc-dev
In-Reply-To: <1247015562.6066.59.camel@pasglop>

On Wed, 08 Jul 2009 11:12:42 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Wed, 2009-07-08 at 01:18 +0400, Andrey Gusev wrote:
> 
> > I tried this drive on ide1 and ide2, there are same issue. This
> > drive worked on P-III before (as separate on channel, with another
> > hard drive and with cdrom) and I didn't have any problem with it.
> > There are chunks of dmesg.
> 
> Wow... It also fails with MWDMA2 ! That's just plain weird. It fails
> as master as well as slave too. Hrm. I wonder if it could be your
> cable... Do you have other machines (non-powermac) you can try that
> drive again just in case it toast itself in some way too ?

It couldn't be cable, because all tests were on different cables. Unfortunately,
I don't have another machine. I try find one, but it wouldn't be fast. This drive
used early and I didn't have any problems. It was unused some time and may be something 
happened with it.
Note, it was slave only with another hard disk, in other cases it was master (single on
cable). 

> 
> Bart, I don't suppose we know of any problem with those drive models ?
> (Q. FireballP LM20.5)
> 
> Cheers,
> Ben.
> 
> 

^ permalink raw reply

* Re: [PATCH 0/2] Setting GPIOs simultaneously
From: Joakim Tjernlund @ 2009-07-13 19:59 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev, David Brownell, linux-kernel
In-Reply-To: <20090713173455.GA9866@oksana.dev.rtsoft.ru>

Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 19:34:55:
>
> On Mon, Jul 13, 2009 at 06:01:02PM +0200, Joakim Tjernlund wrote:
> >
> > Anton Vorontsov <avorontsov@ru.mvista.com> wrote on 13/07/2009 17:19:11:
> > >
> > > Hi all,
> > >
> > > I've been sitting on these patches for some time, but now it appears
> > > that the set_sync() feature is needed elsewhere. So here are the
> > > patches.
> > >
> > > Joakim, I think this is what you need.
> >
> > Yes, it sure looks so :) I will have to look closer later as
> > I will be traveling the next few days.
> >
> > Question though, have you considered using a bitmask instead of
> > an array:
> > static void qe_gpio_set_sync(struct gpio_chip *gc, unsigned int num,
> >                 unsigned int gpio_mask, unsigned int vals)
> > If you want to set bit 0, 3 and 8 you would set positions 0, 3 and 8 in gpio_mask
> > to ones. Similarly in vals, set bit positions 0, 3 and 8 to requested value.
>
> Yeah, I thought about it. We could do the u64 masks (to handle up
> to 64 bits parallel IO buses).
>
> It's all easy with dumb memory-mapped GPIO controllers, because
> we have a 8/16/32/64 bits registers with linear bit<->gpio mapping.

Yes, this is gpio ..

>
> But some gpio controllers aren't that easy. I know at least one
> (FPGA-based) gpio controller that won't change any GPIO lines
> for real unless changes are "commited". The controller has several
> banks (registers) of PIOs (total count > 64 bits), but you can commit
> all the changes to the banks at once (synchronously). This isn't
> because the controller is uber-cool, it's just the controller has
> sequential IO. So with masks approach you won't able to use _sync()
> calls that easily for all GPIOs range.

.. but this one I am not sure qualify as gpio. Feels more like
its own device that should have its own driver.

>
> But OK, if we throw away the special cases, I can't imagine any
> clear api for this approach, all I can think of is something
> along these lines:
>
> int num = 3;
> u32 gpios[3];
> u64 shifts[3];
>
> /* this implies checks whether we can use _sync() */
> if (!gpio_get_shifts(num, gpios, shifts))
>    return -EINVAL;
>
> gpio_set_values_sync(chip, 1 << shifts[0] | 1 << shifts[1],
>            val0 << shifts[0] | val1 << shifts[1]).
>
> We can implement it, if that's acceptable. But that's a bit
> ugly, I think.

I see, that doesn't look good that either.

>
> > While being at it, the reason for me needing this is that the spi_mpc83xx driver
> > was recently converted to a OF only driver so I have no way of defining my own
> > CS function anymore. While OF is good I don't feel that OF drivers should
> block the native
> > method, OF should be a layer on top of the native methods.
>
> Um, I don't get it. You have a mux, which is a sort of GPIO controller.
> All you need to do is to write "of-gpio-mux" driver, that will get all
> the needed gpios from the underlaying GPIO controller.

Well, I already have a mux controller that is using the native spi methods. I
don't want to write a new one, far more complicated than what I got now.
While the OF system is very powerful and flexible I still think that
one should be able to use native SPI methods too. Why can they not
co-exist?

>
> In the device tree it'll look like this:

I must admit that I just started to look at the GPIO subsystem, but
I do wonder if mpc83xx_spi_cs_control() can do this muxing
without any change.

Very good example BTW.

      Jocke

^ permalink raw reply

* Re: removing addr_needs_map in struct dma_mapping_ops
From: Becky Bruce @ 2009-07-13 21:50 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20090710104706I.fujita.tomonori@lab.ntt.co.jp>


On Jul 9, 2009, at 8:47 PM, FUJITA Tomonori wrote:

> I'm trying to convert POWERPC to use asm-generic/dma-mapping-common.h.
>
> POWERPC needs addr_needs_map() in struct dma_mapping_ops for SWIOTLB
> support but I want to avoid add addr_needs_map() in struct
> dma_map_ops. IIRC, you guys think it as a temporary solution and

That is correct - I was planning to update when the iotlb folks had  
decided on a final scheme for this.  We don't like the additional  
indirection we're getting with the current implementation.

>
> talked about defining something like struct dma_data. Then we could
>
> struct dev_archdata {
>      ...
>
>      struct dma_data *ddata;
> };
>
> or
>
> struct dev_archdata {
>      ...
>
>      struct dma_data ddata;
> };
>
>
> struct dma_data needs dma_direct_offset, iommu_table, dma_base, and
> dma_window_size, anything else?

IIRC, what we had talked about was simpler - we talked about changing  
the current dev_archdata from this:

struct dev_archdata {
        struct device_node      *of_node;
        struct dma_mapping_ops  *dma_ops;
        void                    *dma_data;
};

to this:

struct dev_archdata {
	struct device_node *of_node;
	struct dma_mapping_ops *dma_ops;
	unsigned long long dma_data;
#ifdef CONFIG_SWIOTLB
	dma_addr_t max_direct_dma_addr;
#endif
};

Where max_direct_dma_addr is the address beyond which a specific  
device must use swiotlb, and dma_data is the offset like it is now  
(but wider on 32-bit systems than void *). I believe Ben had mentioned  
wanting to make the max_direct_dma_addr part conditional so we don't  
bloat archdata on platforms that don't ever bounce.

The change to the type of dma_data is actually in preparation for an  
optimization I have planned for 64-bit PCI devices (and which probably  
requires more discussion), so that doesn't need to happen now -  just  
leave it as a void *, and I can post a followup patch.

Let me know if I can help or do any testing - I've been meaning to  
look into switching to dma_map_ops for a while now but it hasn't  
managed to pop off my todo stack.

Cheers,
Becky

^ permalink raw reply

* Re: [PATCH 0/2] Setting GPIOs simultaneously
From: Anton Vorontsov @ 2009-07-13 22:20 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: linuxppc-dev, David Brownell, linux-kernel
In-Reply-To: <OF9378E65D.FBBCCFD5-ONC12575F2.006B28D5-C12575F2.006DDAE5@transmode.se>

On Mon, Jul 13, 2009 at 09:59:54PM +0200, Joakim Tjernlund wrote:
[...]
> > > While being at it, the reason for me needing this is that the spi_mpc83xx driver
> > > was recently converted to a OF only driver so I have no way of defining my own
> > > CS function anymore. While OF is good I don't feel that OF drivers should
> > block the native
> > > method, OF should be a layer on top of the native methods.
> >
> > Um, I don't get it. You have a mux, which is a sort of GPIO controller.
> > All you need to do is to write "of-gpio-mux" driver, that will get all
> > the needed gpios from the underlaying GPIO controller.
> 
> Well, I already have a mux controller that is using the native spi methods. I
> don't want to write a new one, far more complicated than what I got now.
> While the OF system is very powerful and flexible I still think that
> one should be able to use native SPI methods too. Why can they not
> co-exist?

I strongly believe that they can. You just need to post patches
so that we could see them, and maybe discuss how to do things
more generic. But if making things generic will be too hard (see
below), I don't think anybody will object applying a non-generic
solution (even permitting "legacy" bindings in the spi_8xxx driver).

But any users of the legacy bindings should be in-tree.

> > In the device tree it'll look like this:
> 
> I must admit that I just started to look at the GPIO subsystem, but
> I do wonder if mpc83xx_spi_cs_control() can do this muxing
> without any change.
> 
> Very good example BTW.

Well, there is one caveat: the "GPIOs" aren't independent,
so thinking about it more, I believe we should now discuss
"chip-select framework", not gpio controllers (it could be
that using gpio controllers for this purpose wasn't that
good idea).

http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg34738.html
^^^ I'm dreaming about this framework. I.e. true addressing
    for chip-selects. :-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: removing addr_needs_map in struct dma_mapping_ops
From: FUJITA Tomonori @ 2009-07-14  0:49 UTC (permalink / raw)
  To: beckyb; +Cc: fujita.tomonori, linuxppc-dev, linux-kernel
In-Reply-To: <B6C36B32-6861-4FD9-B8A3-E8C328687E0F@kernel.crashing.org>

On Mon, 13 Jul 2009 16:50:43 -0500
Becky Bruce <beckyb@kernel.crashing.org> wrote:

> > talked about defining something like struct dma_data. Then we could
> >
> > struct dev_archdata {
> >      ...
> >
> >      struct dma_data *ddata;
> > };
> >
> > or
> >
> > struct dev_archdata {
> >      ...
> >
> >      struct dma_data ddata;
> > };
> >
> >
> > struct dma_data needs dma_direct_offset, iommu_table, dma_base, and
> > dma_window_size, anything else?
> 
> IIRC, what we had talked about was simpler - we talked about changing  
> the current dev_archdata from this:
> 
> struct dev_archdata {
>         struct device_node      *of_node;
>         struct dma_mapping_ops  *dma_ops;
>         void                    *dma_data;
> };
> 
> to this:
> 
> struct dev_archdata {
> 	struct device_node *of_node;
> 	struct dma_mapping_ops *dma_ops;
> 	unsigned long long dma_data;
> #ifdef CONFIG_SWIOTLB
> 	dma_addr_t max_direct_dma_addr;
> #endif
> };
> 
> Where max_direct_dma_addr is the address beyond which a specific  
> device must use swiotlb, and dma_data is the offset like it is now  
> (but wider on 32-bit systems than void *). I believe Ben had mentioned  
> wanting to make the max_direct_dma_addr part conditional so we don't  
> bloat archdata on platforms that don't ever bounce.

Only maximum address is enough? The minimum (dma_window_base_cur in
swiotlb_pci_addr_needs_map) is not necessary?


> The change to the type of dma_data is actually in preparation for an  
> optimization I have planned for 64-bit PCI devices (and which probably  
> requires more discussion), so that doesn't need to happen now -  just  
> leave it as a void *, and I can post a followup patch.
> 
> Let me know if I can help or do any testing - I've been meaning to  
> look into switching to dma_map_ops for a while now but it hasn't  
> managed to pop off my todo stack.

Ok, how about this? I'm not familiar with POWERPC so I might
misunderstand something.


diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 7d2277c..0086f8d 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -16,6 +16,9 @@ struct dev_archdata {
 	/* DMA operations on that device */
 	struct dma_mapping_ops	*dma_ops;
 	void			*dma_data;
+#ifdef CONFIG_SWIOTLB
+	dma_addr_t		max_direct_dma_addr;
+#endif
 };
 
 static inline void dev_archdata_set_node(struct dev_archdata *ad,
diff --git a/arch/powerpc/include/asm/swiotlb.h b/arch/powerpc/include/asm/swiotlb.h
index 30891d6..b23a4f1 100644
--- a/arch/powerpc/include/asm/swiotlb.h
+++ b/arch/powerpc/include/asm/swiotlb.h
@@ -24,4 +24,6 @@ static inline void dma_mark_clean(void *addr, size_t size) {}
 extern unsigned int ppc_swiotlb_enable;
 int __init swiotlb_setup_bus_notifier(void);
 
+extern void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev);
+
 #endif /* __ASM_SWIOTLB_H */
diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
index 68ccf11..e21359e 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c
@@ -56,39 +56,16 @@ 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);
+	struct dev_archdata *sd = &hwdev->archdata;
 
 	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))
+	if (sd->max_direct_dma_addr && addr + size > sd->max_direct_dma_addr)
 		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);
 }
 
-
 /*
  * At the moment, all platforms that use this code only require
  * swiotlb to be used if we're operating on HIGHMEM.  Since
@@ -104,7 +81,6 @@ struct dma_mapping_ops swiotlb_dma_ops = {
 	.dma_supported = swiotlb_dma_supported,
 	.map_page = swiotlb_map_page,
 	.unmap_page = swiotlb_unmap_page,
-	.addr_needs_map = swiotlb_addr_needs_map,
 	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
 	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
@@ -119,13 +95,23 @@ struct dma_mapping_ops swiotlb_pci_dma_ops = {
 	.dma_supported = swiotlb_dma_supported,
 	.map_page = swiotlb_map_page,
 	.unmap_page = swiotlb_unmap_page,
-	.addr_needs_map = swiotlb_pci_addr_needs_map,
 	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
 	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
 	.sync_sg_for_device = swiotlb_sync_sg_for_device
 };
 
+void pci_dma_dev_setup_swiotlb(struct pci_dev *pdev)
+{
+	struct pci_controller *hose;
+	struct dev_archdata *sd;
+
+	hose = pci_bus_to_host(pdev->bus);
+	sd = &pdev->dev.archdata;
+	sd->max_direct_dma_addr =
+		hose->dma_window_base_cur + hose->dma_window_size;
+}
+
 static int ppc_swiotlb_bus_notify(struct notifier_block *nb,
 				  unsigned long action, void *data)
 {
diff --git a/arch/powerpc/platforms/85xx/mpc8536_ds.c b/arch/powerpc/platforms/85xx/mpc8536_ds.c
index 055ff41..401751b 100644
--- a/arch/powerpc/platforms/85xx/mpc8536_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc8536_ds.c
@@ -136,6 +136,7 @@ define_machine(mpc8536_ds) {
 	.init_IRQ		= mpc8536_ds_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 	.get_irq		= mpic_get_irq,
 	.restart		= fsl_rstcr_restart,
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 849c0ac..1ba8e38 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
@@ -277,6 +277,7 @@ define_machine(mpc8544_ds) {
 	.init_IRQ		= mpc85xx_ds_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 	.get_irq		= mpic_get_irq,
 	.restart		= fsl_rstcr_restart,
@@ -291,6 +292,7 @@ define_machine(mpc8572_ds) {
 	.init_IRQ		= mpc85xx_ds_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 	.get_irq		= mpic_get_irq,
 	.restart		= fsl_rstcr_restart,
@@ -305,6 +307,7 @@ define_machine(p2020_ds) {
 	.init_IRQ		= mpc85xx_ds_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 	.get_irq		= mpic_get_irq,
 	.restart		= fsl_rstcr_restart,
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 60ed9c0..165a2de 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -356,6 +356,7 @@ define_machine(mpc8568_mds) {
 	.progress	= udbg_progress,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 };
 
@@ -377,5 +378,6 @@ define_machine(mpc8569_mds) {
 	.progress	= udbg_progress,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 };
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index 6632702..d1878f3 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -187,5 +187,6 @@ define_machine(mpc86xx_hpcn) {
 	.progress		= udbg_progress,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
+	.pci_dma_dev_setup	= pci_dma_dev_setup_swiotlb,
 #endif
 };

^ permalink raw reply related

* Re: [PATCH 04/15] swiotlb: remove unnecessary swiotlb_bus_to_virt
From: Becky Bruce @ 2009-07-14  2:17 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: tony.luck, linux-ia64, x86, linux-kernel, linuxppc-dev
In-Reply-To: <1247187904-31999-5-git-send-email-fujita.tomonori@lab.ntt.co.jp>


On Jul 9, 2009, at 8:04 PM, FUJITA Tomonori wrote:

> swiotlb_bus_to_virt is unncessary; we can use swiotlb_bus_to_phys and
> phys_to_virt instead.

phys_to_virt (also, virt_to_phys) is invalid for highmem addresses on  
ppc.  In most of the uses in this file, it doesn't matter, as the  
iotlb buffers themselves are alloc'd out of lowmem, but the  
dma_mark_clean() calls could happen on a highmem addr.  Currently, on  
powerpc, dma_mark_clean() doesn't do anything, so it isn't a  
functional problem.  I'm fine with the bulk of this patch, because in  
reality, if I need to do something with a virtual address of a highmem  
page, I have to get a kmap for the page.  So the existing  
swiotlb_bus_to_virt isn't really helping.

What is dma_mark_clean used for?  Could it be made to take a paddr,  
and let the implementation deal with making sure there's a valid vaddr  
for the actual "clean" operation?

I'd also like to see a comment with swiotlb_virt_to_bus() that notes  
that it should only be called on addresses in lowmem.

Cheers,
Becky

>
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
> arch/powerpc/kernel/dma-swiotlb.c |   10 ----------
> lib/swiotlb.c                     |   34 +++++++++++++++ 
> +------------------
> 2 files changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/ 
> dma-swiotlb.c
> index 68ccf11..41534ae 100644
> --- a/arch/powerpc/kernel/dma-swiotlb.c
> +++ b/arch/powerpc/kernel/dma-swiotlb.c
> @@ -24,16 +24,6 @@
> int swiotlb __read_mostly;
> unsigned int ppc_swiotlb_enable;
>
> -void *swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t addr)
> -{
> -	unsigned long pfn = PFN_DOWN(swiotlb_bus_to_phys(hwdev, addr));
> -	void *pageaddr = page_address(pfn_to_page(pfn));
> -
> -	if (pageaddr != NULL)
> -		return pageaddr + (addr % PAGE_SIZE);
> -	return NULL;
> -}
> -
> dma_addr_t swiotlb_phys_to_bus(struct device *hwdev, phys_addr_t  
> paddr)
> {
> 	return paddr + get_dma_direct_offset(hwdev);
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index dc1cd1f..1a89c84 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -130,11 +130,6 @@ static dma_addr_t swiotlb_virt_to_bus(struct  
> device *hwdev,
> 	return swiotlb_phys_to_bus(hwdev, virt_to_phys(address));
> }
>
> -void * __weak swiotlb_bus_to_virt(struct device *hwdev, dma_addr_t  
> address)
> -{
> -	return phys_to_virt(swiotlb_bus_to_phys(hwdev, address));
> -}
> -
> int __weak swiotlb_arch_address_needs_mapping(struct device *hwdev,
> 					       dma_addr_t addr, size_t size)
> {
> @@ -307,9 +302,10 @@ address_needs_mapping(struct device *hwdev,  
> dma_addr_t addr, size_t size)
> 	return swiotlb_arch_address_needs_mapping(hwdev, addr, size);
> }
>
> -static int is_swiotlb_buffer(char *addr)
> +static int is_swiotlb_buffer(phys_addr_t paddr)
> {
> -	return addr >= io_tlb_start && addr < io_tlb_end;
> +	return paddr >= virt_to_phys(io_tlb_start) &&
> +		paddr < virt_to_phys(io_tlb_end);
> }
>
> /*
> @@ -582,11 +578,13 @@ EXPORT_SYMBOL(swiotlb_alloc_coherent);
>
> void
> swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> -		      dma_addr_t dma_handle)
> +		      dma_addr_t dev_addr)
> {
> +	phys_addr_t paddr = swiotlb_bus_to_phys(hwdev, dev_addr);
> +
> 	WARN_ON(irqs_disabled());
> -	if (!is_swiotlb_buffer(vaddr))
> -		free_pages((unsigned long) vaddr, get_order(size));
> +	if (!is_swiotlb_buffer(paddr))
> +		free_pages((unsigned long)vaddr, get_order(size));
> 	else
> 		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
> 		do_unmap_single(hwdev, vaddr, size, DMA_TO_DEVICE);
> @@ -671,19 +669,19 @@ EXPORT_SYMBOL_GPL(swiotlb_map_page);
> static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> 			 size_t size, int dir)
> {
> -	char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
> +	phys_addr_t paddr = swiotlb_bus_to_phys(hwdev, dev_addr);
>
> 	BUG_ON(dir == DMA_NONE);
>
> -	if (is_swiotlb_buffer(dma_addr)) {
> -		do_unmap_single(hwdev, dma_addr, size, dir);
> +	if (is_swiotlb_buffer(paddr)) {
> +		do_unmap_single(hwdev, phys_to_virt(paddr), size, dir);
> 		return;
> 	}
>
> 	if (dir != DMA_FROM_DEVICE)
> 		return;
>
> -	dma_mark_clean(dma_addr, size);
> +	dma_mark_clean(phys_to_virt(paddr), size);
> }
>
> void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> @@ -708,19 +706,19 @@ static void
> swiotlb_sync_single(struct device *hwdev, dma_addr_t dev_addr,
> 		    size_t size, int dir, int target)
> {
> -	char *dma_addr = swiotlb_bus_to_virt(hwdev, dev_addr);
> +	phys_addr_t paddr = swiotlb_bus_to_phys(hwdev, dev_addr);
>
> 	BUG_ON(dir == DMA_NONE);
>
> -	if (is_swiotlb_buffer(dma_addr)) {
> -		sync_single(hwdev, dma_addr, size, dir, target);
> +	if (is_swiotlb_buffer(paddr)) {
> +		sync_single(hwdev, phys_to_virt(paddr), size, dir, target);
> 		return;
> 	}
>
> 	if (dir != DMA_FROM_DEVICE)
> 		return;
>
> -	dma_mark_clean(dma_addr, size);
> +	dma_mark_clean(phys_to_virt(paddr), size);

>
> }
>
> void
> -- 
> 1.6.0.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux- 
> kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [00/15] swiotlb cleanup
From: Becky Bruce @ 2009-07-14  3:13 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Jeremy Fitzhardinge, tony.luck, linux-ia64, Ian Campbell, x86,
	linux-kernel, FUJITA Tomonori, linuxppc-dev, Joerg Roedel
In-Reply-To: <20090710051236.GA22218@elte.hu>


On Jul 10, 2009, at 12:12 AM, Ingo Molnar wrote:

>
> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
>
>> - removes unused (and unnecessary) hooks in swiotlb.
>>
>> - adds dma_capable() and converts swiotlb to use it. It can be used  
>> to
>> know if a memory area is dma capable or not. I added
>> is_buffer_dma_capable() for the same purpose long ago but it turned
>> out that the function doesn't work on POWERPC.
>>
>> This can be applied cleanly to linux-next, -mm, and mainline. This
>> patchset touches multiple architectures (ia64, powerpc, x86) so I
>> guess that -mm is appropriate for this patchset (I don't care much
>> what tree would merge this though).
>>
>> This is tested on x86 but only compile tested on POWERPC and IA64.
>>
>> Thanks,
>>
>> =
>> arch/ia64/include/asm/dma-mapping.h    |   18 ++++++
>> arch/powerpc/include/asm/dma-mapping.h |   23 +++++++
>> arch/powerpc/kernel/dma-swiotlb.c      |   48 +---------------
>> arch/x86/include/asm/dma-mapping.h     |   18 ++++++
>> arch/x86/kernel/pci-dma.c              |    2 +-
>> arch/x86/kernel/pci-gart_64.c          |    5 +-
>> arch/x86/kernel/pci-nommu.c            |    2 +-
>> arch/x86/kernel/pci-swiotlb.c          |   25 --------
>> include/linux/dma-mapping.h            |    5 --
>> include/linux/swiotlb.h                |   11 ----
>> lib/swiotlb.c                          |  102 ++++++++ 
>> +-----------------------
>> 11 files changed, 92 insertions(+), 167 deletions(-)
>
> Hm, the functions and facilities you remove here were added as part
> of preparatory patches for Xen guest support. You were aware of
> them, you were involved in discussions about those aspects with Ian
> and Jeremy but still you chose not to Cc: either of them and you
> failed to address that aspect in the changelogs.
>
> I'd like the Xen code to become cleaner more than anyone else here i
> guess, but patch submission methods like this are not really
> helpful. A far better method is to be open about such disagreements,
> to declare them, to Cc: everyone who disagrees, and to line out the
> arguments in the changelogs as well - instead of just curtly
> declaring those APIs 'unused' and failing to Cc: involved parties.
>
> Alas, on the technical level the cleanups themselves look mostly
> fine to me. Ian, Jeremy, the changes will alter Xen's use of
> swiotlb, but can the Xen side still live with these new methods - in
> particular is dma_capable() sufficient as a mechanism and can the
> Xen side filter out DMA allocations to make them physically
> continuous?
>
> Ben, Tony, Becky, any objections wrt. the PowerPC / IA64 impact? If
> everyone agrees i can apply them to the IOMMU tree, test it and push
> it out to -next, etc.
>

Ingo,

With the exception of the patch I commented on, I think these look OK  
from the powerpc point of view.  I've successfully booted one of my  
test platforms with the entire series applied and will run some more  
extensive (i.e. not "Whee!  A prompt!") tests tomorrow.

-Becky

^ permalink raw reply

* Re: [PATCH 04/15] swiotlb: remove unnecessary swiotlb_bus_to_virt
From: FUJITA Tomonori @ 2009-07-14  5:08 UTC (permalink / raw)
  To: beckyb
  Cc: tony.luck, linux-ia64, x86, linux-kernel, fujita.tomonori,
	linuxppc-dev
In-Reply-To: <5E909C02-3FA2-4BEB-8C73-F4E4A5A404BF@kernel.crashing.org>

On Mon, 13 Jul 2009 21:17:21 -0500
Becky Bruce <beckyb@kernel.crashing.org> wrote:

> 
> On Jul 9, 2009, at 8:04 PM, FUJITA Tomonori wrote:
> 
> > swiotlb_bus_to_virt is unncessary; we can use swiotlb_bus_to_phys and
> > phys_to_virt instead.
> 
> phys_to_virt (also, virt_to_phys) is invalid for highmem addresses on  
> ppc.  In most of the uses in this file, it doesn't matter, as the  
> iotlb buffers themselves are alloc'd out of lowmem,

Right,

> but the  
> dma_mark_clean() calls could happen on a highmem addr.  Currently, on  
> powerpc, dma_mark_clean() doesn't do anything, so it isn't a  
> functional problem.

Oops, I overlooked this. However, as you said, this is not a problem
with the current code.


>  I'm fine with the bulk of this patch, because in  
> reality, if I need to do something with a virtual address of a highmem  
> page, I have to get a kmap for the page.  So the existing  
> swiotlb_bus_to_virt isn't really helping.
> 
> What is dma_mark_clean used for?  Could it be made to take a paddr,  
> and let the implementation deal with making sure there's a valid vaddr  
> for the actual "clean" operation?

I think that it's IA64's optimization (it's a NULL function on X86
like POWERPC). It's defined in arch/ia64/mm/init.c. Looks like POWERPC
could use this optimization, I guess.

dma_mark_clean() just modifies the page flag (what it needs is struct
page) so I think that we can make it take a paddr.


> I'd also like to see a comment with swiotlb_virt_to_bus() that notes  
> that it should only be called on addresses in lowmem.

Ok, I'll do.


Thanks,

^ permalink raw reply


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