lkml.org 
[lkml]   [2021]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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

    \
     
     \ /
      Last update: 2021-06-18 15:18    [W:5.644 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site