[Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Peter Crosthwaite]

This was left as NULL on the initial merge due to debate on the mailing list on
how to handle DMA contexts for sysbus devices. Patch
9e11908f12f92e31ea94dc2a4c962c836cba9f2a was later merged to fix OHCI. This is the,
equivalent fix for sysbus EHCI.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/usb/hcd-ehci-sysbus.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 1584079..803df92 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -45,6 +45,7 @@ static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
 
     s->capsbase = 0x100;
     s->opregbase = 0x140;
+    s->dma = &dma_context_memory;
 
     usb_ehci_initfn(s, DEVICE(dev));
     sysbus_init_irq(dev, &s->irq);
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[walimis]

On Thu, Nov 29, 2012 at 11:43:18AM +1000, Peter Crosthwaite wrote:
>This was left as NULL on the initial merge due to debate on the mailing list on
>how to handle DMA contexts for sysbus devices. Patch
>9e11908f12f92e31ea94dc2a4c962c836cba9f2a was later merged to fix OHCI. This is the,
>equivalent fix for sysbus EHCI.

I have also found this issue, but it's not the cause that xilinx
ehci can't work with usb-storage disk. Do you have any update
for xilinx ehci?

>
>Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Tested-by: Liming Wang <walimisdev@gmail.com>

Liming Wang

>---
> hw/usb/hcd-ehci-sysbus.c |    1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
>diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
>index 1584079..803df92 100644
>--- a/hw/usb/hcd-ehci-sysbus.c
>+++ b/hw/usb/hcd-ehci-sysbus.c
>@@ -45,6 +45,7 @@ static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
> 
>     s->capsbase = 0x100;
>     s->opregbase = 0x140;
>+    s->dma = &dma_context_memory;
> 
>     usb_ehci_initfn(s, DEVICE(dev));
>     sysbus_init_irq(dev, &s->irq);
>-- 
>1.7.0.4
>
>

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Peter Crosthwaite]

On Thu, Nov 29, 2012 at 12:00 PM, walimis <walimisdev@gmail.com> wrote:
> On Thu, Nov 29, 2012 at 11:43:18AM +1000, Peter Crosthwaite wrote:
>>This was left as NULL on the initial merge due to debate on the mailing list on
>>how to handle DMA contexts for sysbus devices. Patch
>>9e11908f12f92e31ea94dc2a4c962c836cba9f2a was later merged to fix OHCI. This is the,
>>equivalent fix for sysbus EHCI.
>
> I have also found this issue, but it's not the cause that xilinx
> ehci can't work with usb-storage disk. Do you have any update
> for xilinx ehci?
>

Hi Liming,

I haven't got around to looking into that one yet unfortunately. No
updates just yet - ill let you know if it resolves. It could very well
be a Linux bug as well so it needs to be investigated from both sides
of the fence.

>>
>>Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Tested-by: Liming Wang <walimisdev@gmail.com>
>

Thanks.

Regards,
Peter

> Liming Wang
>
>>---
>> hw/usb/hcd-ehci-sysbus.c |    1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>>diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
>>index 1584079..803df92 100644
>>--- a/hw/usb/hcd-ehci-sysbus.c
>>+++ b/hw/usb/hcd-ehci-sysbus.c
>>@@ -45,6 +45,7 @@ static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
>>
>>     s->capsbase = 0x100;
>>     s->opregbase = 0x140;
>>+    s->dma = &dma_context_memory;
>>
>>     usb_ehci_initfn(s, DEVICE(dev));
>>     sysbus_init_irq(dev, &s->irq);
>>--
>>1.7.0.4
>>
>>
>

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Gerd Hoffmann]

On 11/29/12 02:43, Peter Crosthwaite wrote:
> This was left as NULL on the initial merge due to debate on the mailing list on
> how to handle DMA contexts for sysbus devices. Patch
> 9e11908f12f92e31ea94dc2a4c962c836cba9f2a was later merged to fix OHCI. This is the,
> equivalent fix for sysbus EHCI.

Patch added to usb patch queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[walimis]

On Thu, Nov 29, 2012 at 12:05:14PM +1000, Peter Crosthwaite wrote:
>On Thu, Nov 29, 2012 at 12:00 PM, walimis <walimisdev@gmail.com> wrote:
>> On Thu, Nov 29, 2012 at 11:43:18AM +1000, Peter Crosthwaite wrote:
>>>This was left as NULL on the initial merge due to debate on the mailing list on
>>>how to handle DMA contexts for sysbus devices. Patch
>>>9e11908f12f92e31ea94dc2a4c962c836cba9f2a was later merged to fix OHCI. This is the,
>>>equivalent fix for sysbus EHCI.
>>
>> I have also found this issue, but it's not the cause that xilinx
>> ehci can't work with usb-storage disk. Do you have any update
>> for xilinx ehci?
>>
>
>Hi Liming,
>
>I haven't got around to looking into that one yet unfortunately. No
>updates just yet - ill let you know if it resolves. It could very well
>be a Linux bug as well so it needs to be investigated from both sides
>of the fence.

As said in another mail, I found that the root cause is that xilinx_zynq has
two EHCI controller. If we use usb-storage disk, the disk will be attached to
the second EHCI controller, which the kernel uses the first EHCI controller
by default.

For now, qemu doesn't support two EHCI controller, could we remove the second
EHCI from xilinx_zynq?

Liming Wang

>
>>>
>>>Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> Tested-by: Liming Wang <walimisdev@gmail.com>
>>
>
>Thanks.
>
>Regards,
>Peter
>
>> Liming Wang
>>
>>>---
>>> hw/usb/hcd-ehci-sysbus.c |    1 +
>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>>diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
>>>index 1584079..803df92 100644
>>>--- a/hw/usb/hcd-ehci-sysbus.c
>>>+++ b/hw/usb/hcd-ehci-sysbus.c
>>>@@ -45,6 +45,7 @@ static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
>>>
>>>     s->capsbase = 0x100;
>>>     s->opregbase = 0x140;
>>>+    s->dma = &dma_context_memory;
>>>
>>>     usb_ehci_initfn(s, DEVICE(dev));
>>>     sysbus_init_irq(dev, &s->irq);
>>>--
>>>1.7.0.4
>>>
>>>
>>

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Gerd Hoffmann]

  Hi,

> As said in another mail, I found that the root cause is that xilinx_zynq has
> two EHCI controller. If we use usb-storage disk, the disk will be attached to
> the second EHCI controller, which the kernel uses the first EHCI controller
> by default.

For the linux kernel it shouldn't matter where the usb stick is
connected.  Assuming it finds both ehci controllers.  Does it?

> For now, qemu doesn't support two EHCI controller, could we remove the second
> EHCI from xilinx_zynq?

Two controllers should work just fine.  I'd suggest to find the root
cause instead of doctoring like this.  ehci + usb core are fine with two
controllers & busses, maybe the arch plumbing (device tree?) misses
something so the linux kernel doesn't find the second ehci controller.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[walimis]

On Mon, Dec 03, 2012 at 01:51:00PM +0100, Gerd Hoffmann wrote:
>  Hi,
>
>> As said in another mail, I found that the root cause is that xilinx_zynq has
>> two EHCI controller. If we use usb-storage disk, the disk will be attached to
>> the second EHCI controller, which the kernel uses the first EHCI controller
>> by default.
>
>For the linux kernel it shouldn't matter where the usb stick is
>connected.  Assuming it finds both ehci controllers.  Does it?

The default device tree of linux kernel has only the first ehci controller
support, so the kernel can't detect the second controller. 
But the usb-storage disk is attached to the second controller, so that 
the disk is failed to be detected by the linux kernel.


>
>> For now, qemu doesn't support two EHCI controller, could we remove the second
>> EHCI from xilinx_zynq?
>
>Two controllers should work just fine.  I'd suggest to find the root

Yes, they work fine separately, but I don't know how to use them at the
same time (I mean both controller have device attached) as I have
mentioned in the another mail.

Liming Wang

>cause instead of doctoring like this.  ehci + usb core are fine with two
>controllers & busses, maybe the arch plumbing (device tree?) misses
>something so the linux kernel doesn't find the second ehci controller.
>
>cheers,
>  Gerd
>

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Peter Crosthwaite]

Hi Liming, Gerd,

On Tue, Dec 4, 2012 at 12:50 AM, walimis <walimisdev@gmail.com> wrote:
> On Mon, Dec 03, 2012 at 01:51:00PM +0100, Gerd Hoffmann wrote:
>>  Hi,
>>
>>> As said in another mail, I found that the root cause is that xilinx_zynq has
>>> two EHCI controller. If we use usb-storage disk, the disk will be attached to
>>> the second EHCI controller, which the kernel uses the first EHCI controller
>>> by default.
>>

I am using a device tree driven kernel with this:

		ps7_usb_0: ps7-usb@e0002000 {
			compatible = "xlnx,ps7-usb-1.00.a";
			dr_mode = "host";
			interrupt-parent = <&ps7_scugic_0>;
			interrupts = < 0 21 0 >;
			phy_type = "ulpi";
			reg = < 0xe0002000 0x1000 >;
			xlnx,usb-reset = <0xffffffff>;
		} ;
		ps7_usb_1: ps7-usb@e0003000 {
			compatible = "xlnx,ps7-usb-1.00.a";
			dr_mode = "host";
			interrupt-parent = <&ps7_scugic_0>;
			interrupts = < 0 44 0 >;
			phy_type = "ulpi";
			reg = < 0xe0003000 0x1000 >;
			xlnx,usb-reset = <0xffffffff>;
		} ;

And it now works for me, device successfully attaches to second controller:

	|  usb 2-1: New USB device found, idVendor=46f4, idProduct=0001
	|  usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
	|  usb 2-1: Product: QEMU USB HARDDRIVE
	|  usb 2-1: Manufacturer: QEMU
	|  usb 2-1: SerialNumber: 1-1

>>For the linux kernel it shouldn't matter where the usb stick is
>>connected.  Assuming it finds both ehci controllers.  Does it?
>

Gerd,

Is there any documentation out there on how to tell QEMU on command
line which EHCI you want your usb-storage to attach to?

> The default device tree of linux kernel has only the first ehci controller
> support, so the kernel can't detect the second controller.
> But the usb-storage disk is attached to the second controller, so that
> the disk is failed to be detected by the linux kernel.
>
>
>>
>>> For now, qemu doesn't support two EHCI controller, could we remove the second
>>> EHCI from xilinx_zynq?
>>

Prefer not. Just need a better kernel and DTB. This was definitely
broken for me recently, but I have pulled patches in my kernel, so I
think this has been fixed by the Xilinx kernel devels.

Regards,
Peter

>>Two controllers should work just fine.  I'd suggest to find the root
>
> Yes, they work fine separately, but I don't know how to use them at the
> same time (I mean both controller have device attached) as I have
> mentioned in the another mail.
>
> Liming Wang
>
>>cause instead of doctoring like this.  ehci + usb core are fine with two
>>controllers & busses, maybe the arch plumbing (device tree?) misses
>>something so the linux kernel doesn't find the second ehci controller.
>>
>>cheers,
>>  Gerd
>>
>

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[walimis]

On Tue, Dec 04, 2012 at 03:16:09PM +1000, Peter Crosthwaite wrote:
>Hi Liming, Gerd,
>
>On Tue, Dec 4, 2012 at 12:50 AM, walimis <walimisdev@gmail.com> wrote:
>> On Mon, Dec 03, 2012 at 01:51:00PM +0100, Gerd Hoffmann wrote:
>>>  Hi,
>>>
>>>> As said in another mail, I found that the root cause is that xilinx_zynq has
>>>> two EHCI controller. If we use usb-storage disk, the disk will be attached to
>>>> the second EHCI controller, which the kernel uses the first EHCI controller
>>>> by default.
>>>
>
>I am using a device tree driven kernel with this:
>
>		ps7_usb_0: ps7-usb@e0002000 {
>			compatible = "xlnx,ps7-usb-1.00.a";
>			dr_mode = "host";
>			interrupt-parent = <&ps7_scugic_0>;
>			interrupts = < 0 21 0 >;
>			phy_type = "ulpi";
>			reg = < 0xe0002000 0x1000 >;
>			xlnx,usb-reset = <0xffffffff>;
>		} ;
>		ps7_usb_1: ps7-usb@e0003000 {
>			compatible = "xlnx,ps7-usb-1.00.a";
>			dr_mode = "host";
>			interrupt-parent = <&ps7_scugic_0>;
>			interrupts = < 0 44 0 >;
>			phy_type = "ulpi";
>			reg = < 0xe0003000 0x1000 >;
>			xlnx,usb-reset = <0xffffffff>;
>		} ;
>
>And it now works for me, device successfully attaches to second controller:

OK, that's good.

>
>	|  usb 2-1: New USB device found, idVendor=46f4, idProduct=0001
>	|  usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>	|  usb 2-1: Product: QEMU USB HARDDRIVE
>	|  usb 2-1: Manufacturer: QEMU
>	|  usb 2-1: SerialNumber: 1-1
>
>>>For the linux kernel it shouldn't matter where the usb stick is
>>>connected.  Assuming it finds both ehci controllers.  Does it?
>>
>
>Gerd,
>
>Is there any documentation out there on how to tell QEMU on command
>line which EHCI you want your usb-storage to attach to?
>
>> The default device tree of linux kernel has only the first ehci controller
>> support, so the kernel can't detect the second controller.
>> But the usb-storage disk is attached to the second controller, so that
>> the disk is failed to be detected by the linux kernel.
>>
>>
>>>
>>>> For now, qemu doesn't support two EHCI controller, could we remove the second
>>>> EHCI from xilinx_zynq?
>>>
>
>Prefer not. Just need a better kernel and DTB. This was definitely
>broken for me recently, but I have pulled patches in my kernel, so I
>think this has been fixed by the Xilinx kernel devels.

OK, no problem.

Regards,
Liming Wang

>
>Regards,
>Peter
>
>>>Two controllers should work just fine.  I'd suggest to find the root
>>
>> Yes, they work fine separately, but I don't know how to use them at the
>> same time (I mean both controller have device attached) as I have
>> mentioned in the another mail.
>>
>> Liming Wang
>>
>>>cause instead of doctoring like this.  ehci + usb core are fine with two
>>>controllers & busses, maybe the arch plumbing (device tree?) misses
>>>something so the linux kernel doesn't find the second ehci controller.
>>>
>>>cheers,
>>>  Gerd
>>>
>>

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Gerd Hoffmann]

  Hi,

> Gerd,
> 
> Is there any documentation out there on how to tell QEMU on command
> line which EHCI you want your usb-storage to attach to?

docs/usb2.txt has some examples, although those are pc-centric where the
ehci is created on the command line.  The problem with zynx is that both
ehci controllers get the same (default) name so picking one by name
doesn't work.

Guess we haver to figure a way to explicitly name those devices (so the
busses are named too), especially in case a board has two identical
ones.  Maybe sysbus_create_simple needs an additional argument or a
sysbus_create_simple_with_id variant.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1.3] ehci-sysbus: Attach DMA context.

[Peter Crosthwaite]

Hi Gerd, Liming,

On Tue, Dec 4, 2012 at 6:15 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> Gerd,
>>
>> Is there any documentation out there on how to tell QEMU on command
>> line which EHCI you want your usb-storage to attach to?
>
> docs/usb2.txt has some examples, although those are pc-centric where the
> ehci is created on the command line.  The problem with zynx is that both
> ehci controllers get the same (default) name so picking one by name
> doesn't work.
>
> Guess we haver to figure a way to explicitly name those devices (so the
> busses are named too), especially in case a board has two identical
> ones.  Maybe sysbus_create_simple needs an additional argument or a
> sysbus_create_simple_with_id variant.
>

Ive taken a more manual approach and just used the manual
qdev_create() flow first to get up and running:

--- a/hw/xilinx_zynq.c
+++ b/hw/xilinx_zynq.c
@@ -162,12 +162,28 @@ static void zynq_init(QEMUMachineInitArgs *args)
         pic[n] = qdev_get_gpio_in(dev, n);
     }

+    dev = qdev_create(NULL, "xlnx.ps7-usb");
+    dev->id = "zynq-usb-0";
+    qdev_init_nofail(dev);
+    busdev = sysbus_from_qdev(dev);
+    sysbus_mmio_map(busdev, 0, 0xE0002000);
+    sysbus_connect_irq(busdev, 0, pic[53-IRQ_OFFSET]);
+
+    dev = qdev_create(NULL, "xlnx.ps7-usb");
+    dev->id = "zynq-usb-1";
+    busdev = sysbus_from_qdev(dev);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(busdev, 0, 0xE0003000);
+    sysbus_connect_irq(busdev, 0, pic[76-IRQ_OFFSET]);
+
     zynq_init_spi_flashes(0xE0006000, pic[58-IRQ_OFFSET], false);
     zynq_init_spi_flashes(0xE0007000, pic[81-IRQ_OFFSET], false);
     zynq_init_spi_flashes(0xE000D000, pic[51-IRQ_OFFSET], true);

+#if 0
     sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
     sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
+#endif

This works and gives the two USBs different names, and I can attach to
each on command lines using bus=zynq-usb-0.0 and bus=zynq-usb-0.1.

We can probably now think about a more palatable way to do this,
perhaps your sysbus_create_simple_id() approach.

Regards,
Peter

> cheers,
>   Gerd
>
>