[PATCH/RFT v2 07/17] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable

Sekhar Nori nsekhar at ti.com
Wed Oct 26 02:30:20 PDT 2016


On Tuesday 25 October 2016 09:35 PM, David Lechner wrote:
> On 10/25/2016 05:12 AM, Sekhar Nori wrote:
>> On Monday 24 October 2016 10:16 PM, ahaslam at baylibre.com wrote:
>>> diff --git a/arch/arm/mach-davinci/usb-da8xx.c
>>> b/arch/arm/mach-davinci/usb-da8xx.c
>>> index 9e41a7f..982e105 100644
>>> --- a/arch/arm/mach-davinci/usb-da8xx.c
>>> +++ b/arch/arm/mach-davinci/usb-da8xx.c
>>> @@ -53,11 +53,19 @@ int __init da8xx_register_usb_refclkin(int rate)
>>>
>>>  static void usb20_phy_clk_enable(struct clk *clk)
>>>  {
>>> +    struct clk *usb20_clk;
>>>      u32 val;
>>>      u32 timeout = 500000; /* 500 msec */
>>>
>>>      val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>>>
>>> +    usb20_clk = clk_get(NULL, "usb20");
>>
>> We should not be using a NULL device pointer here. Can you pass the musb
>> device pointer available in the same file? Also, da850_clks[] in da850.c
>> needs to be fixed to add the matching device name.
> 
> This clock can be used for usb 1.1 PHY even when musb is not being used,
> so I don't think we can depend on having a musb device here.

Replied to this against the same question in v6 patch series you posted.

> Also, in a previous review, it was decided that the usb clocks should
> *not* be added to da850_clks[] [1]. Instead, they are dynamically
> registered elsewhere.

Thats only the USB phy clocks since there is associated
enable()/disable()/set_parent() code which is better kept outside of
da850.c for readability. No other reason. Lookup for the USB 2.0 module
clock is already present in da850.c

Thanks,
Sekhar



More information about the linux-arm-kernel mailing list