Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
MIME-Version: 1.0
References: <20231130154337.1.Ie00e07f07f87149c9ce0b27ae4e26991d307e14b@changeid>
 <62b7467f-f142-459d-aa23-8bfd70bbe733@rowland.harvard.edu>
In-Reply-To: <62b7467f-f142-459d-aa23-8bfd70bbe733@rowland.harvard.edu>
From:   Doug Anderson <dianders@chromium.org>
Date:   Fri, 1 Dec 2023 10:32:57 -0800
Message-ID: <CAD=FV=VMZGaNdSKAL8o6jtXwmik0aKgO6DdOpe9OvHth9TZf9Q@mail.gmail.com>
Subject: Re: [PATCH] usb: core: Save the config when a device is deauthorized+authorized
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     linux-usb@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Simon Horman <horms@kernel.org>,
        Grant Grundler <grundler@chromium.org>,
        Hayes Wang <hayeswang@realtek.com>,
        =?UTF-8?Q?Bj=C3=B8rn_Mork?= <bjorn@mork.no>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        "David S . Miller" <davem@davemloft.net>,
        Brian Geffon <bgeffon@google.com>,
        Bastien Nocera <hadess@hadess.net>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Flavio Suligoi <f.suligoi@asem.it>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        =?UTF-8?Q?Ricardo_Ca=C3=B1uelo?= <ricardo.canuelo@collabora.com>,
        Rob Herring <robh@kernel.org>, Roy Luo <royluo@google.com>,
        Stanley Chang <stanley_chang@realtek.com>,
        Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

On Fri, Dec 1, 2023 at 7:59=E2=80=AFAM Alan Stern <stern@rowland.harvard.ed=
u> wrote:
>
> On Thu, Nov 30, 2023 at 03:43:47PM -0800, Douglas Anderson wrote:
> > Right now, when a USB device is deauthorized (by writing 0 to the
> > "authorized" field in sysfs) and then reauthorized (by writing a 1) it
> > loses any configuration it might have had. This is because
> > usb_deauthorize_device() calls:
> >   usb_set_configuration(usb_dev, -1);
> > ...and then usb_authorize_device() calls:
> >   usb_choose_configuration(udev);
> > ...to choose the "best" configuration.
> >
> > This generally works OK and it looks like the above design was chosen
> > on purpose. In commit 93993a0a3e52 ("usb: introduce
> > usb_authorize/deauthorize()") we can see some discussion about keeping
> > the old config but it was decided not to bother since we can't save it
> > for wireless USB anyway. It can be noted that as of commit
> > 1e4c574225cc ("USB: Remove remnants of Wireless USB and UWB") wireless
> > USB is removed anyway, so there's really not a good reason not to keep
> > the old config.
> >
> > Unfortunately, throwing away the old config breaks when something has
> > decided to choose a config other than the normal "best" config.
> > Specifically, it can be noted that as of commit ec51fbd1b8a2 ("r8152:
> > add USB device driver for config selection") that the r8152 driver
> > subclasses the generic USB driver and selects a config other than the
> > one that would have been selected by usb_choose_configuration(). This
> > logic isn't re-run after a deauthorize + authorize and results in the
> > r8152 driver not being re-bound.
> >
> > Let's change things to save the old config when we deauthorize and
> > then restore it when we re-authorize. We'll disable this logic for
> > wireless USB where we re-fetch the descriptor after authorization.
>
> Would it be better to make the r8152 driver override
> usb_choose_configuration()?  This is the sort of thing that subclassing
> is intended for.

Yes, this is a nice solution. Posted.

https://lore.kernel.org/r/20231201183113.343256-1-dianders@chromium.org