Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20231020083125.1.I3e5f7abcbf6f08d392e31a5826b7f234df662276@changeid>
 <60a358c9-b44b-4d25-9a20-aa9e00c65ab6@rowland.harvard.edu>
In-Reply-To: <60a358c9-b44b-4d25-9a20-aa9e00c65ab6@rowland.harvard.edu>
From:   Doug Anderson <dianders@chromium.org>
Date:   Fri, 20 Oct 2023 08:59:42 -0700
Message-ID: <CAD=FV=VQ22p7iDdzK-CXOBEqpu0b3J=LKRZuPcajQWFUmr=1cA@mail.gmail.com>
Subject: Re: [PATCH] usb: core: Prevent infinite loops when usb_reset_device() unbinds/binds
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, Grant Grundler <grundler@chromium.org>,
        Hayes Wang <hayeswang@realtek.com>,
        Bastien Nocera <hadess@hadess.net>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Flavio Suligoi <f.suligoi@asem.it>,
        Hans de Goede <hdegoede@redhat.com>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Ivan Orlov <ivan.orlov0322@gmail.com>,
        Marc Kleine-Budde <mkl@pengutronix.de>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Ray Chi <raychi@google.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, Oct 20, 2023 at 8:46=E2=80=AFAM Alan Stern <stern@rowland.harvard.e=
du> wrote:
>
> On Fri, Oct 20, 2023 at 08:31:38AM -0700, Douglas Anderson wrote:
> > When we call usb_reset_device() and a driver doesn't implement
> > pre_reset() and post_reset() methods then the USB core will attempt to
> > unbind and rebind the driver in order to make reset work. This is a
> > great general solution, but it has the potential to loop forever.
> > Specifically, if the USB device is in a state that the USB device
> > driver issues another usb_reset_device() after each rebind then we'll
> > just continually unbind and rebind with no end.
> >
> > It's difficult to address this condition in a USB device driver
> > because it's hard for the driver to keep state across each
> > unbind/bind.
>
> How about just adding appropriate pre_reset() and post_reset() methods?
> This is precisely what they are meant for.  Then the the unbind/rebind
> loop wouldn't ever get started.

Right, and we have pre_reset() and post_reset() in the r1852 driver.
The issue is that we are seeing occasional control transfer errors
while the r8152 driver is still running its probe() routine and we
want to reset in response to those. It is relatively difficult to have
the pre_reset() and post_reset() methods work properly if failures
happen and probe() hasn't finished yet. The current proposal I have
for the r8152 driver is to have the pre_reset() routine return -EIO if
we saw errors during probe, which tells the USB core to unbind/rebind
us. This gets us to a known good state. If we need to do a reset later
on (after probe finished successfully) then pre_reset() and
post_reset() can avoid the unbind/bind.

The worry was that this could cause an infinite loop. Hence this patch. ;-)

-Doug