Re: [RFCv1 2/8] phy: amlogic: meson8b-usb2: Use phy init callback function

[Anand Moon <linux.amoon@gmail.com>]

Hi Martin,

On Fri, 18 Jun 2021 at 17:56, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Thu, Jun 17, 2021 at 9:43 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > 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