Messages in this thread | | | From | Anand Moon <> | Date | Fri, 18 Jun 2021 18:47:03 +0530 | Subject | Re: [RFCv1 2/8] phy: amlogic: meson8b-usb2: Use phy init callback function |
| |
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
| |