On 02/04/2013 03:24 PM, Sascha Hauer wrote:
> From: Marc Kleine-Budde <m...@pengutronix.de>
>
> In patch "5d3c28b usb: otg: add device tree support to otg library"
> devm_usb_get_phy_by_phandle() was added. It uses try_module_get() to lock the
> phy driver in memory. The corresponding module_put() is missing in that patch.
>
> This patch adds try_module_get() to usb_get_phy() and usb_get_phy_dev().
> Further the missing module_put() is added to usb_put_phy().
>
> Signed-off-by: Marc Kleine-Budde <m...@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.ha...@pengutronix.de>
> ---
> drivers/usb/otg/otg.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index e181439..2bd03d2 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
> spin_lock_irqsave(&phy_lock, flags);
>
> phy = __usb_find_phy(&phy_list, type);
> - if (IS_ERR(phy)) {
> + if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> pr_err("unable to find transceiver of type %s\n",
> usb_phy_type_string(type));
> goto err0;
There are 2 problems with this.
- If phy was found but try_module_get failed you are not returning error code.
- If phy was found and try_module_get fails we would want to use deferred
probing,
since this case is possible if the phy module is not yet loaded and alive but
can be in
the future.
I would change the code to
if (IS_ERR(phy)) {
pr_err("unable to find transceiver of type %s\n",
usb_phy_type_string(type));
goto err0;
}
if (!try_module_get(phy->dev->driver->owner)) {
phy = ERR_PTR(-EPROBE_DEFER);
goto err0;
}
> @@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
> index)
> spin_lock_irqsave(&phy_lock, flags);
>
> phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
> - if (IS_ERR(phy)) {
> + if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> pr_err("unable to find transceiver\n");
> goto err0;
> }
> @@ -301,8 +301,12 @@ EXPORT_SYMBOL(devm_usb_put_phy);
> */
> void usb_put_phy(struct usb_phy *x)
> {
> - if (x)
> + if (x) {
> + struct module *owner = x->dev->driver->owner;
> +
> put_device(x->dev);
> + module_put(owner);
> + }
> }
> EXPORT_SYMBOL(usb_put_phy);
>
>
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
