Re: [LEDE-DEV] [PATCH] lantiq: set the usb clock source
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
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
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(