Re: [PATCH v2] usb: phy: don't overwrite a device_type the bus already set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: phy: mutated device_type suppresses unbind/remove uevents
--

commit 3154a296e918fd840b2f1b31a53c7b6b47a5a85e
Author: Palla Raghunath <raghunathpalla.0209@gmail.com>

usb: phy: don't overwrite a device_type the bus already set

usb_add_phy_dev() replaces the device_type of whatever device the PHY driver
passed in. This hurts i2c clients as they keep their release callback on the
device_type. Only take the device_type if nothing else has been set.

> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
> index 5a9b9353f343d..ded18c7fe32ff 100644
> --- a/drivers/usb/phy/phy.c
> +++ b/drivers/usb/phy/phy.c
> @@ -705,7 +705,13 @@ int usb_add_phy_dev(struct usb_phy *x)
>  	if (ret)
>  		return ret;
>  
> -	x->dev->type = &usb_phy_dev_type;
> +	/*
> +	 * Don't clobber a device_type the bus already set.  x->dev is the
> +	 * PHY driver's own device, and for an i2c client the release
> +	 * callback lives on the type.
> +	 */
> +	if (!x->dev->type)
> +		x->dev->type = &usb_phy_dev_type;

[Severity: Medium]
Will mutating the device type here suppress teardown uevents for platform
devices?

When a platform device driver unbinds, it first removes the PHY and then the
driver core emits uevents:

driver_remove()
  usb_remove_phy(x)
    list_del(&x->head);

device_unbind() / device_remove()
  kobject_uevent_env()
    dev_uevent()
      usb_phy_uevent()

Because usb_remove_phy() does not restore x->dev->type when it deletes the
PHY from phy_list, __device_to_usb_phy() will return NULL during the
teardown uevents.

usb_phy_uevent() then returns -ENODEV:

drivers/usb/phy/phy.c:usb_phy_uevent() {
	...
	spin_lock_irqsave(&phy_lock, flags);
	usb_phy = __device_to_usb_phy(dev);
	spin_unlock_irqrestore(&phy_lock, flags);

	if (!usb_phy)
		return -ENODEV;
	...
}

Does this -ENODEV return abort the kobject_uevent() emission entirely,
silently dropping the unbind and remove uevents?

Could this regression cause userspace tools like udev to miss device teardown
events?

>  
>  	ATOMIC_INIT_NOTIFIER_HEAD(&x->notifier);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011174853.59234-1-raghunathpalla.0209@gmail.com?part=1