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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Jun 2021 18:47:03 +0530
From:   Anand Moon <linux.amoon@...il.com>
To:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        linux-phy@...ts.infradead.org,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-amlogic@...ts.infradead.org,
        Linux Kernel <linux-kernel@...r.kernel.org>
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@...glemail.com> wrote:
>
> Hi Anand,
>
> On Thu, Jun 17, 2021 at 9:43 PM Anand Moon <linux.amoon@...il.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