Re: [PATCH] usb: roles: Cakk try_module_get() from usb_role_switch_find_by_fwnode()

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Thu, Apr 08, 2021 at 04:09:04PM -0700, Guenter Roeck wrote:
> On Thu, Apr 08, 2021 at 10:36:11PM +0200, Hans de Goede wrote:
> > usb_role_switch_find_by_fwnode() returns a reference to the role-switch
> > which must be put by calling usb_role_switch_put().
> > 
> > usb_role_switch_put() calls module_put(sw->dev.parent->driver->owner),
> > add a matching try_module_get() to usb_role_switch_find_by_fwnode(),
> > making it behave the same as the other usb_role_switch functions
> > which return a reference.
> > 
> > This avoids a WARN_ON being hit at kernel/module.c:1158 due to the
> > module-refcount going below 0.
> > 
> 
> Took me a while to figure out what the subject line is supposed
> to mean.
> 
> s/Cakk/Call/
> 
> Otherwise
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> It might be useful though to explain the difference between
> fwnode_usb_role_switch_get() and usb_role_switch_find_by_fwnode(),
> and why two different functions are needed, both passing fwnode
> as parameter and returning a pointer to usb_role_switch.

Yes, the function names are confusing indeed. My proposal is to rename
usb_role_switch_find_by_fwnode() to fwnode_to_usb_role_switch().

I can prepare a patch for that if you guys are OK with it, or Hans,
would you prefer to send that together with this one?

Actually, shouldn't this be marked as a fix?


thanks,

-- 
heikki