Re: [LEDE-DEV] [PATCH] lantiq: set the usb clock source

2017-01-01 Thread Hauke Mehrtens


On 01/01/2017 11:21 AM, Mathias Kresin wrote:
> The clock source is set by the ltq-hcd driver but are not by the
> dwc2 driver. Without having the correct clock set the dwc driver
> fails to reset the usb core and errors out. The values for supported
> lantiq targets are exactly the same as set by the ltq-hcd driver and
> should not cause any regressions.
> 
> Fixes FS#351
> 
> Signed-off-by: Mathias Kresin 
> ---
> 
> Well the code works but is not really a beauty. But I need some input
> to _possibly_ improve the code or at least the commit message.
> 
> Based on the disabled code in the ltq-hcd driver I guess for all
> targets two bits are used to set the clock source.
> 
> I know that on vr9 the bits set to 00 or 11 means "XTAL 36 MHz input
> devide by 3" where 00 is the default value.
> 
> After playing a bit with the bits, I'm the opinion the same applies to
> Danube a well.
> 
> It's kinda strange that for Amazon only one bit is cleared within the
> ltq-hcd driver. It looks more like an error or yet unknown workaround.
> Setting the interface clock bits to 00 (default) does make more sense
> to me. I've changed this in contrast to the ltq-hcd driver but do not
> have any hardware to check this change or the dwc2 at all on Amazon.
> 
> For some reason the usb clock source has already the correct value on
> vr9. I'm not sure if any vr9 board uses a clock source different than
> the default "XTAL 36 MHz input devide by 3". For completeness I tend to
> add the clock source for vr9 as well. Any opinions?
> 
> Albeit we obviously don't have an alien board which need a different
> clock source than the others, setting the clock source to a fixed value
> seams to me a really bad idea. And since it is the second time I had to
> touch the CGU_IFCCR register [0][1] I've somehow the impression we need
> some glue code to set the different clock sources via some kind of glue
> code. I'm not sure what is the best way to do this. Should I add
> something similar to the lantiq,phy-clk-src? Is there any chance that
> this code is accepted upstream or should we consider it as LEDE hack?
> Is the common clock framework maybe the saviour?
> 
> Would anyone with access to the datasheets be so kind to confirm:
> 
> - all lantiq targets are using two bits to set the clock source
> - the meaning of the usbclk bit combinations are the same for all
>   lantiq targets (including 00 == 11)
> - it is safe to set the usb clock to 00 or 11 for Amazon
> 
> If 00 == 11 on Amazon/Danube, the lantiq,ase and lantiq,danube
> conditions can be merged.
> 
> The lantiq,ar9 can be extended to cover vr9 as well.
> 
> [0] https://git.lede-project.org/44cace4dabe186a486d6713de6196bc7cea2228b
> [1] https://git.lede-project.org/d4de9f72f31c4716f78fea8950261a3bdafdbccb

This is copied from the different documentations:

On Danube it looks like this:

USB Clock Source
Clock config is configured to:
00 B Internal, 12 MHz internal
01 B External, from USB XTAL (default)
10 B External/4, from USB XTAL then divided by 4
11 B 12, special internal clock generated from 36 MHz source

Bit 5:4 on reg 0x0018, reset value of this bit is 01 B


On Amazon SE it looks like this:

12 MHz USB Clock Source
Clock config is configured to:
0 B Divider after divider
1 B XTALInput From XTAL only

Bit 5 on reg 0x0018, reset value of this bit is 0 B


On xRX200 (VR9) it looke like this:

USB 12 MHz Clock Source
Clock config is configured to:
00 B 12 MHz XTAL 36 MHz input divide by 3 (default)
01 B 12 MHz XTAL 36 MHz input is directly provided to USB (This is
reserved for running with 12 MHz XTAL)
10 B 12 MHz 12 MHz USB clock derived from PLL0 300 MHz output as
divide by 25 (for 25 MHz XTAL input, this is only valid option)
11 B 12 MHz XTAL 36 MHz input divide by 3

Bit 1:0 on reg 0x0024, reset value of this bit is 00 B

On xRX200 this depends on the confiuration of the CLK_OUT0, CLK_OUT1 and
CLK_OUT3.


Currently I do not have the documentation of the AR9 at hand, will
search for it later.

>  .../0032-lantiq-set-the-usb-clock-source.patch | 52 
> ++
>  ...MIPS-lantiq-danube-initialize-usb-on-boot.patch |  2 +-
>  .../linux/lantiq/patches-4.4/0047-poweroff.patch   |  4 +-
>  3 files changed, 55 insertions(+), 3 deletions(-)
>  create mode 100644 
> target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch
> 
> diff --git 
> a/target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch 
> b/target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch
> new file mode 100644
> index 000..bfc41aa
> --- /dev/null
> +++ 
> b/target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch
> @@ -0,0 +1,52 @@
> +From 6e6569954319ef5f3e8c6a2c56056a90dd3c4ca0 Mon Sep 17 00:00:00 2001
> +From: Mathias Kresin 
> +Date: Sat, 31 Dec 2016 10:46:18 +0100
> +Subject: [PATCH] MIPS: lantiq: set the usb clock source
> +
> +The clock source is set by the ltq-hcd driver but are not by the
> +dwc2 driver. Wit

Re: [LEDE-DEV] [PATCH] lantiq: set the usb clock source

2017-01-01 Thread Mathias Kresin

01.01.2017 11:21, Mathias Kresin:

The clock source is set by the ltq-hcd driver but are not by the
dwc2 driver. Without having the correct clock set the dwc driver
fails to reset the usb core and errors out. The values for supported
lantiq targets are exactly the same as set by the ltq-hcd driver and
should not cause any regressions.

Fixes FS#351

Signed-off-by: Mathias Kresin 


Failed to set the correct subject prefix. This one is a RFC.

Mathias

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


[LEDE-DEV] [PATCH] lantiq: set the usb clock source

2017-01-01 Thread Mathias Kresin
The clock source is set by the ltq-hcd driver but are not by the
dwc2 driver. Without having the correct clock set the dwc driver
fails to reset the usb core and errors out. The values for supported
lantiq targets are exactly the same as set by the ltq-hcd driver and
should not cause any regressions.

Fixes FS#351

Signed-off-by: Mathias Kresin 
---

Well the code works but is not really a beauty. But I need some input
to _possibly_ improve the code or at least the commit message.

Based on the disabled code in the ltq-hcd driver I guess for all
targets two bits are used to set the clock source.

I know that on vr9 the bits set to 00 or 11 means "XTAL 36 MHz input
devide by 3" where 00 is the default value.

After playing a bit with the bits, I'm the opinion the same applies to
Danube a well.

It's kinda strange that for Amazon only one bit is cleared within the
ltq-hcd driver. It looks more like an error or yet unknown workaround.
Setting the interface clock bits to 00 (default) does make more sense
to me. I've changed this in contrast to the ltq-hcd driver but do not
have any hardware to check this change or the dwc2 at all on Amazon.

For some reason the usb clock source has already the correct value on
vr9. I'm not sure if any vr9 board uses a clock source different than
the default "XTAL 36 MHz input devide by 3". For completeness I tend to
add the clock source for vr9 as well. Any opinions?

Albeit we obviously don't have an alien board which need a different
clock source than the others, setting the clock source to a fixed value
seams to me a really bad idea. And since it is the second time I had to
touch the CGU_IFCCR register [0][1] I've somehow the impression we need
some glue code to set the different clock sources via some kind of glue
code. I'm not sure what is the best way to do this. Should I add
something similar to the lantiq,phy-clk-src? Is there any chance that
this code is accepted upstream or should we consider it as LEDE hack?
Is the common clock framework maybe the saviour?

Would anyone with access to the datasheets be so kind to confirm:

- all lantiq targets are using two bits to set the clock source
- the meaning of the usbclk bit combinations are the same for all
  lantiq targets (including 00 == 11)
- it is safe to set the usb clock to 00 or 11 for Amazon

If 00 == 11 on Amazon/Danube, the lantiq,ase and lantiq,danube
conditions can be merged.

The lantiq,ar9 can be extended to cover vr9 as well.

[0] https://git.lede-project.org/44cace4dabe186a486d6713de6196bc7cea2228b
[1] https://git.lede-project.org/d4de9f72f31c4716f78fea8950261a3bdafdbccb

 .../0032-lantiq-set-the-usb-clock-source.patch | 52 ++
 ...MIPS-lantiq-danube-initialize-usb-on-boot.patch |  2 +-
 .../linux/lantiq/patches-4.4/0047-poweroff.patch   |  4 +-
 3 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 
target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch

diff --git 
a/target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch 
b/target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch
new file mode 100644
index 000..bfc41aa
--- /dev/null
+++ b/target/linux/lantiq/patches-4.4/0032-lantiq-set-the-usb-clock-source.patch
@@ -0,0 +1,52 @@
+From 6e6569954319ef5f3e8c6a2c56056a90dd3c4ca0 Mon Sep 17 00:00:00 2001
+From: Mathias Kresin 
+Date: Sat, 31 Dec 2016 10:46:18 +0100
+Subject: [PATCH] MIPS: lantiq: set the usb clock source
+
+The clock source is set by the ltq-hcd driver but are not by the
+dwc2 driver. Without having the correct clock set the dwc driver
+fails to reset the usb core and errors out. The values for supported
+lantiq targets are exactly the same as set by the ltq-hcd driver and
+should not cause any regressions.
+
+Signed-off-by: Mathias Kresin 
+---
+
+ arch/mips/lantiq/xway/reset.c | 14 ++
+ 1 file changed, 14 insertions(+)
+
+--- a/arch/mips/lantiq/xway/reset.c
 b/arch/mips/lantiq/xway/reset.c
+@@ -95,6 +95,14 @@
+ #define PMU_USB0_PBIT(0)
+ #define PMU_USB1_PBIT(26)
+ 
++/* USB clock */
++#define CGU_IFCCR 0x0018
++#define CGU_USBCLK_MASK   0x3
++#define CGU_USBCLK_DEFAULT0x0
++#define CGU_USBCLK_DIRECT 0x1
++#define CGU_USBCLK_PPL0   0x2
++#define CGU_USBCLK_XTAL   0x3
++
+ /* remapped base addr of the reset control unit */
+ static void __iomem *ltq_rcu_membase;
+ static struct device_node *ltq_rcu_np;
+@@ -317,6 +325,17 @@ static void ltq_usb_init(void)
+   ltq_rcu_w32(ltq_rcu_r32(RCU_CFG1A) | BIT(0), RCU_CFG1A);
+   ltq_rcu_w32(ltq_rcu_r32(RCU_CFG1B) | BIT(0), RCU_CFG1B);
+ 
++  /* set usb clock source */
++  if (of_machine_is_compatible("lantiq,ase"))
++  ltq_cgu_w32((ltq_cgu_r32(CGU_IFCCR) & ~(CGU_USBCLK_MASK << 4))
++  | (CGU_USBCLK_DEFAULT << 4), CGU_IFCCR);
++  else if (of_machine_is_compatible("lantiq,danube"))
++  ltq_cgu_w32(