Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 13 May 2021 01:32:25 +0200
From:   Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>
To:     Rajat Jain <rajatja@google.com>
Cc:     Rajat Jain <rajatxjain@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Alan Stern <stern@rowland.harvard.edu>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-pci <linux-pci@vger.kernel.org>,
        "open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" 
        <linux-usb@vger.kernel.org>, Bjorn Helgaas <helgaas@kernel.org>,
        Jesse Barnes <jsbarnes@google.com>,
        Dmitry Torokhov <dtor@google.com>
Subject: Re: [PATCH v2 1/2] driver core: Move the "removable" attribute from
 USB to core
Message-ID: <20210512233225.GA137103@rocinante.localdomain>
References: <20210424021631.1972022-1-rajatja@google.com>
 <20210512010049.GA89346@rocinante.localdomain>
 <CAA93t1ohAFM1U2xTvbd1J1dUCaZwh6GYNGib_AM0J7+qHwSf1A@mail.gmail.com>
 <CACK8Z6HuKqgYQZGJZGQGr5FC96naV+1yXZuwYTy5Ydb5=k40KA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CACK8Z6HuKqgYQZGJZGQGr5FC96naV+1yXZuwYTy5Ydb5=k40KA@mail.gmail.com>
Precedence: bulk

Hi Rajat,

> Posted a v3 of this patch here:
> https://lore.kernel.org/patchwork/patch/1428133/

I saw!  Thank you!

I also found the original conversation around the main idea that's
driving the work done here, as per:

  https://lore.kernel.org/lkml/20200601232542.GA473883@bjorn-Precision-5520/

This helped to fill-in some missing pieces, so to speak, I've bad.

[...]
> > > > @@ -2504,8 +2523,16 @@ static int device_add_attrs(struct device *dev)
> > > >                       goto err_remove_dev_online;
> > > >       }
> > > >
> > > > +     if (type && type->supports_removable) {
> > > > +             error = device_create_file(dev, &dev_attr_removable);
> > > > +             if (error)
> > > > +                     goto err_remove_dev_waiting_for_supplier;
> > > > +     }
> > > > +
> > > >       return 0;
> > >
> > One of my primary considerations was also that the existing UAPI for
> > the USB's "removable" attribute shouldn't be changed. Currently, it
> > exists for all USB devices, so I think the current code / check is OK.
[...]

We wouldn't change addition of this attribute to the USB devices - after
the lift to the device core it still has to work as before, as you say.

Just to clarify.   What I was wondering is whether we should add this new
sysfs object so that every device now would get this attribute going
forward, regardless of whether it would have the "type->supports_removable"
set.

We would then have a lot of devices with this attribute set to
"unknown", and then were the is an actual support it would then be
either "fixed" or "removable".

Having said that, Bjorn pointed out to me that this might be
not necessarily desirable.

We were also wondering if we should only set DEVICE_REMOVABLE for
devices known to be behind an external-facing port, and let everything
else be set to "unknown" (or whatever the default would be).

Krzysztof