| Message ID | 1444671232-10253-1-git-send-email-ben.dooks@codethink.co.uk |
|---|---|
| State | Under Review, archived |
| Headers | show |
* Ben Dooks <ben.dooks@codethink.co.uk> [151012 10:38]: > The AM3715 OHCI controller will not function without the EHCI > unit's 120m fclk being enabled. If all the ports in the system > are set to OHCI then the 120m_fclk will not get enabled and no > devices are detected. > > Add a new (optional) property to signal the system must enable > the 120m_fck for OHCI so that if no EHCI ports are signalled > then the 120m_fclk should be enabled. > > We have found no information about why this is necessary, but > it is suspected the EHCI controller does not complete the initial > reset sequence and therefore does not hand control of the USB > port back. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ > drivers/mfd/omap-usb-host.c | 4 ++++ > 2 files changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt > index 38d9bb8..fb5fea5 100644 > --- a/Documentation/devicetree/bindings/usb/omap-usb.txt > +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt > @@ -23,6 +23,9 @@ OMAP MUSB GLUE > Optional properties: > - ctrl-module : phandle of the control module this glue uses to write to > mailbox > + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just > + using ohci. Needed for AM3517 in OHCI only mode. > + > > SOC specific device node entry > usb_otg_hs: usb_otg_hs@4a0ab000 { > diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c > index 1d924d1..13880cf 100644 > --- a/drivers/mfd/omap-usb-host.c > +++ b/drivers/mfd/omap-usb-host.c > @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) > need_logic_fck |= true; > } > > + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ > + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) > + need_logic_fck |= true; > + > if (need_logic_fck) { > omap->ehci_logic_fck = devm_clk_get(dev, > "usbhost_120m_fck"); Hmm why not just use the standard device tree clocks property and then do clk_get_rate() on the clock? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/10/15 18:45, Tony Lindgren wrote: > * Ben Dooks <ben.dooks@codethink.co.uk> [151012 10:38]: >> The AM3715 OHCI controller will not function without the EHCI >> unit's 120m fclk being enabled. If all the ports in the system >> are set to OHCI then the 120m_fclk will not get enabled and no >> devices are detected. >> >> Add a new (optional) property to signal the system must enable >> the 120m_fck for OHCI so that if no EHCI ports are signalled >> then the 120m_fclk should be enabled. >> >> We have found no information about why this is necessary, but >> it is suspected the EHCI controller does not complete the initial >> reset sequence and therefore does not hand control of the USB >> port back. >> >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >> --- >> Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ >> drivers/mfd/omap-usb-host.c | 4 ++++ >> 2 files changed, 7 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt >> index 38d9bb8..fb5fea5 100644 >> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt >> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt >> @@ -23,6 +23,9 @@ OMAP MUSB GLUE >> Optional properties: >> - ctrl-module : phandle of the control module this glue uses to write to >> mailbox >> + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just >> + using ohci. Needed for AM3517 in OHCI only mode. >> + >> >> SOC specific device node entry >> usb_otg_hs: usb_otg_hs@4a0ab000 { >> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c >> index 1d924d1..13880cf 100644 >> --- a/drivers/mfd/omap-usb-host.c >> +++ b/drivers/mfd/omap-usb-host.c >> @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) >> need_logic_fck |= true; >> } >> >> + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ >> + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) >> + need_logic_fck |= true; >> + >> if (need_logic_fck) { >> omap->ehci_logic_fck = devm_clk_get(dev, >> "usbhost_120m_fck"); > > Hmm why not just use the standard device tree clocks property and then do > clk_get_rate() on the clock? I don't see that helps enabling the clock. The code decideds if no EHCI ports in use that it doesn't need to enable the EHCI fclk.
* Ben Dooks <ben.dooks@codethink.co.uk> [151012 11:22]: > On 12/10/15 18:45, Tony Lindgren wrote: > > * Ben Dooks <ben.dooks@codethink.co.uk> [151012 10:38]: > >> The AM3715 OHCI controller will not function without the EHCI > >> unit's 120m fclk being enabled. If all the ports in the system > >> are set to OHCI then the 120m_fclk will not get enabled and no > >> devices are detected. > >> > >> Add a new (optional) property to signal the system must enable > >> the 120m_fck for OHCI so that if no EHCI ports are signalled > >> then the 120m_fclk should be enabled. > >> > >> We have found no information about why this is necessary, but > >> it is suspected the EHCI controller does not complete the initial > >> reset sequence and therefore does not hand control of the USB > >> port back. > >> > >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > >> --- > >> Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ > >> drivers/mfd/omap-usb-host.c | 4 ++++ > >> 2 files changed, 7 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt > >> index 38d9bb8..fb5fea5 100644 > >> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt > >> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt > >> @@ -23,6 +23,9 @@ OMAP MUSB GLUE > >> Optional properties: > >> - ctrl-module : phandle of the control module this glue uses to write to > >> mailbox > >> + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just > >> + using ohci. Needed for AM3517 in OHCI only mode. > >> + > >> > >> SOC specific device node entry > >> usb_otg_hs: usb_otg_hs@4a0ab000 { > >> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c > >> index 1d924d1..13880cf 100644 > >> --- a/drivers/mfd/omap-usb-host.c > >> +++ b/drivers/mfd/omap-usb-host.c > >> @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) > >> need_logic_fck |= true; > >> } > >> > >> + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ > >> + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) > >> + need_logic_fck |= true; > >> + > >> if (need_logic_fck) { > >> omap->ehci_logic_fck = devm_clk_get(dev, > >> "usbhost_120m_fck"); > > > > Hmm why not just use the standard device tree clocks property and then do > > clk_get_rate() on the clock? > > I don't see that helps enabling the clock. The code decideds if > no EHCI ports in use that it doesn't need to enable the EHCI fclk. Right, you need to do clk_prepare_enable() in it first? :) Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/10/15 20:19, Tony Lindgren wrote: > * Ben Dooks <ben.dooks@codethink.co.uk> [151012 11:22]: >> On 12/10/15 18:45, Tony Lindgren wrote: >>> * Ben Dooks <ben.dooks@codethink.co.uk> [151012 10:38]: >>>> The AM3715 OHCI controller will not function without the EHCI >>>> unit's 120m fclk being enabled. If all the ports in the system >>>> are set to OHCI then the 120m_fclk will not get enabled and no >>>> devices are detected. >>>> >>>> Add a new (optional) property to signal the system must enable >>>> the 120m_fck for OHCI so that if no EHCI ports are signalled >>>> then the 120m_fclk should be enabled. >>>> >>>> We have found no information about why this is necessary, but >>>> it is suspected the EHCI controller does not complete the initial >>>> reset sequence and therefore does not hand control of the USB >>>> port back. >>>> >>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >>>> --- >>>> Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ >>>> drivers/mfd/omap-usb-host.c | 4 ++++ >>>> 2 files changed, 7 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt >>>> index 38d9bb8..fb5fea5 100644 >>>> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt >>>> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt >>>> @@ -23,6 +23,9 @@ OMAP MUSB GLUE >>>> Optional properties: >>>> - ctrl-module : phandle of the control module this glue uses to write to >>>> mailbox >>>> + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just >>>> + using ohci. Needed for AM3517 in OHCI only mode. >>>> + >>>> >>>> SOC specific device node entry >>>> usb_otg_hs: usb_otg_hs@4a0ab000 { >>>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c >>>> index 1d924d1..13880cf 100644 >>>> --- a/drivers/mfd/omap-usb-host.c >>>> +++ b/drivers/mfd/omap-usb-host.c >>>> @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) >>>> need_logic_fck |= true; >>>> } >>>> >>>> + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ >>>> + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) >>>> + need_logic_fck |= true; >>>> + >>>> if (need_logic_fck) { >>>> omap->ehci_logic_fck = devm_clk_get(dev, >>>> "usbhost_120m_fck"); >>> >>> Hmm why not just use the standard device tree clocks property and then do >>> clk_get_rate() on the clock? >> >> I don't see that helps enabling the clock. The code decideds if >> no EHCI ports in use that it doesn't need to enable the EHCI fclk. > > Right, you need to do clk_prepare_enable() in it first? :) No, if that was the case the driver would never work for the EHCI case. The issue is: 1) All ports on the system are set to OHCI 2) The omap-usb-host.c does not touch usbhost_120m_fck if no EHCI ports 3) The OHCI fails to detect any devices due to point 2.
Ben, On 13/10/15 11:23, Ben Dooks wrote: > On 12/10/15 20:19, Tony Lindgren wrote: >> * Ben Dooks <ben.dooks@codethink.co.uk> [151012 11:22]: >>> On 12/10/15 18:45, Tony Lindgren wrote: >>>> * Ben Dooks <ben.dooks@codethink.co.uk> [151012 10:38]: >>>>> The AM3715 OHCI controller will not function without the EHCI >>>>> unit's 120m fclk being enabled. If all the ports in the system >>>>> are set to OHCI then the 120m_fclk will not get enabled and no >>>>> devices are detected. >>>>> >>>>> Add a new (optional) property to signal the system must enable >>>>> the 120m_fck for OHCI so that if no EHCI ports are signalled >>>>> then the 120m_fclk should be enabled. >>>>> >>>>> We have found no information about why this is necessary, but >>>>> it is suspected the EHCI controller does not complete the initial >>>>> reset sequence and therefore does not hand control of the USB >>>>> port back. >>>>> >>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >>>>> --- >>>>> Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ >>>>> drivers/mfd/omap-usb-host.c | 4 ++++ >>>>> 2 files changed, 7 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt >>>>> index 38d9bb8..fb5fea5 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt >>>>> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt >>>>> @@ -23,6 +23,9 @@ OMAP MUSB GLUE >>>>> Optional properties: >>>>> - ctrl-module : phandle of the control module this glue uses to write to >>>>> mailbox >>>>> + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just >>>>> + using ohci. Needed for AM3517 in OHCI only mode. >>>>> + >>>>> >>>>> SOC specific device node entry >>>>> usb_otg_hs: usb_otg_hs@4a0ab000 { >>>>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c >>>>> index 1d924d1..13880cf 100644 >>>>> --- a/drivers/mfd/omap-usb-host.c >>>>> +++ b/drivers/mfd/omap-usb-host.c >>>>> @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) >>>>> need_logic_fck |= true; >>>>> } >>>>> >>>>> + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ >>>>> + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) >>>>> + need_logic_fck |= true; >>>>> + >>>>> if (need_logic_fck) { >>>>> omap->ehci_logic_fck = devm_clk_get(dev, >>>>> "usbhost_120m_fck"); >>>> >>>> Hmm why not just use the standard device tree clocks property and then do >>>> clk_get_rate() on the clock? >>> >>> I don't see that helps enabling the clock. The code decideds if >>> no EHCI ports in use that it doesn't need to enable the EHCI fclk. >> >> Right, you need to do clk_prepare_enable() in it first? :) > > No, if that was the case the driver would never work for the EHCI case. > > The issue is: > > 1) All ports on the system are set to OHCI > 2) The omap-usb-host.c does not touch usbhost_120m_fck if no EHCI ports > 3) The OHCI fails to detect any devices due to point 2. > Instead of your existing approach why not just modify the preceeding if condition that sets need_logic_fck to suit the OHCI case. That way you don't need to add a new DT binding. The old assumption was that 120m_fck logic clock is only needed for EHCI mode but it looks like OHCI mode needs it as well. You should also rename omap->ehci_logic_fck to omap->hci_logic_fck as it is no longer ehci specific. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/10/15 09:45, Roger Quadros wrote: > Ben, > > On 13/10/15 11:23, Ben Dooks wrote: >> On 12/10/15 20:19, Tony Lindgren wrote: >>> * Ben Dooks <ben.dooks@codethink.co.uk> [151012 11:22]: >>>> On 12/10/15 18:45, Tony Lindgren wrote: >>>>> * Ben Dooks <ben.dooks@codethink.co.uk> [151012 10:38]: >>>>>> The AM3715 OHCI controller will not function without the EHCI >>>>>> unit's 120m fclk being enabled. If all the ports in the system >>>>>> are set to OHCI then the 120m_fclk will not get enabled and no >>>>>> devices are detected. >>>>>> >>>>>> Add a new (optional) property to signal the system must enable >>>>>> the 120m_fck for OHCI so that if no EHCI ports are signalled >>>>>> then the 120m_fclk should be enabled. >>>>>> >>>>>> We have found no information about why this is necessary, but >>>>>> it is suspected the EHCI controller does not complete the initial >>>>>> reset sequence and therefore does not hand control of the USB >>>>>> port back. >>>>>> >>>>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >>>>>> --- >>>>>> Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ >>>>>> drivers/mfd/omap-usb-host.c | 4 ++++ >>>>>> 2 files changed, 7 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt >>>>>> index 38d9bb8..fb5fea5 100644 >>>>>> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt >>>>>> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt >>>>>> @@ -23,6 +23,9 @@ OMAP MUSB GLUE >>>>>> Optional properties: >>>>>> - ctrl-module : phandle of the control module this glue uses to write to >>>>>> mailbox >>>>>> + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just >>>>>> + using ohci. Needed for AM3517 in OHCI only mode. >>>>>> + >>>>>> >>>>>> SOC specific device node entry >>>>>> usb_otg_hs: usb_otg_hs@4a0ab000 { >>>>>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c >>>>>> index 1d924d1..13880cf 100644 >>>>>> --- a/drivers/mfd/omap-usb-host.c >>>>>> +++ b/drivers/mfd/omap-usb-host.c >>>>>> @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) >>>>>> need_logic_fck |= true; >>>>>> } >>>>>> >>>>>> + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ >>>>>> + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) >>>>>> + need_logic_fck |= true; >>>>>> + >>>>>> if (need_logic_fck) { >>>>>> omap->ehci_logic_fck = devm_clk_get(dev, >>>>>> "usbhost_120m_fck"); >>>>> >>>>> Hmm why not just use the standard device tree clocks property and then do >>>>> clk_get_rate() on the clock? >>>> >>>> I don't see that helps enabling the clock. The code decideds if >>>> no EHCI ports in use that it doesn't need to enable the EHCI fclk. >>> >>> Right, you need to do clk_prepare_enable() in it first? :) >> >> No, if that was the case the driver would never work for the EHCI case. >> >> The issue is: >> >> 1) All ports on the system are set to OHCI >> 2) The omap-usb-host.c does not touch usbhost_120m_fck if no EHCI ports >> 3) The OHCI fails to detect any devices due to point 2. >> > > Instead of your existing approach why not just modify the preceeding > if condition that sets need_logic_fck to suit the OHCI case. > > That way you don't need to add a new DT binding. > > The old assumption was that 120m_fck logic clock is only needed for > EHCI mode but it looks like OHCI mode needs it as well. > > You should also rename omap->ehci_logic_fck to omap->hci_logic_fck > as it is no longer ehci specific. Thanks, will go and have a look at the manual later as didn't see two separate 120m clocks when looking.
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt index 38d9bb8..fb5fea5 100644 --- a/Documentation/devicetree/bindings/usb/omap-usb.txt +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt @@ -23,6 +23,9 @@ OMAP MUSB GLUE Optional properties: - ctrl-module : phandle of the control module this glue uses to write to mailbox + - ti,ohci-needs-120m-fck : bool, enable the 120m ehci clock even if just + using ohci. Needed for AM3517 in OHCI only mode. + SOC specific device node entry usb_otg_hs: usb_otg_hs@4a0ab000 { diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c index 1d924d1..13880cf 100644 --- a/drivers/mfd/omap-usb-host.c +++ b/drivers/mfd/omap-usb-host.c @@ -680,6 +680,10 @@ static int usbhs_omap_probe(struct platform_device *pdev) need_logic_fck |= true; } + /* The AM3517 requries the 120m-fck active to allow the OHCI to work */ + if (of_property_read_bool(dev->of_node, "ti,ohci-needs-120m-fck")) + need_logic_fck |= true; + if (need_logic_fck) { omap->ehci_logic_fck = devm_clk_get(dev, "usbhost_120m_fck");
The AM3715 OHCI controller will not function without the EHCI unit's 120m fclk being enabled. If all the ports in the system are set to OHCI then the 120m_fclk will not get enabled and no devices are detected. Add a new (optional) property to signal the system must enable the 120m_fck for OHCI so that if no EHCI ports are signalled then the 120m_fclk should be enabled. We have found no information about why this is necessary, but it is suspected the EHCI controller does not complete the initial reset sequence and therefore does not hand control of the USB port back. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- Documentation/devicetree/bindings/usb/omap-usb.txt | 3 +++ drivers/mfd/omap-usb-host.c | 4 ++++ 2 files changed, 7 insertions(+)