linux-kernel - RE: [PATCH v2 1/3] usb: phy: add usb phy notify port status API

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: <647ded70ff024a3081cbf5c45f5da12c@realtek.com>
Date:   Tue, 30 May 2023 02:19:45 +0000
From:   Stanley Chang[昌育德] <stanley_chang@...ltek.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     Vinod Koul <vkoul@...nel.org>,
        Kishon Vijay Abraham I <kishon@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Michael Grzeschik <m.grzeschik@...gutronix.de>,
        Bagas Sanjaya <bagasdotme@...il.com>,
        "Flavio Suligoi" <f.suligoi@...m.it>,
        Matthias Kaehlcke <mka@...omium.org>,
        "Mathias Nyman" <mathias.nyman@...ux.intel.com>,
        Ray Chi <raychi@...gle.com>,
        "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: RE: [PATCH v2 1/3] usb: phy: add usb phy notify port status API

Hi Greg,

> > --- a/include/linux/usb/phy.h
> > +++ b/include/linux/usb/phy.h
> > @@ -144,6 +144,10 @@ struct usb_phy {
> >        */
> >       int     (*set_wakeup)(struct usb_phy *x, bool enabled);
> >
> > +     /* notify phy port status change */
> > +     int     (*notify_port_status)(struct usb_phy *x,
> > +             int port, u16 portstatus, u16 portchange);
> > +
> >       /* notify phy connect status change */
> >       int     (*notify_connect)(struct usb_phy *x,
> >                       enum usb_device_speed speed);
> 
> Why can't this be part of the same notify_connect() callback?

The notify connect is at device ready. But I want notify port status change before port reset.

> What makes it different somehow?  Please document this much better.

In Realtek phy driver, we have designed to dynamically adjust disconnection level and calibrate phy parameters.
So we do this when the device connected bit changes and when the disconnected bit changes.
Port status change notification:
1. Check if portstatus is USB_PORT_STAT_CONNECTION and portchange is USB_PORT_STAT_C_CONNECTION.
  The device is connected, the driver lowers the disconnection level and calibrates the phy parameters.
2. The device disconnects, the driver increases the disconnect level and calibrates the phy parameters.

If we adjust the disconnection level in notify_connect , the disconnect may have been triggered at this stage. 
So we need to change that as early as possible.


> 
> > @@ -316,6 +320,16 @@ usb_phy_set_wakeup(struct usb_phy *x, bool
> enabled)
> >               return 0;
> >  }
> >
> > +static inline int
> > +usb_phy_notify_port_status(struct usb_phy *x, int port, u16 portstatus,
> > +         u16 portchange)
> > +{
> > +     if (x && x->notify_port_status)
> 
> How can x ever be NULL?

It is possible.
If the controller not use usb-phy driver. It  is NULL.

Thanks,
Stanley