[PATCH v5 4/6] usb: dwc3: Add Amlogic A1 DWC3 glue

Hanjie Lin hanjie.lin at amlogic.com
Wed Jan 15 18:12:03 PST 2020



On 2020/1/15 16:01, Neil Armstrong wrote:
> On 11/01/2020 21:45, Martin Blumenstingl wrote:
>> Hi Hanjie,
>>
>> On Fri, Jan 10, 2020 at 6:43 AM Hanjie Lin <hanjie.lin at amlogic.com> wrote:
>> [...]
>>> -       devm_add_action_or_reset(dev,
>>> -                                (void(*)(void *))clk_disable_unprepare,
>>> -                                priv->clk);
>>> +       ret = clk_bulk_prepare_enable(priv->drvdata->num_clks,
>>> +                                     priv->drvdata->clks);
>> I don't see clk_bulk_disable_unprepare in dwc3_meson_g12a_remove()
>> please test that the clocks are all disabled (see
>> /sys/kernel/debug/clk/clk_summary for example) when unloading all USB
>> related kernel modules
>>
>>> +
>>> +       if (!priv->drvdata->otg_switch_supported)
>>> +               goto setup_pm_runtime;
>> my brain doesn't like the goto in this place because this is not an
>> error condition. I was about to write that
>> usb_role_switch_unregister() is now skipped even though we're calling
>> usb_role_switch_register().
>>
>> I want to hear Neil's opinion on this because I got confused while
>> reading the code again.
>> my proposal is to move all of this OTG related code from
>> dwc3_meson_g12a_probe() into a new function, for example called
>> dwc3_meson_g12a_otg_init()
>> then only call that function when OTG switching is supported
> 
> Indeed it's not cleanest way to do that, if you respin a v6, put all the OTG
> setup and role switch register in a separate function.
> 
> with that and :clk_bulk_disable_unprepare() in remove:
> Reviewed-by: Neil Armstrong <narmstrong at baylibre.com>
> 
> Neil
> 

Ok, 

I will do it in v6 patch.

Thanks,
Hanjie

>>
>>
>> Martin
>>
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> .
> 



More information about the linux-arm-kernel mailing list