On Thu, Aug 8, 2024 at 9:38 AM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
On Wed, Aug 07, 2024, Kyle Tso wrote:
On Wed, Aug 7, 2024 at 7:29 AM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
On Sun, Aug 04, 2024, Kyle Tso wrote:
It is possible that the usb power_supply is registered after the probe
Should we defer the dwc3 probe until the power_supply is registered then?
We can do that, but getting the power_supply reference just before using the power_supply APIs is safer because we don't risk waiting for the registration of the usb power_supply. If vbus_draw is being called
I'm a bit confused, wouldn't we need the power_supply to be registered before you can get the reference. Can you clarify the risk here?
I know it's weird, but usb power_supply module is not guaranteed to be loaded while dwc3 is being probed. What if, for example, it requires userspace to manually load the usb power_supply module. If we defer the probe just to wait for the usb power_supply, it might be waiting for a long time.
but the usb power_supply is still not ready, just let it fail without doing anything (only print the error logs). The usb gadget function still works. And once the usb power_supply is ready, the vbus_draw will be fine in following usb state changes.
Moreover, all drivers using power_supply_get_by_name in the source tree adopt this way. IMO it should be okay.
of dwc3. In this case, trying to get the usb power_supply during the probe will fail and there is no chance to try again. Also the usb power_supply might be unregistered at anytime so that the handle of it
This is problematic... If the power_supply is unregistered, the device is no longer usable.
in dwc3 would become invalid. To fix this, get the handle right before calling to power_supply functions and put it afterward.
Shouldn't the life-cycle of the dwc3 match with the power_supply? How can we maintain function without the proper power_supply?
BR, Thinh
usb power_supply is controlled by "another" driver which can be unloaded without notifying other drivers using it (such as dwc3). Unless there is a notification mechanism for the (un)registration of the power_supply class, getting/putting the reference right before/after calling the power_supply api is the best we can do for now.
The power_supply driver should not be able to unload while the dwc3 holds the power_supply handle due to dependency between the two. Why would we want to release the handle while dwc3 still needs it.
It is possible. Calling power_supply_unregister only results in WARN_ON if the use_cnt is not equal to 1.
/** * power_supply_unregister() - Remove this power supply from system * @psy: Pointer to power supply to unregister * * Remove this power supply from the system. The resources of power supply * will be freed here or on last power_supply_put() call. */ void power_supply_unregister(struct power_supply *psy) { WARN_ON(atomic_dec_return(&psy->use_cnt)); ...
This creates an unpredictable behavior where sometime vbus can be drawn and sometime it can't. Your specific gadget function may work for its specific purpose, some other may not as its vbus_draw may be essential for its application.
BR, Thinh
I agree that it might be unpredictable. But If we rely on the power_supply class to control the vbus_draw, it is the risk that we need to take. I believe most of the time the usb power_supply would be there until some specific timing such as shutdown/reboot. This patch is only to handle the small chances that the usb power_supply is unregistered for some weird reason. It is better to give up the vbus_draw than dwc3 accessing an invalid reference.
Kyle