Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 17 Apr 2019 12:31:52 -0600
From:   Raul Rangel <rrangel@chromium.org>
To:     Alan Stern <stern@rowland.harvard.edu>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, groeck@chromium.org, oneukum@suse.com,
        djkurtz@chromium.org, zwisler@chromium.org,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
        Dmitry Torokhov <dtor@chromium.org>,
        Suwan Kim <suwan.kim027@gmail.com>,
        linux-kernel@vger.kernel.org,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Johan Hovold <johan@kernel.org>,
        Mathias Nyman <mathias.nyman@linux.intel.com>,
        Raul Rangel <rrangel@chromium.org>
Subject: Re: [PATCH v2] usb/hcd: Send a uevent signaling that the host
 controller has died
Message-ID: <20190417183152.GA118957@google.com>
References: <20190417175300.GA74282@google.com>
 <Pine.LNX.4.44L0.1904171419260.1400-100000@iolanthe.rowland.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Pine.LNX.4.44L0.1904171419260.1400-100000@iolanthe.rowland.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 17, 2019 at 02:24:06PM -0400, Alan Stern wrote:
> On Wed, 17 Apr 2019, Raul Rangel wrote:
> 
> > > > Also where would be a good place to document this?
> > > 
> > > Documentation/ABI/ is a good start.
> > I'll add something to Documentation/ABI/testing/xhci-uevent
> 
> Your patch will apply to all host controllers, not just xhci.  The
> documentation filename should reflect this.  Perhaps "usb-uevent"?
I realized that after I had sent the email :) I'll do usb-uevent.
> 
> > > Why do you need to lock something that is "dead"?  And why is the idr
> > > lock the correct one here?
> > We need to ensure that root_hub is not null. Though I'm not sure the
> > lock is entirely necessary in this case. usb_remove_hcd stops the work
> > item before it sets the rhdev to null. The reason I picked
> > usb_bus_idr_lock was because it's the same lock that usb_remove_hcd uses
> > when setting rhdev = NULL.
> > 
> > Alan, what do you think? Should I remove the lock?
> 
> You're both right; the lock isn't needed because the work is stopped
> before the root hub gets removed.  Acquiring the lock doesn't do any
> harm, but it isn't needed so you probably should remove it.  In fact, 
> you don't even need to test for whether hcd->self.root_hub is 
> non-NULL.
Sounds good, I'll clean it up.
> 
> Alan Stern
> 
>