[PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine Barshak]

The first patch adds USBHS device and the rest add PCI USB host support to Lager board.
The patches depend on the following commits:
USBHS and PCI USB host:
* http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
* http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
PCI USB host:
* http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
* http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
* http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1

Changes in V3:
* fixed USB1 and USB2 device names in the pinmux table.
* fixed a typo in the "Add internal PCI support" log message.

Changes in V2:
* capitalized ARM in the subject;
* rebased on top the latest devel tag;
* added pipe_type array to the usbhs platform info since it differs from the default.

Valentine Barshak (3):
  ARM: shmobile: lager: Add USBHS support
  ARM: shmobile: r8a7790: Add PCI USB host clock support
  ARM: shmobile: lager: Add internal PCI support

 arch/arm/mach-shmobile/board-lager.c   | 206 +++++++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/clock-r8a7790.c |   6 +-
 2 files changed, 211 insertions(+), 1 deletion(-)

-- 
1.8.3.1

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Ben Dooks]

On 24/01/14 22:28, Valentine Barshak wrote:
> The first patch adds USBHS device and the rest add PCI USB host support to Lager board.
> The patches depend on the following commits:
> USBHS and PCI USB host:
> * http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
> * http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
> PCI USB host:
> * http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
> * http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
> * http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1

Hi, when doing this have you come across a problem where the OHCI
controller on USB0 fails to start with an initialisation error? I
think it is something to do with the bridge setup but have yet to
have enough time to look into this issue.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine]

On 01/27/2014 01:59 PM, Ben Dooks wrote:
> On 24/01/14 22:28, Valentine Barshak wrote:
>> The first patch adds USBHS device and the rest add PCI USB host support to Lager board.
>> The patches depend on the following commits:
>> USBHS and PCI USB host:
>> * http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>> * http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>> PCI USB host:
>> * http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>> * http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>> * http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>
> Hi, when doing this have you come across a problem where the OHCI
> controller on USB0 fails to start with an initialisation error? I
> think it is something to do with the bridge setup but have yet to
> have enough time to look into this issue.
>

All PCI ports have worked fine for me.
Please, check that the SW5/SW6 pins are set correctly on your board.

Thanks,
Val.

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

Hi Valentine,

On Mon, Jan 27, 2014 at 7:02 PM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>
>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>
>>> The first patch adds USBHS device and the rest add PCI USB host support
>>> to Lager board.
>>> The patches depend on the following commits:
>>> USBHS and PCI USB host:
>>> *
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>> *
>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>> PCI USB host:
>>> *
>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>> *
>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>> *
>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>
>>
>> Hi, when doing this have you come across a problem where the OHCI
>> controller on USB0 fails to start with an initialisation error? I
>> think it is something to do with the bridge setup but have yet to
>> have enough time to look into this issue.
>>
>
> All PCI ports have worked fine for me.
> Please, check that the SW5/SW6 pins are set correctly on your board.

Thanks for your recommendation. From my side I must say that I
recommend against using USB0 for USB host unless you're sure about
your hardware settings. This to prevent driving VBUS incorrectly from
two hosts with the wrong cable. Normally micro type of connector is
used for USB Function, and on Lager the cable detection functionality
is missing. So due to missing cable detection implementation it is
probably much safer to just use USB0 for Function.

If I may jump to a slightly different topic, but still USB PCI, then
let me ask if USB PCI been tested with LPAE enabled? I am asking
because Lager has 4 GiB or RAM over 40 bits of physical address space,
but the PCI host controller probably only allows for up to 1 GiB
physical address space.

I suspect that this physical address space mismatch means that bounce
buffers will be needed for correct operation, but those seem to be
lacking from the upstream driver unless I'm mistaken. I believe USB
storage will be busted on Lager with LPAE enabled unless bounce
buffers are implemented. For a reference implementation, please grep
for "ixp4xx_needs_bounce".

Can you please check and let me know what you think?

Cheers,

/ magnus

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Ben Dooks]

On 27/01/14 10:19, Magnus Damm wrote:
> Hi Valentine,
>
> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
> <valentine.barshak@cogentembedded.com> wrote:
>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>
>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>
>>>> The first patch adds USBHS device and the rest add PCI USB host support
>>>> to Lager board.
>>>> The patches depend on the following commits:
>>>> USBHS and PCI USB host:
>>>> *
>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>> *
>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>> PCI USB host:
>>>> *
>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>> *
>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>> *
>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>
>>>
>>> Hi, when doing this have you come across a problem where the OHCI
>>> controller on USB0 fails to start with an initialisation error? I
>>> think it is something to do with the bridge setup but have yet to
>>> have enough time to look into this issue.
>>>
>>
>> All PCI ports have worked fine for me.
>> Please, check that the SW5/SW6 pins are set correctly on your board.
>
> Thanks for your recommendation. From my side I must say that I
> recommend against using USB0 for USB host unless you're sure about
> your hardware settings. This to prevent driving VBUS incorrectly from
> two hosts with the wrong cable. Normally micro type of connector is
> used for USB Function, and on Lager the cable detection functionality
> is missing. So due to missing cable detection implementation it is
> probably much safer to just use USB0 for Function.
>
> If I may jump to a slightly different topic, but still USB PCI, then
> let me ask if USB PCI been tested with LPAE enabled? I am asking
> because Lager has 4 GiB or RAM over 40 bits of physical address space,
> but the PCI host controller probably only allows for up to 1 GiB
> physical address space.
>
> I suspect that this physical address space mismatch means that bounce
> buffers will be needed for correct operation, but those seem to be
> lacking from the upstream driver unless I'm mistaken. I believe USB
> storage will be busted on Lager with LPAE enabled unless bounce
> buffers are implemented. For a reference implementation, please grep
> for "ixp4xx_needs_bounce".

The host bridge cannot cope with all the memory in the normal
address space, let alone the extended 40bit addressing. There
needs to be something done about (a) iommu support and (b) if
there is no iommu available limiting the DMA pool available for
the OHCI and EHCI drivers.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

Hi Ben,

On Mon, Jan 27, 2014 at 7:24 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 27/01/14 10:19, Magnus Damm wrote:
>>
>> Hi Valentine,
>>
>> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
>> <valentine.barshak@cogentembedded.com> wrote:
>>>
>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>
>>>>
>>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>>
>>>>>
>>>>> The first patch adds USBHS device and the rest add PCI USB host support
>>>>> to Lager board.
>>>>> The patches depend on the following commits:
>>>>> USBHS and PCI USB host:
>>>>> *
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>>> *
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>>> PCI USB host:
>>>>> *
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>>> *
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>>> *
>>>>>
>>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>>
>>>>
>>>>
>>>> Hi, when doing this have you come across a problem where the OHCI
>>>> controller on USB0 fails to start with an initialisation error? I
>>>> think it is something to do with the bridge setup but have yet to
>>>> have enough time to look into this issue.
>>>>
>>>
>>> All PCI ports have worked fine for me.
>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>
>>
>> Thanks for your recommendation. From my side I must say that I
>> recommend against using USB0 for USB host unless you're sure about
>> your hardware settings. This to prevent driving VBUS incorrectly from
>> two hosts with the wrong cable. Normally micro type of connector is
>> used for USB Function, and on Lager the cable detection functionality
>> is missing. So due to missing cable detection implementation it is
>> probably much safer to just use USB0 for Function.
>>
>> If I may jump to a slightly different topic, but still USB PCI, then
>> let me ask if USB PCI been tested with LPAE enabled? I am asking
>> because Lager has 4 GiB or RAM over 40 bits of physical address space,
>> but the PCI host controller probably only allows for up to 1 GiB
>> physical address space.
>>
>> I suspect that this physical address space mismatch means that bounce
>> buffers will be needed for correct operation, but those seem to be
>> lacking from the upstream driver unless I'm mistaken. I believe USB
>> storage will be busted on Lager with LPAE enabled unless bounce
>> buffers are implemented. For a reference implementation, please grep
>> for "ixp4xx_needs_bounce".
>
>
> The host bridge cannot cope with all the memory in the normal
> address space, let alone the extended 40bit addressing. There
> needs to be something done about (a) iommu support and (b) if
> there is no iommu available limiting the DMA pool available for
> the OHCI and EHCI drivers.

Thanks I'm hinting at that yes. =)

I don't think DMA pool by itself is enough though, HIGHMEM is probably
used for cached disk contents so to also cover the USB storage case I
believe we need to use bounce buffers. I recall something similar was
needed for SM501 with local memory, I may be wrong about this case
though. As-is is probably not enough though.

Cheers,

/ magnus

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

Hi Ben,

On Mon, Jan 27, 2014 at 7:31 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> Hi Ben,
>
> On Mon, Jan 27, 2014 at 7:24 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> On 27/01/14 10:19, Magnus Damm wrote:
>>>
>>> Hi Valentine,
>>>
>>> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>
>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>>
>>>>>
>>>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>>>
>>>>>>
>>>>>> The first patch adds USBHS device and the rest add PCI USB host support
>>>>>> to Lager board.
>>>>>> The patches depend on the following commits:
>>>>>> USBHS and PCI USB host:
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>>>> PCI USB host:
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>>>
>>>>>
>>>>>
>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>> think it is something to do with the bridge setup but have yet to
>>>>> have enough time to look into this issue.
>>>>>
>>>>
>>>> All PCI ports have worked fine for me.
>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>
>>>
>>> Thanks for your recommendation. From my side I must say that I
>>> recommend against using USB0 for USB host unless you're sure about
>>> your hardware settings. This to prevent driving VBUS incorrectly from
>>> two hosts with the wrong cable. Normally micro type of connector is
>>> used for USB Function, and on Lager the cable detection functionality
>>> is missing. So due to missing cable detection implementation it is
>>> probably much safer to just use USB0 for Function.
>>>
>>> If I may jump to a slightly different topic, but still USB PCI, then
>>> let me ask if USB PCI been tested with LPAE enabled? I am asking
>>> because Lager has 4 GiB or RAM over 40 bits of physical address space,
>>> but the PCI host controller probably only allows for up to 1 GiB
>>> physical address space.
>>>
>>> I suspect that this physical address space mismatch means that bounce
>>> buffers will be needed for correct operation, but those seem to be
>>> lacking from the upstream driver unless I'm mistaken. I believe USB
>>> storage will be busted on Lager with LPAE enabled unless bounce
>>> buffers are implemented. For a reference implementation, please grep
>>> for "ixp4xx_needs_bounce".
>>
>>
>> The host bridge cannot cope with all the memory in the normal
>> address space, let alone the extended 40bit addressing. There
>> needs to be something done about (a) iommu support and (b) if
>> there is no iommu available limiting the DMA pool available for
>> the OHCI and EHCI drivers.
>
> Thanks I'm hinting at that yes. =)
>
> I don't think DMA pool by itself is enough though, HIGHMEM is probably
> used for cached disk contents so to also cover the USB storage case I
> believe we need to use bounce buffers. I recall something similar was
> needed for SM501 with local memory, I may be wrong about this case
> though. As-is is probably not enough though.

Oh by the way, I may be wrong but I suspect it will be possible to
only use bounce buffers without IOMMU. If we hook up the IOMMU then we
will still not have enough physical / bus address space to cover all
potential memory that can be hooked up via LPAE.

Cheers,

/ magnus

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine]

On 01/27/2014 02:31 PM, Magnus Damm wrote:
> Hi Ben,
>
> On Mon, Jan 27, 2014 at 7:24 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> On 27/01/14 10:19, Magnus Damm wrote:
>>>
>>> Hi Valentine,
>>>
>>> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>
>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>>
>>>>>
>>>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>>>
>>>>>>
>>>>>> The first patch adds USBHS device and the rest add PCI USB host support
>>>>>> to Lager board.
>>>>>> The patches depend on the following commits:
>>>>>> USBHS and PCI USB host:
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>>>> PCI USB host:
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>>>> *
>>>>>>
>>>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>>>
>>>>>
>>>>>
>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>> think it is something to do with the bridge setup but have yet to
>>>>> have enough time to look into this issue.
>>>>>
>>>>
>>>> All PCI ports have worked fine for me.
>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>
>>>
>>> Thanks for your recommendation. From my side I must say that I
>>> recommend against using USB0 for USB host unless you're sure about
>>> your hardware settings. This to prevent driving VBUS incorrectly from
>>> two hosts with the wrong cable. Normally micro type of connector is
>>> used for USB Function, and on Lager the cable detection functionality
>>> is missing. So due to missing cable detection implementation it is
>>> probably much safer to just use USB0 for Function.
>>>
>>> If I may jump to a slightly different topic, but still USB PCI, then
>>> let me ask if USB PCI been tested with LPAE enabled? I am asking
>>> because Lager has 4 GiB or RAM over 40 bits of physical address space,
>>> but the PCI host controller probably only allows for up to 1 GiB
>>> physical address space.
>>>
>>> I suspect that this physical address space mismatch means that bounce
>>> buffers will be needed for correct operation, but those seem to be
>>> lacking from the upstream driver unless I'm mistaken. I believe USB
>>> storage will be busted on Lager with LPAE enabled unless bounce
>>> buffers are implemented. For a reference implementation, please grep
>>> for "ixp4xx_needs_bounce".
>>
>>
>> The host bridge cannot cope with all the memory in the normal
>> address space, let alone the extended 40bit addressing. There
>> needs to be something done about (a) iommu support and (b) if
>> there is no iommu available limiting the DMA pool available for
>> the OHCI and EHCI drivers.
>
> Thanks I'm hinting at that yes. =)
>
> I don't think DMA pool by itself is enough though, HIGHMEM is probably
> used for cached disk contents so to also cover the USB storage case I
> believe we need to use bounce buffers. I recall something similar was
> needed for SM501 with local memory, I may be wrong about this case
> though. As-is is probably not enough though.

I have played with this a bit. I haven't seen any issues with HIGHMEM.
As long as the kernel uses 1G/3G memory split (kernel/user-space) I have not seen any issues.

I have not tested the PAE yet. I planned to play more with it and send incremental patches
for that later.

Using bounce buffers with different memory model works fine for DMA transfers but it involves
changes to the board-specific code which should limit DMA zone to just 1GB for all DMA
allocations.

Unfortunately ARM PCI doesn't seem to have specific DMA memory limitation support as PowerPC does.
So I used platform notifiers to fix up DMA mask for PCI devices.

>
> Cheers,
>
> / magnus
>

Thanks,
Val.

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Ben Dooks]

On 27/01/14 10:45, Valentine wrote:
> On 01/27/2014 02:31 PM, Magnus Damm wrote:
>> Hi Ben,
>>
>> On Mon, Jan 27, 2014 at 7:24 PM, Ben Dooks <ben.dooks@codethink.co.uk>
>> wrote:
>>> On 27/01/14 10:19, Magnus Damm wrote:
>>>>
>>>> Hi Valentine,
>>>>
>>>> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
>>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>>
>>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>>>
>>>>>>
>>>>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>>>>
>>>>>>>
>>>>>>> The first patch adds USBHS device and the rest add PCI USB host
>>>>>>> support
>>>>>>> to Lager board.
>>>>>>> The patches depend on the following commits:
>>>>>>> USBHS and PCI USB host:
>>>>>>> *
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>>>>>
>>>>>>> *
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>>>>>
>>>>>>> PCI USB host:
>>>>>>> *
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>>>>>
>>>>>>> *
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>>>>>
>>>>>>> *
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>>> think it is something to do with the bridge setup but have yet to
>>>>>> have enough time to look into this issue.
>>>>>>
>>>>>
>>>>> All PCI ports have worked fine for me.
>>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>>
>>>>
>>>> Thanks for your recommendation. From my side I must say that I
>>>> recommend against using USB0 for USB host unless you're sure about
>>>> your hardware settings. This to prevent driving VBUS incorrectly from
>>>> two hosts with the wrong cable. Normally micro type of connector is
>>>> used for USB Function, and on Lager the cable detection functionality
>>>> is missing. So due to missing cable detection implementation it is
>>>> probably much safer to just use USB0 for Function.
>>>>
>>>> If I may jump to a slightly different topic, but still USB PCI, then
>>>> let me ask if USB PCI been tested with LPAE enabled? I am asking
>>>> because Lager has 4 GiB or RAM over 40 bits of physical address space,
>>>> but the PCI host controller probably only allows for up to 1 GiB
>>>> physical address space.
>>>>
>>>> I suspect that this physical address space mismatch means that bounce
>>>> buffers will be needed for correct operation, but those seem to be
>>>> lacking from the upstream driver unless I'm mistaken. I believe USB
>>>> storage will be busted on Lager with LPAE enabled unless bounce
>>>> buffers are implemented. For a reference implementation, please grep
>>>> for "ixp4xx_needs_bounce".
>>>
>>>
>>> The host bridge cannot cope with all the memory in the normal
>>> address space, let alone the extended 40bit addressing. There
>>> needs to be something done about (a) iommu support and (b) if
>>> there is no iommu available limiting the DMA pool available for
>>> the OHCI and EHCI drivers.
>>
>> Thanks I'm hinting at that yes. =)
>>
>> I don't think DMA pool by itself is enough though, HIGHMEM is probably
>> used for cached disk contents so to also cover the USB storage case I
>> believe we need to use bounce buffers. I recall something similar was
>> needed for SM501 with local memory, I may be wrong about this case
>> though. As-is is probably not enough though.
>
> I have played with this a bit. I haven't seen any issues with HIGHMEM.
> As long as the kernel uses 1G/3G memory split (kernel/user-space) I have
> not seen any issues.

The issue is the AHB<>PCI bridge can only support a 1GiB mapping on the
Lager board. If you select the 2GiB window then you're force to align
the memory window to either the lower or top half of the 32bit space
and the legacy memory mapping straddles this.

> I have not tested the PAE yet. I planned to play more with it and send
> incremental patches
> for that later.
>
> Using bounce buffers with different memory model works fine for DMA
> transfers but it involves
> changes to the board-specific code which should limit DMA zone to just
> 1GB for all DMA
> allocations.
>
> Unfortunately ARM PCI doesn't seem to have specific DMA memory
> limitation support as PowerPC does.
> So I used platform notifiers to fix up DMA mask for PCI devices.

Interesting.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

Hi Valentine,

On Mon, Jan 27, 2014 at 7:45 PM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> On 01/27/2014 02:31 PM, Magnus Damm wrote:
>>
>> Hi Ben,
>>
>> On Mon, Jan 27, 2014 at 7:24 PM, Ben Dooks <ben.dooks@codethink.co.uk>
>> wrote:
>>>
>>> On 27/01/14 10:19, Magnus Damm wrote:
>>>>
>>>>
>>>> Hi Valentine,
>>>>
>>>> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
>>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>>
>>>>>
>>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The first patch adds USBHS device and the rest add PCI USB host
>>>>>>> support
>>>>>>> to Lager board.
>>>>>>> The patches depend on the following commits:
>>>>>>> USBHS and PCI USB host:
>>>>>>> *
>>>>>>>
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>>>>> *
>>>>>>>
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>>>>> PCI USB host:
>>>>>>> *
>>>>>>>
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>>>>> *
>>>>>>>
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>>>>> *
>>>>>>>
>>>>>>>
>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>>> think it is something to do with the bridge setup but have yet to
>>>>>> have enough time to look into this issue.
>>>>>>
>>>>>
>>>>> All PCI ports have worked fine for me.
>>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>>
>>>>
>>>>
>>>> Thanks for your recommendation. From my side I must say that I
>>>> recommend against using USB0 for USB host unless you're sure about
>>>> your hardware settings. This to prevent driving VBUS incorrectly from
>>>> two hosts with the wrong cable. Normally micro type of connector is
>>>> used for USB Function, and on Lager the cable detection functionality
>>>> is missing. So due to missing cable detection implementation it is
>>>> probably much safer to just use USB0 for Function.
>>>>
>>>> If I may jump to a slightly different topic, but still USB PCI, then
>>>> let me ask if USB PCI been tested with LPAE enabled? I am asking
>>>> because Lager has 4 GiB or RAM over 40 bits of physical address space,
>>>> but the PCI host controller probably only allows for up to 1 GiB
>>>> physical address space.
>>>>
>>>> I suspect that this physical address space mismatch means that bounce
>>>> buffers will be needed for correct operation, but those seem to be
>>>> lacking from the upstream driver unless I'm mistaken. I believe USB
>>>> storage will be busted on Lager with LPAE enabled unless bounce
>>>> buffers are implemented. For a reference implementation, please grep
>>>> for "ixp4xx_needs_bounce".
>>>
>>>
>>>
>>> The host bridge cannot cope with all the memory in the normal
>>> address space, let alone the extended 40bit addressing. There
>>> needs to be something done about (a) iommu support and (b) if
>>> there is no iommu available limiting the DMA pool available for
>>> the OHCI and EHCI drivers.
>>
>>
>> Thanks I'm hinting at that yes. =)
>>
>> I don't think DMA pool by itself is enough though, HIGHMEM is probably
>> used for cached disk contents so to also cover the USB storage case I
>> believe we need to use bounce buffers. I recall something similar was
>> needed for SM501 with local memory, I may be wrong about this case
>> though. As-is is probably not enough though.
>
>
> I have played with this a bit. I haven't seen any issues with HIGHMEM.
> As long as the kernel uses 1G/3G memory split (kernel/user-space) I have not
> seen any issues.

Ok, thanks, I see. Can you please share your test cases with us?

My half-educated guess is that devices that do not use HIGHMEM will
most likely function. Whatever serial ports and keyboards will be
fine. USB storage will most likely fail if you stress test, and it is
a pretty important feature IMO so I'd like to make sure that it is
working as expected.

If you have not tested USB storage yet, would it be possible for you
to perform some basic testing?

> I have not tested the PAE yet. I planned to play more with it and send
> incremental patches
> for that later.

The thing is that LPAE is needed to support all physical memory on the
Lager board. So it's not really optional in my opinion.

To be fair, actual board support for 4GiB of memory was added rather
recently due to upstream ARM architecture code wasn't working as
expected in some cases. I expect that the defconfig for Lager will
enable LPAE. For multiplatform distro kernels this is something that
the distribution will select. Regardless of setting we want the USB
code to work as expected.

> Using bounce buffers with different memory model works fine for DMA
> transfers but it involves
> changes to the board-specific code which should limit DMA zone to just 1GB
> for all DMA
> allocations.

Uhm, I don't think this has anything to do with the DMA zone. You can
write code that specifically allocates from DMA or DMA32 zones, but in
case of general purpose OS disk caching we cannot select that. And if
we could then all HIGHMEM would be pretty darn useless since we
couldn't use it for disk caching. So HIGHMEM will be around -
especially on 32-bit ARM and especially especially on systems that use
LPAE. =)

> Unfortunately ARM PCI doesn't seem to have specific DMA memory limitation
> support as PowerPC does.
> So I used platform notifiers to fix up DMA mask for PCI devices.

Is this enough really? I think you can use the ixp4xx implementation
as a reference to see how to make use of bounce buffer support on ARM.

Thanks,

/ magnus

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Ben Dooks]

On 27/01/14 10:02, Valentine wrote:
> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>> On 24/01/14

[snip]

>> Hi, when doing this have you come across a problem where the OHCI
>> controller on USB0 fails to start with an initialisation error? I
>> think it is something to do with the bridge setup but have yet to
>> have enough time to look into this issue.
>>
>
> All PCI ports have worked fine for me.
> Please, check that the SW5/SW6 pins are set correctly on your board.

SW5 is at position 1
SW6 is at position 1

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine]

On 01/27/2014 03:25 PM, Magnus Damm wrote:
> Hi Valentine,
>
> On Mon, Jan 27, 2014 at 7:45 PM, Valentine
> <valentine.barshak@cogentembedded.com> wrote:
>> On 01/27/2014 02:31 PM, Magnus Damm wrote:
>>>
>>> Hi Ben,
>>>
>>> On Mon, Jan 27, 2014 at 7:24 PM, Ben Dooks <ben.dooks@codethink.co.uk>
>>> wrote:
>>>>
>>>> On 27/01/14 10:19, Magnus Damm wrote:
>>>>>
>>>>>
>>>>> Hi Valentine,
>>>>>
>>>>> On Mon, Jan 27, 2014 at 7:02 PM, Valentine
>>>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 24/01/14 22:28, Valentine Barshak wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The first patch adds USBHS device and the rest add PCI USB host
>>>>>>>> support
>>>>>>>> to Lager board.
>>>>>>>> The patches depend on the following commits:
>>>>>>>> USBHS and PCI USB host:
>>>>>>>> *
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idÈba8115a21226fba3211085f570b128fa271e31
>>>>>>>> *
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?idÃe5d2985ef720cbbdc63546a5c545ac4450d96e
>>>>>>>> PCI USB host:
>>>>>>>> *
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x103e127d1f8f985e8a662da6537ebc5e08902ee3
>>>>>>>> *
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id\x1ae5799ef63176cc75ec10e545cb65f620a82747
>>>>>>>> *
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-rcar&idû178d8b2fab3f2a9f203c13ffe80cfd6e01bdf1
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>>>> think it is something to do with the bridge setup but have yet to
>>>>>>> have enough time to look into this issue.
>>>>>>>
>>>>>>
>>>>>> All PCI ports have worked fine for me.
>>>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>>>
>>>>>
>>>>>
>>>>> Thanks for your recommendation. From my side I must say that I
>>>>> recommend against using USB0 for USB host unless you're sure about
>>>>> your hardware settings. This to prevent driving VBUS incorrectly from
>>>>> two hosts with the wrong cable. Normally micro type of connector is
>>>>> used for USB Function, and on Lager the cable detection functionality
>>>>> is missing. So due to missing cable detection implementation it is
>>>>> probably much safer to just use USB0 for Function.
>>>>>
>>>>> If I may jump to a slightly different topic, but still USB PCI, then
>>>>> let me ask if USB PCI been tested with LPAE enabled? I am asking
>>>>> because Lager has 4 GiB or RAM over 40 bits of physical address space,
>>>>> but the PCI host controller probably only allows for up to 1 GiB
>>>>> physical address space.
>>>>>
>>>>> I suspect that this physical address space mismatch means that bounce
>>>>> buffers will be needed for correct operation, but those seem to be
>>>>> lacking from the upstream driver unless I'm mistaken. I believe USB
>>>>> storage will be busted on Lager with LPAE enabled unless bounce
>>>>> buffers are implemented. For a reference implementation, please grep
>>>>> for "ixp4xx_needs_bounce".
>>>>
>>>>
>>>>
>>>> The host bridge cannot cope with all the memory in the normal
>>>> address space, let alone the extended 40bit addressing. There
>>>> needs to be something done about (a) iommu support and (b) if
>>>> there is no iommu available limiting the DMA pool available for
>>>> the OHCI and EHCI drivers.
>>>
>>>
>>> Thanks I'm hinting at that yes. =)
>>>
>>> I don't think DMA pool by itself is enough though, HIGHMEM is probably
>>> used for cached disk contents so to also cover the USB storage case I
>>> believe we need to use bounce buffers. I recall something similar was
>>> needed for SM501 with local memory, I may be wrong about this case
>>> though. As-is is probably not enough though.
>>
>>
>> I have played with this a bit. I haven't seen any issues with HIGHMEM.
>> As long as the kernel uses 1G/3G memory split (kernel/user-space) I have not
>> seen any issues.
>
> Ok, thanks, I see. Can you please share your test cases with us?

I've been doing disk reads/writes with dd using direct flags and various
buffer sizes in parallel with memtest.

>
> My half-educated guess is that devices that do not use HIGHMEM will
> most likely function. Whatever serial ports and keyboards will be
> fine. USB storage will most likely fail if you stress test, and it is
> a pretty important feature IMO so I'd like to make sure that it is
> working as expected.
>
> If you have not tested USB storage yet, would it be possible for you
> to perform some basic testing?

Yes, I've been doing it with USB storage. HIGHMEM is not a problem
since USB storage driver copies everything to lowmem. Besides DMA
bouncing of HIGHMEM is not supported.

>
>> I have not tested the PAE yet. I planned to play more with it and send
>> incremental patches
>> for that later.
>
> The thing is that LPAE is needed to support all physical memory on the
> Lager board. So it's not really optional in my opinion.
>
> To be fair, actual board support for 4GiB of memory was added rather
> recently due to upstream ARM architecture code wasn't working as
> expected in some cases. I expect that the defconfig for Lager will
> enable LPAE. For multiplatform distro kernels this is something that
> the distribution will select. Regardless of setting we want the USB
> code to work as expected.
>
>> Using bounce buffers with different memory model works fine for DMA
>> transfers but it involves
>> changes to the board-specific code which should limit DMA zone to just 1GB
>> for all DMA
>> allocations.
>
> Uhm, I don't think this has anything to do with the DMA zone. You can
> write code that specifically allocates from DMA or DMA32 zones, but in
> case of general purpose OS disk caching we cannot select that. And if
> we could then all HIGHMEM would be pretty darn useless since we
> couldn't use it for disk caching. So HIGHMEM will be around -
> especially on 32-bit ARM and especially especially on systems that use
> LPAE. =)
>
>> Unfortunately ARM PCI doesn't seem to have specific DMA memory limitation
>> support as PowerPC does.
>> So I used platform notifiers to fix up DMA mask for PCI devices.
>
> Is this enough really? I think you can use the ixp4xx implementation
> as a reference to see how to make use of bounce buffer support on ARM.

This is exactly what ixp4xx is doing. I'll send RFC patches for you to test.
I also needed adjust the size of the default ARM coherent DMA pool since 256K
was not enough for bouncing USB DMA buffers.

As I have said before I have not been able to see any problems so far with HIGHMEM.
Enabling LPAE doesn't seem to hard either. Besides we may want to revisit other
drivers that set 32-bit DMA mask for LPAE support as well.

The problems were observed with memory split other than 3G/1G user/kernel, when
more than 1G was available in lowmem for DMA.
>
> Thanks,
>
> / magnus
>

Thanks.
Val.

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine]

On 01/27/2014 04:17 PM, Ben Dooks wrote:
> On 27/01/14 10:02, Valentine wrote:
>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>> On 24/01/14
>
> [snip]
>
>>> Hi, when doing this have you come across a problem where the OHCI
>>> controller on USB0 fails to start with an initialisation error? I
>>> think it is something to do with the bridge setup but have yet to
>>> have enough time to look into this issue.
>>>
>>
>> All PCI ports have worked fine for me.
>> Please, check that the SW5/SW6 pins are set correctly on your board.
>
> SW5 is at position 1
> SW6 is at position 1
>

If the issue doesn't happen on other busses this is most likely
port misconfiguration issue. Looks like the USB phy channel 0 is not
configured properly.
Please, make sure that you have all the patches
noted in the cover-letter applied and that you're using the latest
devel tag from the renesas git.

Thanks,
Val.

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Ben Dooks]

On 28/01/14 21:37, Valentine wrote:
> On 01/27/2014 04:17 PM, Ben Dooks wrote:
>> On 27/01/14 10:02, Valentine wrote:
>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>> On 24/01/14
>>
>> [snip]
>>
>>>> Hi, when doing this have you come across a problem where the OHCI
>>>> controller on USB0 fails to start with an initialisation error? I
>>>> think it is something to do with the bridge setup but have yet to
>>>> have enough time to look into this issue.
>>>>
>>>
>>> All PCI ports have worked fine for me.
>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>
>> SW5 is at position 1
>> SW6 is at position 1
>>
>
> If the issue doesn't happen on other busses this is most likely
> port misconfiguration issue. Looks like the USB phy channel 0 is not
> configured properly.
> Please, make sure that you have all the patches
> noted in the cover-letter applied and that you're using the latest
> devel tag from the renesas git.

Thanks. I am currently in the middle of trying to sort out merging
together what we do have to just get the DT support going on the
Lager board. This is making re-basing quite difficult as our tree
is in the region of 60 patches over Simon's devel tree!

I am going to have to push working on this out to at-least next week
when hopefully we will have a new kernel to start working on with
some of these patches merged.

It is weird, since the OHCI driver dies with what looks like a failure
to DMA memory.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine]

On 01/29/2014 12:40 PM, Ben Dooks wrote:
> On 28/01/14 21:37, Valentine wrote:
>> On 01/27/2014 04:17 PM, Ben Dooks wrote:
>>> On 27/01/14 10:02, Valentine wrote:
>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>> On 24/01/14
>>>
>>> [snip]
>>>
>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>> think it is something to do with the bridge setup but have yet to
>>>>> have enough time to look into this issue.
>>>>>
>>>>
>>>> All PCI ports have worked fine for me.
>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>
>>> SW5 is at position 1
>>> SW6 is at position 1
>>>
>>
>> If the issue doesn't happen on other busses this is most likely
>> port misconfiguration issue. Looks like the USB phy channel 0 is not
>> configured properly.
>> Please, make sure that you have all the patches
>> noted in the cover-letter applied and that you're using the latest
>> devel tag from the renesas git.
>
> Thanks. I am currently in the middle of trying to sort out merging
> together what we do have to just get the DT support going on the
> Lager board. This is making re-basing quite difficult as our tree
> is in the region of 60 patches over Simon's devel tree!
>
> I am going to have to push working on this out to at-least next week
> when hopefully we will have a new kernel to start working on with
> some of these patches merged.
>
> It is weird, since the OHCI driver dies with what looks like a failure
> to DMA memory.

Could you please share the full kernel log?

Thanks,
Val.

>
>

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

Hi Valentine,

Thanks a lot for your efforts.

On Wed, Jan 29, 2014 at 3:34 AM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> On 01/27/2014 03:25 PM, Magnus Damm wrote:
>> On Mon, Jan 27, 2014 at 7:45 PM, Valentine
>> <valentine.barshak@cogentembedded.com> wrote:
>>> On 01/27/2014 02:31 PM, Magnus Damm wrote:
>>>> I don't think DMA pool by itself is enough though, HIGHMEM is probably
>>>> used for cached disk contents so to also cover the USB storage case I
>>>> believe we need to use bounce buffers. I recall something similar was
>>>> needed for SM501 with local memory, I may be wrong about this case
>>>> though. As-is is probably not enough though.
>>>
>>> I have played with this a bit. I haven't seen any issues with HIGHMEM.
>>> As long as the kernel uses 1G/3G memory split (kernel/user-space) I have
>>> not
>>> seen any issues.
>>
>> Ok, thanks, I see. Can you please share your test cases with us?
>
>
> I've been doing disk reads/writes with dd using direct flags and various
> buffer sizes in parallel with memtest.

The "direct flag" thing here sticks out a bit. Have you tried using a
file system?

>> My half-educated guess is that devices that do not use HIGHMEM will
>> most likely function. Whatever serial ports and keyboards will be
>> fine. USB storage will most likely fail if you stress test, and it is
>> a pretty important feature IMO so I'd like to make sure that it is
>> working as expected.
>>
>> If you have not tested USB storage yet, would it be possible for you
>> to perform some basic testing?
>
>
> Yes, I've been doing it with USB storage. HIGHMEM is not a problem
> since USB storage driver copies everything to lowmem. Besides DMA
> bouncing of HIGHMEM is not supported.

I see. Can you point out where this code is located to I can have a look?

>>> I have not tested the PAE yet. I planned to play more with it and send
>>> incremental patches
>>> for that later.
>>
>>
>> The thing is that LPAE is needed to support all physical memory on the
>> Lager board. So it's not really optional in my opinion.
>>
>> To be fair, actual board support for 4GiB of memory was added rather
>> recently due to upstream ARM architecture code wasn't working as
>> expected in some cases. I expect that the defconfig for Lager will
>> enable LPAE. For multiplatform distro kernels this is something that
>> the distribution will select. Regardless of setting we want the USB
>> code to work as expected.
>>
>>> Using bounce buffers with different memory model works fine for DMA
>>> transfers but it involves
>>> changes to the board-specific code which should limit DMA zone to just
>>> 1GB
>>> for all DMA
>>> allocations.
>>
>>
>> Uhm, I don't think this has anything to do with the DMA zone. You can
>> write code that specifically allocates from DMA or DMA32 zones, but in
>> case of general purpose OS disk caching we cannot select that. And if
>> we could then all HIGHMEM would be pretty darn useless since we
>> couldn't use it for disk caching. So HIGHMEM will be around -
>> especially on 32-bit ARM and especially especially on systems that use
>> LPAE. =)
>>
>>> Unfortunately ARM PCI doesn't seem to have specific DMA memory limitation
>>> support as PowerPC does.
>>> So I used platform notifiers to fix up DMA mask for PCI devices.
>>
>>
>> Is this enough really? I think you can use the ixp4xx implementation
>> as a reference to see how to make use of bounce buffer support on ARM.
>
>
> This is exactly what ixp4xx is doing. I'll send RFC patches for you to test.
> I also needed adjust the size of the default ARM coherent DMA pool since
> 256K
> was not enough for bouncing USB DMA buffers.

Thanks for your work on that!

> As I have said before I have not been able to see any problems so far with
> HIGHMEM.
> Enabling LPAE doesn't seem to hard either. Besides we may want to revisit
> other
> drivers that set 32-bit DMA mask for LPAE support as well.
>
> The problems were observed with memory split other than 3G/1G user/kernel,
> when
> more than 1G was available in lowmem for DMA.

I'm not so concerned about the user/kernel split, but I understand
that it may affect things. My main concern is that HIGHMEM should be
handled correctly at this point.

Thanks,

/ magnus

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Valentine]

On 01/30/2014 03:39 AM, Magnus Damm wrote:
> Hi Valentine,
>
> Thanks a lot for your efforts.
>
> On Wed, Jan 29, 2014 at 3:34 AM, Valentine
> <valentine.barshak@cogentembedded.com> wrote:
>> On 01/27/2014 03:25 PM, Magnus Damm wrote:
>>> On Mon, Jan 27, 2014 at 7:45 PM, Valentine
>>> <valentine.barshak@cogentembedded.com> wrote:
>>>> On 01/27/2014 02:31 PM, Magnus Damm wrote:
>>>>> I don't think DMA pool by itself is enough though, HIGHMEM is probably
>>>>> used for cached disk contents so to also cover the USB storage case I
>>>>> believe we need to use bounce buffers. I recall something similar was
>>>>> needed for SM501 with local memory, I may be wrong about this case
>>>>> though. As-is is probably not enough though.
>>>>
>>>> I have played with this a bit. I haven't seen any issues with HIGHMEM.
>>>> As long as the kernel uses 1G/3G memory split (kernel/user-space) I have
>>>> not
>>>> seen any issues.
>>>
>>> Ok, thanks, I see. Can you please share your test cases with us?
>>
>>
>> I've been doing disk reads/writes with dd using direct flags and various
>> buffer sizes in parallel with memtest.
>
> The "direct flag" thing here sticks out a bit. Have you tried using a
> file system?

Yes, I actually switched to direct after I had tested without it,
I did some extensive copying back and forth from one USB storage to another
and stuff like that with some memory consuming processes in the background.

>
>>> My half-educated guess is that devices that do not use HIGHMEM will
>>> most likely function. Whatever serial ports and keyboards will be
>>> fine. USB storage will most likely fail if you stress test, and it is
>>> a pretty important feature IMO so I'd like to make sure that it is
>>> working as expected.
>>>
>>> If you have not tested USB storage yet, would it be possible for you
>>> to perform some basic testing?
>>
>>
>> Yes, I've been doing it with USB storage. HIGHMEM is not a problem
>> since USB storage driver copies everything to lowmem. Besides DMA
>> bouncing of HIGHMEM is not supported.
>
> I see. Can you point out where this code is located to I can have a look?

grep -r  "HIGHMEM" arch/arm/common/dmabounce.c
# dev_err(dev, "DMA buffer bouncing of HIGHMEM pages is not supported\n");

>
>>>> I have not tested the PAE yet. I planned to play more with it and send
>>>> incremental patches
>>>> for that later.
>>>
>>>
>>> The thing is that LPAE is needed to support all physical memory on the
>>> Lager board. So it's not really optional in my opinion.
>>>
>>> To be fair, actual board support for 4GiB of memory was added rather
>>> recently due to upstream ARM architecture code wasn't working as
>>> expected in some cases. I expect that the defconfig for Lager will
>>> enable LPAE. For multiplatform distro kernels this is something that
>>> the distribution will select. Regardless of setting we want the USB
>>> code to work as expected.
>>>
>>>> Using bounce buffers with different memory model works fine for DMA
>>>> transfers but it involves
>>>> changes to the board-specific code which should limit DMA zone to just
>>>> 1GB
>>>> for all DMA
>>>> allocations.
>>>
>>>
>>> Uhm, I don't think this has anything to do with the DMA zone. You can
>>> write code that specifically allocates from DMA or DMA32 zones, but in
>>> case of general purpose OS disk caching we cannot select that. And if
>>> we could then all HIGHMEM would be pretty darn useless since we
>>> couldn't use it for disk caching. So HIGHMEM will be around -
>>> especially on 32-bit ARM and especially especially on systems that use
>>> LPAE. =)
>>>
>>>> Unfortunately ARM PCI doesn't seem to have specific DMA memory limitation
>>>> support as PowerPC does.
>>>> So I used platform notifiers to fix up DMA mask for PCI devices.
>>>
>>>
>>> Is this enough really? I think you can use the ixp4xx implementation
>>> as a reference to see how to make use of bounce buffer support on ARM.
>>
>>
>> This is exactly what ixp4xx is doing. I'll send RFC patches for you to test.
>> I also needed adjust the size of the default ARM coherent DMA pool since
>> 256K
>> was not enough for bouncing USB DMA buffers.
>
> Thanks for your work on that!
>
>> As I have said before I have not been able to see any problems so far with
>> HIGHMEM.
>> Enabling LPAE doesn't seem to hard either. Besides we may want to revisit
>> other
>> drivers that set 32-bit DMA mask for LPAE support as well.
>>
>> The problems were observed with memory split other than 3G/1G user/kernel,
>> when
>> more than 1G was available in lowmem for DMA.
>
> I'm not so concerned about the user/kernel split, but I understand
> that it may affect things. My main concern is that HIGHMEM should be
> handled correctly at this point.

Haven't seen any problems with HIGHMEM (and no dmabounce) so far.
AFAIU, the USB storage protocol copies all data from HIGHMEM to the internal
buffer before the transfer:
drivers/usb/storage/protocol.c:unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,

>
> Thanks,
>
> / magnus
>

Thanks,
Val.

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

Hi Valentine,

On Thu, Jan 30, 2014 at 9:08 AM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> On 01/30/2014 03:39 AM, Magnus Damm wrote:
>>
>> Hi Valentine,
>>
>> Thanks a lot for your efforts.
>>
>> On Wed, Jan 29, 2014 at 3:34 AM, Valentine
>> <valentine.barshak@cogentembedded.com> wrote:
>>>
>>> On 01/27/2014 03:25 PM, Magnus Damm wrote:
>>>>
>>>> On Mon, Jan 27, 2014 at 7:45 PM, Valentine
>>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>>
>>>>> On 01/27/2014 02:31 PM, Magnus Damm wrote:
>>>>>>
>>>>>> I don't think DMA pool by itself is enough though, HIGHMEM is probably
>>>>>> used for cached disk contents so to also cover the USB storage case I
>>>>>> believe we need to use bounce buffers. I recall something similar was
>>>>>> needed for SM501 with local memory, I may be wrong about this case
>>>>>> though. As-is is probably not enough though.
>>>>>
>>>>>
>>>>> I have played with this a bit. I haven't seen any issues with HIGHMEM.
>>>>> As long as the kernel uses 1G/3G memory split (kernel/user-space) I
>>>>> have
>>>>> not
>>>>> seen any issues.
>>>>
>>>>
>>>> Ok, thanks, I see. Can you please share your test cases with us?
>>>
>>>
>>>
>>> I've been doing disk reads/writes with dd using direct flags and various
>>> buffer sizes in parallel with memtest.
>>
>>
>> The "direct flag" thing here sticks out a bit. Have you tried using a
>> file system?
>
>
> Yes, I actually switched to direct after I had tested without it,
> I did some extensive copying back and forth from one USB storage to another
> and stuff like that with some memory consuming processes in the background.

Ok, thanks for the clarification.

>>>> My half-educated guess is that devices that do not use HIGHMEM will
>>>> most likely function. Whatever serial ports and keyboards will be
>>>> fine. USB storage will most likely fail if you stress test, and it is
>>>> a pretty important feature IMO so I'd like to make sure that it is
>>>> working as expected.
>>>>
>>>> If you have not tested USB storage yet, would it be possible for you
>>>> to perform some basic testing?
>>>
>>>
>>>
>>> Yes, I've been doing it with USB storage. HIGHMEM is not a problem
>>> since USB storage driver copies everything to lowmem. Besides DMA
>>> bouncing of HIGHMEM is not supported.
>>
>>
>> I see. Can you point out where this code is located to I can have a look?
>
>
> grep -r  "HIGHMEM" arch/arm/common/dmabounce.c
> # dev_err(dev, "DMA buffer bouncing of HIGHMEM pages is not supported\n");

So I think there has been some confusion in our discussion related to
the bounce buffers. You are correct that DMABOUNCE does not support
HIGHMEM.

>>>>> I have not tested the PAE yet. I planned to play more with it and send
>>>>> incremental patches
>>>>> for that later.
>>>>
>>>>
>>>>
>>>> The thing is that LPAE is needed to support all physical memory on the
>>>> Lager board. So it's not really optional in my opinion.
>>>>
>>>> To be fair, actual board support for 4GiB of memory was added rather
>>>> recently due to upstream ARM architecture code wasn't working as
>>>> expected in some cases. I expect that the defconfig for Lager will
>>>> enable LPAE. For multiplatform distro kernels this is something that
>>>> the distribution will select. Regardless of setting we want the USB
>>>> code to work as expected.
>>>>
>>>>> Using bounce buffers with different memory model works fine for DMA
>>>>> transfers but it involves
>>>>> changes to the board-specific code which should limit DMA zone to just
>>>>> 1GB
>>>>> for all DMA
>>>>> allocations.
>>>>
>>>>
>>>>
>>>> Uhm, I don't think this has anything to do with the DMA zone. You can
>>>> write code that specifically allocates from DMA or DMA32 zones, but in
>>>> case of general purpose OS disk caching we cannot select that. And if
>>>> we could then all HIGHMEM would be pretty darn useless since we
>>>> couldn't use it for disk caching. So HIGHMEM will be around -
>>>> especially on 32-bit ARM and especially especially on systems that use
>>>> LPAE. =)
>>>>
>>>>> Unfortunately ARM PCI doesn't seem to have specific DMA memory
>>>>> limitation
>>>>> support as PowerPC does.
>>>>> So I used platform notifiers to fix up DMA mask for PCI devices.
>>>>
>>>>
>>>>
>>>> Is this enough really? I think you can use the ixp4xx implementation
>>>> as a reference to see how to make use of bounce buffer support on ARM.
>>>
>>>
>>>
>>> This is exactly what ixp4xx is doing. I'll send RFC patches for you to
>>> test.
>>> I also needed adjust the size of the default ARM coherent DMA pool since
>>> 256K
>>> was not enough for bouncing USB DMA buffers.

Thanks for your efforts there. I can see that you planned to use the
DMA zone and also board specific bounce buffer setup. In my mind the
1GiB limitation comes from the actual PCI hardware so it should be
handled by the driver. Please see the following series for my
proposal:

[PATCH 00/04] PCI: rcar: Driver model and physical address space update

>> Thanks for your work on that!
>>
>>> As I have said before I have not been able to see any problems so far
>>> with
>>> HIGHMEM.
>>> Enabling LPAE doesn't seem to hard either. Besides we may want to revisit
>>> other
>>> drivers that set 32-bit DMA mask for LPAE support as well.
>>>
>>> The problems were observed with memory split other than 3G/1G
>>> user/kernel,
>>> when
>>> more than 1G was available in lowmem for DMA.
>>
>>
>> I'm not so concerned about the user/kernel split, but I understand
>> that it may affect things. My main concern is that HIGHMEM should be
>> handled correctly at this point.
>
>
> Haven't seen any problems with HIGHMEM (and no dmabounce) so far.
> AFAIU, the USB storage protocol copies all data from HIGHMEM to the internal
> buffer before the transfer:
> drivers/usb/storage/protocol.c:unsigned int
> usb_stor_access_xfer_buf(unsigned char *buffer,

Thanks for the pointer, but I don't think that function is used by the
general purpose usb storage code. Instead I believe you mean to refer
to the CONFIG_BOUNCE code in block/blk-settings.c.

So without any modifications to the driver and CONFIG_BOUNCE=n we get
some broken runtime behaviour in case of HIGHMEM.

By the way, USB1 is working well on my Lager board. Regardless of
kernel configuration and pci-rcar-gen2.c patches I can't get USB2 to
work well though. Is USB2 working well on your Lager board?

Thanks,

/ magnus

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Ben Dooks]

On 28/01/14 21:37, Valentine wrote:
> On 01/27/2014 04:17 PM, Ben Dooks wrote:
>> On 27/01/14 10:02, Valentine wrote:
>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>> On 24/01/14
>>
>> [snip]
>>
>>>> Hi, when doing this have you come across a problem where the OHCI
>>>> controller on USB0 fails to start with an initialisation error? I
>>>> think it is something to do with the bridge setup but have yet to
>>>> have enough time to look into this issue.
>>>>
>>>
>>> All PCI ports have worked fine for me.
>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>
>> SW5 is at position 1
>> SW6 is at position 1
>>
>
> If the issue doesn't happen on other busses this is most likely
> port misconfiguration issue. Looks like the USB phy channel 0 is not
> configured properly.
> Please, make sure that you have all the patches
> noted in the cover-letter applied and that you're using the latest
> devel tag from the renesas git.

I tracked it down to the phy driver being called after the ohci/ehci
drivers being initialised. It now seems to be working with all three
channels.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH V3 0/3] ARM: shmobile: lager: Add USB support

[Magnus Damm]

On Wed, Feb 5, 2014 at 6:31 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 28/01/14 21:37, Valentine wrote:
>>
>> On 01/27/2014 04:17 PM, Ben Dooks wrote:
>>>
>>> On 27/01/14 10:02, Valentine wrote:
>>>>
>>>> On 01/27/2014 01:59 PM, Ben Dooks wrote:
>>>>>
>>>>> On 24/01/14
>>>
>>>
>>> [snip]
>>>
>>>>> Hi, when doing this have you come across a problem where the OHCI
>>>>> controller on USB0 fails to start with an initialisation error? I
>>>>> think it is something to do with the bridge setup but have yet to
>>>>> have enough time to look into this issue.
>>>>>
>>>>
>>>> All PCI ports have worked fine for me.
>>>> Please, check that the SW5/SW6 pins are set correctly on your board.
>>>
>>>
>>> SW5 is at position 1
>>> SW6 is at position 1
>>>
>>
>> If the issue doesn't happen on other busses this is most likely
>> port misconfiguration issue. Looks like the USB phy channel 0 is not
>> configured properly.
>> Please, make sure that you have all the patches
>> noted in the cover-letter applied and that you're using the latest
>> devel tag from the renesas git.
>
>
> I tracked it down to the phy driver being called after the ohci/ehci
> drivers being initialised. It now seems to be working with all three
> channels.

Thanks for sharing. I too managed to get USB1 and USB2 on lager going
as expected.

Cheers,

/ magnus