LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rajat Jain <rajatja@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dmitry Torokhov <dtor@google.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Mathias Nyman <mathias.nyman@linux.intel.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Chris Chiu <chris.chiu@canonical.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	levinale@google.com, bleung@google.com, rajatxjain@gmail.com,
	jsbarnes@google.com, pmalani@google.com
Subject: Re: [PATCH 2/2] usb: hub: Mark devices downstream a removable hub, as removable
Date: Mon, 4 Oct 2021 15:42:46 -0700	[thread overview]
Message-ID: <CACK8Z6EamamgYExt629gyNrYKpvnu2Gh0eGOOvOa5LH-jnOmaQ@mail.gmail.com> (raw)
In-Reply-To: <YVVLxi/on9x6nfCZ@kroah.com>

+Dmitry Torokhov

Hi Greg, Oliver,

Thanks for taking a look.

On Wed, Sep 29, 2021 at 10:31 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Sep 29, 2021 at 03:48:23PM -0700, Rajat Jain wrote:
> > If a usb device sits below a removable hub, mark the device also as
> > removable. This helps with devices inserted on a standard removable hub or
> > also thunderbold docks, to be shown as removable.
> >
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > ---
> >  drivers/usb/core/hub.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
>
> Combined with the previous patch, you are now marking all devices that
> happen to be attached to a root hub that is on a thunderbolt controller
> as removable.  So all USB devices inside of a docking station are now
> removable?

With this patch, yes that was my intent. I think what we are debating
here is should the "removable" attribute imply possibility of removal
from "the system" or just the "local immediate box" (e.g. thunderbolt
dock). In my mind, the removable property was analogous to imply an
"external device", i.e a device that may be removed from the system,
perhaps as a result of its parent devices being removed from the
system. I guess this definition doesn't match what you believe it
should be?

[Oliver says]
> frankly, why? You are needlessly throwing away information about where
> in the tree
> removal can happen.

I believe you are referring to multi level USB hubs and feel that
"removable" should be set only for devices that hang off a port, and
not for children of such device. I wouldn't necessarily disagree,
pending the discussion above (although I think it applies to this
patch only, I think the previous patch still provides value without
throwing away any info).

As a data point, I notice that with my USB hub, the USB device
representing the hub is correctly marked as "removable", however a USB
device I insert into the USB hub, is shown as "unknown". I don't know
if this is the behavior with all USB hubs or just because my USB hub
has a bug. But my patch helps solve this issue and makes the device
show up as "removable".

Thanks

Rajat

>
> What type of devices did you test this series out with?  And again, what
> problem are you trying to solve?



>
> thanks,
>
> greg k-h

  reply	other threads:[~2021-10-04 22:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 22:48 [PATCH 1/2] usb: hub: Mark root hubs on removable devices, " Rajat Jain
2021-09-29 22:48 ` [PATCH 2/2] usb: hub: Mark devices downstream a removable hub, " Rajat Jain
2021-09-30  5:31   ` Greg Kroah-Hartman
2021-10-04 22:42     ` Rajat Jain [this message]
2021-10-05 14:56       ` Alan Stern
2021-10-05 16:51         ` Dmitry Torokhov
2021-10-05 19:59           ` Alan Stern
2021-10-05 23:43             ` Rajat Jain
2021-10-06  0:41               ` Andrew Lunn
2021-10-06 16:08               ` Alan Stern
2021-10-06  9:37             ` Oliver Neukum
2021-10-06 16:10               ` Alan Stern
2021-10-06 18:36                 ` Oliver Neukum
2021-09-30  5:30 ` [PATCH 1/2] usb: hub: Mark root hubs on removable devices, " Greg Kroah-Hartman
2021-10-04 21:51   ` Rajat Jain
2021-09-30  8:02 ` Oliver Neukum
2021-10-04 21:56   ` Rajat Jain
2021-10-05 11:19 ` Greg Kroah-Hartman
2021-10-05 23:49   ` Rajat Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACK8Z6EamamgYExt629gyNrYKpvnu2Gh0eGOOvOa5LH-jnOmaQ@mail.gmail.com \
    --to=rajatja@google.com \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=andrew@lunn.ch \
    --cc=bleung@google.com \
    --cc=chris.chiu@canonical.com \
    --cc=dtor@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jsbarnes@google.com \
    --cc=levinale@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=pmalani@google.com \
    --cc=rajatxjain@gmail.com \
    --cc=stern@rowland.harvard.edu \
    --subject='Re: [PATCH 2/2] usb: hub: Mark devices downstream a removable hub, as removable' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).