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]

Message-ID: <CAFBinCAZn13-GpMw-b4=rd9zwwNCtvmv0Oq0BXt+1PTTQ+Gngw@mail.gmail.com>
Date:   Fri, 18 Jun 2021 14:26:39 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Anand Moon <linux.amoon@...il.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@...ts.infradead.org,
        linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RFCv1 2/8] phy: amlogic: meson8b-usb2: Use phy init callback function

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