linux-kernel - Re: [PATCH] phy: rockchip-inno-usb2: Sync initial otg state

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: <CAMdYzYqUsr+orVtSbD023=GAyNLg4VdP9Wcrg6gON+Ut9FdzZA@mail.gmail.com>
Date:   Wed, 22 Jun 2022 04:31:39 -0400
From:   Peter Geis <pgwipeout@...il.com>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Heiko Stuebner <heiko@...ech.de>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        "open list:GENERIC PHY FRAMEWORK" <linux-phy@...ts.infradead.org>,
        arm-mail-list <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] phy: rockchip-inno-usb2: Sync initial otg state

On Wed, Jun 22, 2022 at 1:16 AM Samuel Holland <samuel@...lland.org> wrote:
>
> On 6/21/22 7:31 PM, Peter Geis wrote:
> > The initial otg state for the phy defaults to device mode. The actual
> > state isn't detected until an ID IRQ fires. Fix this by syncing the ID
> > state during initialization.
> >
> > Fixes: 51a9b2c03dd3 ("phy: rockchip-inno-usb2: Handle ID IRQ")
> > Signed-off-by: Peter Geis <pgwipeout@...il.com>
>
> One comment below. Either way it fixes a bug, so:
>
> Reviewed-by: Samuel Holland <samuel@...lland.org>

Thanks!

>
> > ---
> >  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > index 6711659f727c..6e44069617df 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > @@ -1162,6 +1162,12 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> >                                       EXTCON_USB_HOST, &rport->event_nb);
> >               if (ret)
> >                       dev_err(rphy->dev, "register USB HOST notifier failed\n");
> > +
> > +             if (!of_property_read_bool(rphy->dev->of_node, "extcon")) {
>
> Currently the IRQ handler does not care who created the extcon. We should at
> least be consistent between here and there.

We care in this instance because if there is an external extcon it is
responsible for setting the initial state. Our ID pin may or may not
be used in this case and if it isn't we would overwrite the initial
state with an invalid one.

>
> Regards,
> Samuel
>
> > +                     /* do initial sync of usb state */
> > +                     ret = property_enabled(rphy->grf, &rport->port_cfg->utmi_id);
> > +                     extcon_set_state_sync(rphy->edev, EXTCON_USB_HOST, !ret);
> > +             }
> >       }
> >
> >  out:
> >
>