Hi Martin,

On Fri, 18 Jun 2021 at 17:56, Martin Blumenstingl
[off-list ref] wrote:

Hi Anand,

On Thu, Jun 17, 2021 at 9:43 PM Anand Moon [off-list ref] wrote:

quoted

Reorder the code for bulk clk_enable into .init callback function.

Depending on whether changes are made to the dwc2 driver this is
either fine or might cause some issues.
My understanding is that drivers which are using a PHY should use the
following code-flow:
- phy_init
- phy_power_on
- phy_power_off
- phy_exit

dwc2's platform.c [0] however currently uses the following order:
- phy_init
- phy_power_on
- (remaining ones omitted to keep this example short)

So with this patch and the way dwc2 is currently implemented the
phy_meson8b_usb2_power_on implementation might not work anymore.
That is because the clocks are turned off and in this case all
registers will read 0.
You might be lucky that u-boot left the clocks enabled for your case -
but in general I would not rely on this.

In case of the phy-meson-gxl-usb2 PHY driver the ordering is different
in the "consumer" driver (dwc3-meson-g12a.c).
There the order is:
- phy_init
- phy_power_on
- (remaining ones omitted to keep this example short)

I don't know if the order in the dwc2 driver can be changed easily
(without breaking other platforms).
Until that dwc2 driver issue is resolved I suggest not applying this
patch (even though in general I like the approach).
The same thing also applies for patch #3 from this series (for
implementing phy_ops.exit)


Best regards,
Martin


[0] https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc2/platform.c#L157

Ok got your point of view.
Thanks for your review comments.

Thanks
-Anand

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel