[U-Boot] Regression in fastboot on X15 due to CDCE913 enabling

[Vignesh Raghavendra]

Hi Andrew,

On 24/10/19 11:49 PM, Andrew F. Davis wrote:
> On 10/24/19 11:25 AM, Vignesh Raghavendra wrote:
>> Hi Sam,
>>
>> On 24-Oct-19 7:16 PM, Sam Protsenko wrote:
>>> Putting Vignesh to "To:".
>>>
>>> Hi Vignesh,
>>>
>>> Please address Tero's comments below (I've marked with ^^^). Thanks.
>>>
>>> On Thu, Oct 24, 2019 at 3:54 PM Tero Kristo <t-kristo at ti.com> wrote:
>>>>
>>>> On 24/10/2019 13:32, Sam Protsenko wrote:
>>>>> Hi Tero,
>>>>>
>>>>> On Thu, Oct 24, 2019 at 10:32 AM Tero Kristo <t-kristo at ti.com> wrote:
>>>>>>
>>>>>> On 23/10/2019 17:20, Sam Protsenko wrote:
>>>>>>> + fixed U-Boot mailing list address
>>>>>>>
>>>>>>> On Wed, Oct 23, 2019 at 3:13 PM Sam Protsenko
>>>>>>> <semen.protsenko at linaro.org> wrote:
>>>>>>>>
>>>>>>>> Hi Tero,
>>>>>>>>
>>>>>>>> This patch was merged in U-Boot/master recently:
>>>>>>>>
>>>>>>>>       e8e683d33b0c ("board: ti: am57xx-idk: Configure the CDCE913 clock
>>>>>>>> synthesizer")
>>>>>>>>
>>>>>>>> Unfortunately it leads to a regression in fastboot code on BeagleBoard
>>>>>>>> X15. Basically fastboot stops to work, with a message like this:
>>>>>>>>
>>>>>>>>       => fastboot 1
>>>>>>>>       No USB device found
>>>>>>>>       USB init failed: -19
>>>>>>>>
>>>>>>>> I haven't investigated that as I'm busy with Android boot flow patch
>>>>>>>> series at the moment (trying to merge it before merge window closes).
>>>>>>>> Can you please investigate what is the cause and how to fix it?
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>
>>>>>> This seems to be caused by usb dwc3 init failing to get clks, if
>>>>>> CONFIG_CLK is enabled. We appear to have the attached patch which fixes
>>>>>> it in TI internal u-boot tree; it fixes the issue with latest u-boot also.
>>>>>>
>>>>>> Should this be picked for upstream also? (CC: Vignesh)
>>>
>>
>> Yes, this patch is applicable upstream as well. I did not get time to
>> post this series to upstream list.
>>
>>> ^^^
>>>
>>>>>>
>>>>>
>>>>> I can confirm that that patch helps. If it's a proper fix, can you
>>>>> please send it here? Also, I guess it would be nice to have next tags
>>>>> specified: Fixes:, Reported-by:.
>>>>>
>>>>> Thanks!
>>>>
>>>> Vignesh, any comments as you are the author of that patch? The
>>>> background on it is somewhat unclear to me.
>>>>
>>>
>>
>> In case of AM57xx, USB clocks are enabled by board files. Therefore clk
>> and reset APIs may return -ENOENT as nodes may be absent in the DT.
> 
> 
> Would it be better to move these clocks over to DT?
> 

There is no clk infra in U-Boot for AM57xx, all clock management is done
via direct register writes from board file.
Kernel DT does not have clock entries either as clock enabling is taken
care of by "ti,sysc" (previously hwmod) and pm_runtime_* APIs.

> 
>> Since clks and resets are optional, -ENOENT needs to be ignored as well.
>>
> 
> 
> So clocks populated by board config and those that are missing/optional
> will have the same effect? Sounds wrong to me.


Clocks are not populated by board config, but are "enabled" by board
files by writing to those clk registers "directly".
See arch/arm/mach-omap2/omap5/hw_data.c::enable_usb_clocks()

Since DT is "missing" clk entries and since its okay to not have those
clock entries (see Documentation/devicetree/bindings/usb/dwc3.txt in
kernel source, which states its optional), -ENOENT needs to be ignored.

Regards
Vignesh

> Andrew
> 
> 
>>
>>> ^^^
>>>
>>>> -Tero
>>>> --
>>>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>>

-- 
Regards
Vignesh