linux-kernel - Re: [RFCv1 6/8] phy: amlogic: meson8b-usb2: Use phy reset 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: <CANAwSgRqixugUr-t2OheLBVwD=EOkaLqxnGkT-Bx=p_A4Nyr8Q@mail.gmail.com>
Date:   Fri, 18 Jun 2021 21:03:22 +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 6/8] phy: amlogic: meson8b-usb2: Use phy reset callback function

Hi Martin

On Fri, 18 Jun 2021 at 03:54, Martin Blumenstingl
<martin.blumenstingl@...glemail.com> wrote:
>
> Hi Anand,
>
> On Thu, Jun 17, 2021 at 9:44 PM Anand Moon <linux.amoon@...il.com> wrote:
> [...]
> > +static int phy_meson8b_usb2_reset(struct phy *phy)
> > +{
> > +       struct phy_meson8b_usb2_priv *priv = phy_get_drvdata(phy);
> > +       int ret;
> > +
> > +       if (priv->is_enabled) {
> > +               ret = reset_control_reset(priv->reset);
> > +               if (ret) {
> > +                       dev_err(&phy->dev, "Failed to trigger USB reset\n");
> > +                       return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> will this reset change the register values back to some kind of default?
> If it does then we're not re-applying our desired register values
> afterwards - which is probably asking for trouble.
>
> For shared resets (like the one we have here) reset_control_reset will
> only trigger the reset line once until all drivers using that reset
> line are unloaded.
> So effectively this new phy_ops.reset callback will be a no-op.

I know his register is shared between two USB IPs,
but I have not observed any issues.
>
> [...]
> > -       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
> > +       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, "phy");
> I think this breaks compatibility with existing .dtbs and our
> dt-bindings (as we're not documenting a "reset-names" property).
> What is the goal of this one?
>

OK, If we pass NULL over here there is the possibility
USB phy will not get registered.

>
> Best regards,
> Martin

Thanks

-Anand