[PATCH] usb: dwc3-meson-g12a: add power-on/off of the PHYs

[Marek Vasut]

On 4/20/20 3:56 PM, Neil Armstrong wrote:
> On 20/04/2020 15:52, Marek Vasut wrote:
>> On 4/20/20 3:49 PM, Neil Armstrong wrote:
>>> On 20/04/2020 15:47, Marek Vasut wrote:
>>>> On 4/20/20 3:46 PM, Neil Armstrong wrote:
>>>>> Power on/off the PHYs to enable power to the USB ports, fixing USB support
>>>>> on Khadas VIM3/VIM3L boards.
>>>>>
>>>>> Signed-off-by: Neil Armstrong <narmstrong at baylibre.com>
>>>>> ---
>>>>>  drivers/usb/dwc3/dwc3-meson-g12a.c | 16 ++++++++++++++++
>>>>>  1 file changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
>>>>> index d4453f8784..8f4a2f3f82 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
>>>>> @@ -408,6 +408,15 @@ static int dwc3_meson_g12a_probe(struct udevice *dev)
>>>>>  			goto err_phy_init;
>>>>>  	}
>>>>>  
>>>>> +	for (i = 0 ; i < PHY_COUNT ; ++i) {
>>>>
>>>> Doesn't checkpatch complain about this extra space before semicolon ?
>>>
>>> Nop, even in --strict
>>
>> Shouldn't it though ? I thought this extra space was forbidden.
> 
> No idea, this line is copied from the for loop doing the phy init,
> itself copied from the Linux code passing all checkpatch checks.

Could you change it to for (i = 0; i < PHY_COUNT; i++) please ?

>>>>
>>>>> +		if (!priv->phys[i].dev)
>>>>> +			continue;
>>>>> +
>>>>> +		ret = generic_phy_power_on(&priv->phys[i]);
>>>>
>>>> Do we really need to turn on all the PHYs ?
>>>>
>>>
>>> Yes
>>
>> Then should we have something like clk_bulk_*(), but for phys ?
>>
> 
> Eventually, yes, but this goes beyond a fix.

The merge window is open for quite a bit longer, so doing this in a
generic way would be appreciated.

btw while you're at it, shouldn't the PHY_COUNT be replaced with some
PHY count detection by reading the DT ? ( I don't have the hardware and
I don't know the amlogic SoCs though )