LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rajat Jain <rajatxjain@gmail.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Dmitry Torokhov <dtor@google.com>,
	Rajat Jain <rajatja@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	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, jsbarnes@google.com,
	pmalani@google.com
Subject: Re: [PATCH 2/2] usb: hub: Mark devices downstream a removable hub, as removable
Date: Tue, 5 Oct 2021 16:43:33 -0700	[thread overview]
Message-ID: <CAA93t1qzJuN8M2zbs+Kt9JXWP1H2kjKSxBp8-TXEfaMeZ1iggQ@mail.gmail.com> (raw)
In-Reply-To: <20211005195929.GA634685@rowland.harvard.edu>

Hello,

On Tue, Oct 5, 2021 at 12:59 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Tue, Oct 05, 2021 at 09:51:02AM -0700, Dmitry Torokhov wrote:
> > Hi Alan,
> >
> > On Tue, Oct 5, 2021 at 7:56 AM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > As I understand it, the "removable" property refers specifically to
> > > the device's upstream link, not to whether _any_ of the links leading
> > > from the device to the computer could be removed.
> >
> > No, that is not what it means. I'll cite our sysfs ABI:
> >
> > What:           /sys/devices/.../removable
> > Date:           May 2021
> > Contact:        Rajat Jain <rajatxjain@gmail.com>
> > Description:
> >                 Information about whether a given device can be removed from the
> >                 platform by the user. This is determined by its subsystem in a
> >                 bus / platform-specific way. This attribute is only present for
> >                 devices that can support determining such information:
> >
> >                 "removable": device can be removed from the platform by the user
> >                 "fixed":     device is fixed to the platform / cannot be removed
> >                              by the user.
> >                 "unknown":   The information is unavailable / cannot be deduced.
> >
> >                 Currently this is only supported by USB (which infers the
> >                 information from a combination of hub descriptor bits and
> >                 platform-specific data such as ACPI) and PCI (which gets this
> >                 from ACPI / device tree).
> >
> > It specifically talks about _platform_, not about properties of some
> > peripheral attached to a system. Note that the wording is very similar
> > to what we had for USB devices that originally implemented "removable"
> > attribute:
>
> In that case, shouldn't Rajat's patch change go into the driver core
> rather than the hub driver?  _Every_ device downstream from a
> removable link should count as removable, yes?  Not just the USB
> devices.

I have no preference either way, and can do that if that is more acceptable.

>
> And to say that the attribute is supported only by USB and PCI is
> misleading, since it applies to every device downstream from a
> removable link.

However I do think it makes sense to have the bus control whether this
attribute applies to it or not. Determining the first point in a
hierarchy of devices, where a device can be removed is highly bus
specific (set_usb_port_removable()).

AFAIK, the primary reason / use of this attribute was to distinguish
devices that can be removed by the user, and really all such devices
(at least the ones that matter to user) today sit either on PCI or USB
bus. We intend to use this attribute to segregate internal devices
from external devices, and collect some statistics about usb device
usage this way. There is also a VM case that I think Dmitry or Benson
on this thread can elaborate more about. There seem to be hundreds of
other bus types and I'm not sure if we want to unnecessarily flood the
sysfs with a removable attribute under each device.

Thanks & Best Regards,

Rajat

>
> > > This is probably what Oliver meant when he complained about losing
> > > information.  With the knowledge of whether each individual link is
> > > removable, you can easily tell whether there's some way to remove a
> > > device from the system.  But if you only know whether the device is
> > > removable from the system overall, you generally can't tell whether
> > > the link to the device's parent is removable.
> >
> > If we need this data then we need to establish some new attribute to
> > convey this info.
>
> I don't know if we need it, but such an attribute seems like a good
> idea.
>
> Alan Stern

  reply	other threads:[~2021-10-05 23: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
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 [this message]
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=CAA93t1qzJuN8M2zbs+Kt9JXWP1H2kjKSxBp8-TXEfaMeZ1iggQ@mail.gmail.com \
    --to=rajatxjain@gmail.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=rajatja@google.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).