diff mbox

mfd: omap-usb-host: AM3715 OHCI needs 120m functional clock

Message ID 1444671232-10253-1-git-send-email-ben.dooks@codethink.co.uk
State New, archived
Headers show

Commit Message

Ben Dooks Oct. 12, 2015, 5:33 p.m. UTC
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(+)

Comments

Tony Lindgren Oct. 12, 2015, 5:45 p.m. UTC | #1
* 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
Ben Dooks Oct. 12, 2015, 6:18 p.m. UTC | #2
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.
Tony Lindgren Oct. 12, 2015, 7:19 p.m. UTC | #3
* 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
Ben Dooks Oct. 13, 2015, 8:23 a.m. UTC | #4
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.
Roger Quadros Oct. 13, 2015, 8:45 a.m. UTC | #5
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
Ben Dooks Oct. 13, 2015, 8:50 a.m. UTC | #6
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 mbox

Patch

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");