diff mbox

[V2,2/3] ARM: shmobile: r8a7790: Add PCI USB host clock support

Message ID 1389982854-20680-3-git-send-email-valentine.barshak@cogentembedded.com (mailing list archive)
State Superseded
Headers show

Commit Message

Valentine Barshak Jan. 17, 2014, 6:20 p.m. UTC
This adds internal PCI USB host clock support.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 arch/arm/mach-shmobile/clock-r8a7790.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Changes in V2:
* capitalized ARM in the subject;
* rebased on top the latest devel tag.

Comments

Magnus Damm Jan. 24, 2014, 4:47 a.m. UTC | #1
On Sat, Jan 18, 2014 at 3:20 AM, Valentine Barshak
<valentine.barshak@cogentembedded.com> wrote:
> This adds internal PCI USB host clock support.
>
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  arch/arm/mach-shmobile/clock-r8a7790.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> Changes in V2:
> * capitalized ARM in the subject;
> * rebased on top the latest devel tag.

This patch for PCI USB clock support seems fine to me.

Acked-by: Magnus Damm <damm@opensource.se>

It is common practice that maintainers reject code to encourage people
to do further development. I don't think that will help us in this
particular case, so I think it is best to merge this as-is but also
request you to spend your future time on DT development.

So please work on DT reference support together with CCF for PCI USB.

Simon, please pick up if you agree with me.

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Dooks Jan. 24, 2014, 11:37 a.m. UTC | #2
On 24/01/14 04:47, Magnus Damm wrote:
> On Sat, Jan 18, 2014 at 3:20 AM, Valentine Barshak
> <valentine.barshak@cogentembedded.com> wrote:
>> This adds internal PCI USB host clock support.
>>
>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>> ---
>>   arch/arm/mach-shmobile/clock-r8a7790.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> Changes in V2:
>> * capitalized ARM in the subject;
>> * rebased on top the latest devel tag.
>
> This patch for PCI USB clock support seems fine to me.
>
> Acked-by: Magnus Damm <damm@opensource.se>
>
> It is common practice that maintainers reject code to encourage people
> to do further development. I don't think that will help us in this
> particular case, so I think it is best to merge this as-is but also
> request you to spend your future time on DT development.
>
> So please work on DT reference support together with CCF for PCI USB.

I've already posted a series for getting this device treeed.

As a note, does channel 0 work for you? we are getting a lot of
failures with the OHCI controller failing to start with what looks
like a bus-master failure.
Magnus Damm Jan. 28, 2014, 12:28 a.m. UTC | #3
On Fri, Jan 24, 2014 at 8:37 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 24/01/14 04:47, Magnus Damm wrote:
>>
>> On Sat, Jan 18, 2014 at 3:20 AM, Valentine Barshak
>> <valentine.barshak@cogentembedded.com> wrote:
>>>
>>> This adds internal PCI USB host clock support.
>>>
>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>>> ---
>>>   arch/arm/mach-shmobile/clock-r8a7790.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> Changes in V2:
>>> * capitalized ARM in the subject;
>>> * rebased on top the latest devel tag.
>>
>>
>> This patch for PCI USB clock support seems fine to me.
>>
>> Acked-by: Magnus Damm <damm@opensource.se>
>>
>> It is common practice that maintainers reject code to encourage people
>> to do further development. I don't think that will help us in this
>> particular case, so I think it is best to merge this as-is but also
>> request you to spend your future time on DT development.
>>
>> So please work on DT reference support together with CCF for PCI USB.
>
>
> I've already posted a series for getting this device treeed.

Thanks for that!

> As a note, does channel 0 work for you? we are getting a lot of
> failures with the OHCI controller failing to start with what looks
> like a bus-master failure.

I've only used USB0 as USBHS, and due to lack of cable detection
hardware support on Lager (and me missing the cable needed for host) I
plan on simply fixing USB0 as USBHS Function. Anyone wanting to test
USB PCI Host should in my opinion use USB1 on Lager. USB2 is also fine
until USB 3.0 support is tied in there.

As both you and I have noted, the USB PCI stuff can't work well as-is
due to mismatch in physical address space between the PCI hardware and
the actual memory on Lager. So as we discussed, bounce buffers would
be needed and perhaps also IOMMU support but my feeling is that may as
well be for later in case of USB.

So until I see USB PCI bounce buffer support or similar I will simply
assume all errors on USB PCI on r8a7790 and r8a7791 are related to
physical address size mismatch and lacking software support. Do you
have anything that points in a different direction?

Does USB1 and/or USB2 work for you as-is today?

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Dooks Jan. 28, 2014, 8:35 a.m. UTC | #4
On 28/01/14 00:28, Magnus Damm wrote:
> On Fri, Jan 24, 2014 at 8:37 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> On 24/01/14 04:47, Magnus Damm wrote:
>>>
>>> On Sat, Jan 18, 2014 at 3:20 AM, Valentine Barshak
>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>
>>>> This adds internal PCI USB host clock support.
>>>>
>>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>>>> ---
>>>>    arch/arm/mach-shmobile/clock-r8a7790.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> Changes in V2:
>>>> * capitalized ARM in the subject;
>>>> * rebased on top the latest devel tag.
>>>
>>>
>>> This patch for PCI USB clock support seems fine to me.
>>>
>>> Acked-by: Magnus Damm <damm@opensource.se>
>>>
>>> It is common practice that maintainers reject code to encourage people
>>> to do further development. I don't think that will help us in this
>>> particular case, so I think it is best to merge this as-is but also
>>> request you to spend your future time on DT development.
>>>
>>> So please work on DT reference support together with CCF for PCI USB.
>>
>>
>> I've already posted a series for getting this device treeed.
>
> Thanks for that!
>
>> As a note, does channel 0 work for you? we are getting a lot of
>> failures with the OHCI controller failing to start with what looks
>> like a bus-master failure.
>
> I've only used USB0 as USBHS, and due to lack of cable detection
> hardware support on Lager (and me missing the cable needed for host) I
> plan on simply fixing USB0 as USBHS Function. Anyone wanting to test
> USB PCI Host should in my opinion use USB1 on Lager. USB2 is also fine
> until USB 3.0 support is tied in there.
>
> As both you and I have noted, the USB PCI stuff can't work well as-is
> due to mismatch in physical address space between the PCI hardware and
> the actual memory on Lager. So as we discussed, bounce buffers would
> be needed and perhaps also IOMMU support but my feeling is that may as
> well be for later in case of USB.
>
> So until I see USB PCI bounce buffer support or similar I will simply
> assume all errors on USB PCI on r8a7790 and r8a7791 are related to
> physical address size mismatch and lacking software support. Do you
> have anything that points in a different direction?
>
> Does USB1 and/or USB2 work for you as-is today?

Yes, although I have been doing my initial testing with the RAM size
limited to 1GiB of memory.

I've found a couple of minor issues with the PCI bridge driver that
so far have not actually fixed my problem which I hope to get reviewed
and sent to the lists today.

I am going to try the 'legacy' boot method today and see if I get the
same problems, just to verify if the problem is with the fdt conversion
or there is an issue with my board/code.
Ben Dooks Jan. 28, 2014, 10:20 a.m. UTC | #5
On 28/01/14 08:35, Ben Dooks wrote:

> Yes, although I have been doing my initial testing with the RAM size
> limited to 1GiB of memory.
>
> I've found a couple of minor issues with the PCI bridge driver that
> so far have not actually fixed my problem which I hope to get reviewed
> and sent to the lists today.
>
> I am going to try the 'legacy' boot method today and see if I get the
> same problems, just to verify if the problem is with the fdt conversion
> or there is an issue with my board/code.

I've sent the test/fix patches to the list, and have tried building
with the legacy H2 code which seems to do the same thing.

I cannot seem to currently build a working 3.4-ltsi release, so will
have to wait to see if that will work on my board or not.
Magnus Damm Jan. 28, 2014, 10:29 a.m. UTC | #6
Hi Ben,

On Tue, Jan 28, 2014 at 7:20 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> On 28/01/14 08:35, Ben Dooks wrote:
>
>> Yes, although I have been doing my initial testing with the RAM size
>> limited to 1GiB of memory.
>>
>> I've found a couple of minor issues with the PCI bridge driver that
>> so far have not actually fixed my problem which I hope to get reviewed
>> and sent to the lists today.
>>
>> I am going to try the 'legacy' boot method today and see if I get the
>> same problems, just to verify if the problem is with the fdt conversion
>> or there is an issue with my board/code.
>
>
> I've sent the test/fix patches to the list, and have tried building
> with the legacy H2 code which seems to do the same thing.

Ok, thanks for checking.

> I cannot seem to currently build a working 3.4-ltsi release, so will
> have to wait to see if that will work on my board or not.

So I prefer to focus on moving forward upstream, but if you're
blocking on this then feel free to ask Simon about LTSI and he can
most likely guide you. This includes stuff in renesas-backport git
repo but excludes the BSP bits.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Dooks Jan. 28, 2014, 10:34 a.m. UTC | #7
On 28/01/14 10:29, Magnus Damm wrote:
> Hi Ben,
>
> On Tue, Jan 28, 2014 at 7:20 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> On 28/01/14 08:35, Ben Dooks wrote:
>>
>>> Yes, although I have been doing my initial testing with the RAM size
>>> limited to 1GiB of memory.
>>>
>>> I've found a couple of minor issues with the PCI bridge driver that
>>> so far have not actually fixed my problem which I hope to get reviewed
>>> and sent to the lists today.
>>>
>>> I am going to try the 'legacy' boot method today and see if I get the
>>> same problems, just to verify if the problem is with the fdt conversion
>>> or there is an issue with my board/code.
>>
>>
>> I've sent the test/fix patches to the list, and have tried building
>> with the legacy H2 code which seems to do the same thing.
>
> Ok, thanks for checking.
>
>> I cannot seem to currently build a working 3.4-ltsi release, so will
>> have to wait to see if that will work on my board or not.
>
> So I prefer to focus on moving forward upstream, but if you're
> blocking on this then feel free to ask Simon about LTSI and he can
> most likely guide you. This includes stuff in renesas-backport git
> repo but excludes the BSP bits.

Yeah, we've had some weird bit-rot issue with this. I will try a clean
checkout later. The ltsi is just really to see if our Lager board has
some issue with the USB0 channel (verify the hardware has not been
broken at some point)

I am going to leave the CH0 issue for the moment and wait until we've
cleared this release so that we can then look at rebasing on 3.14-rc1
after FOSDEM.
Valentine Barshak Jan. 28, 2014, 6:23 p.m. UTC | #8
On 01/28/2014 04:28 AM, Magnus Damm wrote:
> On Fri, Jan 24, 2014 at 8:37 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> On 24/01/14 04:47, Magnus Damm wrote:
>>>
>>> On Sat, Jan 18, 2014 at 3:20 AM, Valentine Barshak
>>> <valentine.barshak@cogentembedded.com> wrote:
>>>>
>>>> This adds internal PCI USB host clock support.
>>>>
>>>> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
>>>> ---
>>>>    arch/arm/mach-shmobile/clock-r8a7790.c | 6 +++++-
>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> Changes in V2:
>>>> * capitalized ARM in the subject;
>>>> * rebased on top the latest devel tag.
>>>
>>>
>>> This patch for PCI USB clock support seems fine to me.
>>>
>>> Acked-by: Magnus Damm <damm@opensource.se>
>>>
>>> It is common practice that maintainers reject code to encourage people
>>> to do further development. I don't think that will help us in this
>>> particular case, so I think it is best to merge this as-is but also
>>> request you to spend your future time on DT development.
>>>
>>> So please work on DT reference support together with CCF for PCI USB.
>>
>>
>> I've already posted a series for getting this device treeed.
>
> Thanks for that!
>
>> As a note, does channel 0 work for you? we are getting a lot of
>> failures with the OHCI controller failing to start with what looks
>> like a bus-master failure.
>
> I've only used USB0 as USBHS, and due to lack of cable detection
> hardware support on Lager (and me missing the cable needed for host) I
> plan on simply fixing USB0 as USBHS Function. Anyone wanting to test
> USB PCI Host should in my opinion use USB1 on Lager. USB2 is also fine
> until USB 3.0 support is tied in there.
>
> As both you and I have noted, the USB PCI stuff can't work well as-is
> due to mismatch in physical address space between the PCI hardware and
> the actual memory on Lager. So as we discussed, bounce buffers would
> be needed and perhaps also IOMMU support but my feeling is that may as
> well be for later in case of USB.

As I have said earlier I have not experienced any issues with the current kernel configuration.
Enabling highmem also didn't show any harm. I'm currently testing LPAE and have not been able
to see any issues as well. I've been using memtest and reading/writing USB storage at the same time.

AFAIU, the DMA is allocated in the lowmem for all USB transfers. Even when disk buffers are allocated
in the highmem, the contents is still copied to a lowmem buffer by the USB storage driver.
If anyone tried to allocate DMA buffer in highmem, using DMA bounce buffers would not help
since bouncing of highmem is not supported.

>
> So until I see USB PCI bounce buffer support or similar I will simply
> assume all errors on USB PCI on r8a7790 and r8a7791 are related to
> physical address size mismatch and lacking software support. Do you
> have anything that points in a different direction?

The only case that I have been able to see any errors is when a different memory split
model is used in the kernel. For example, if 2G/2G user/kernel memory model is used
instead of the default 3G/1G one. In this case DMA-able low memory is 2G and the bounce buffer
is needed.

Thanks,
Val.

>
> Does USB1 and/or USB2 work for you as-is today?
>
> Thanks,
>
> / magnus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index f25b43a..507073e 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -201,7 +201,7 @@  enum {
 	MSTP811, MSTP810, MSTP809, MSTP808,
 	MSTP726, MSTP725, MSTP724, MSTP723, MSTP722, MSTP721, MSTP720,
 	MSTP717, MSTP716,
-	MSTP704,
+	MSTP704, MSTP703,
 	MSTP522,
 	MSTP502, MSTP501,
 	MSTP315, MSTP314, MSTP313, MSTP312, MSTP311, MSTP305, MSTP304,
@@ -244,6 +244,7 @@  static struct clk mstp_clks[MSTP_NR] = {
 	[MSTP717] = SH_CLK_MSTP32_STS(&zs_clk, SMSTPCR7, 17, MSTPSR7, 0), /* HSCIF0 */
 	[MSTP716] = SH_CLK_MSTP32_STS(&zs_clk, SMSTPCR7, 16, MSTPSR7, 0), /* HSCIF1 */
 	[MSTP704] = SH_CLK_MSTP32_STS(&mp_clk, SMSTPCR7, 4, MSTPSR7, 0), /* HSUSB */
+	[MSTP703] = SH_CLK_MSTP32_STS(&mp_clk, SMSTPCR7, 3, MSTPSR7, 0), /* EHCI */
 	[MSTP522] = SH_CLK_MSTP32_STS(&extal_clk, SMSTPCR5, 22, MSTPSR5, 0), /* Thermal */
 	[MSTP502] = SH_CLK_MSTP32_STS(&zs_clk, SMSTPCR5, 2, MSTPSR5, 0), /* Audio-DMAC low */
 	[MSTP501] = SH_CLK_MSTP32_STS(&zs_clk, SMSTPCR5, 1, MSTPSR5, 0), /* Audio-DMAC hi */
@@ -343,6 +344,9 @@  static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]),
 	CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]),
 	CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP704]),
+	CLKDEV_DEV_ID("pci-rcar-gen2.0", &mstp_clks[MSTP703]),
+	CLKDEV_DEV_ID("pci-rcar-gen2.1", &mstp_clks[MSTP703]),
+	CLKDEV_DEV_ID("pci-rcar-gen2.2", &mstp_clks[MSTP703]),
 	CLKDEV_DEV_ID("sata-r8a7790.0", &mstp_clks[MSTP815]),
 	CLKDEV_DEV_ID("sata-r8a7790.1", &mstp_clks[MSTP814]),