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
