Re: [PATCH v5 2/3] usb: otg: support for multiple transceivers by a single controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/12/2012 10:52 AM, Kishon Vijay Abraham I wrote:
> Add a linked list for keeping multiple PHY instances with different
> types so that we can have separate USB2 and USB3 PHYs on one single
> board. _get_phy_ has been changed so that the controller gets
> the transceiver by type. _remove_phy_ has been added to let the phy
> be removed from the phy list.

drivers/usb/otg/otg.c:106:16: warning: context imbalance in 'usb_get_phy' - wrong count at exit

See below:
 
> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> ---
>  drivers/usb/musb/omap2430.c     |    2 +-
>  drivers/usb/otg/ab8500-usb.c    |    4 +-
>  drivers/usb/otg/fsl_otg.c       |    4 +-
>  drivers/usb/otg/gpio_vbus.c     |    4 +-
>  drivers/usb/otg/isp1301_omap.c  |    4 +-
>  drivers/usb/otg/msm_otg.c       |    4 +-
>  drivers/usb/otg/mv_otg.c        |    4 +-
>  drivers/usb/otg/nop-usb-xceiv.c |    4 +-
>  drivers/usb/otg/otg.c           |   96 ++++++++++++++++++++++++++++++++++-----
>  drivers/usb/otg/twl4030-usb.c   |    2 +-
>  drivers/usb/otg/twl6030-usb.c   |    2 +-
>  include/linux/usb/otg.h         |   29 +++++++++++-
>  12 files changed, 127 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index e16dbbf..e279cf3 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -292,7 +292,7 @@ static int omap2430_musb_init(struct musb *musb)
>  	 * up through ULPI.  TWL4030-family PMICs include one,
>  	 * which needs a driver, drivers aren't always needed.
>  	 */
> -	musb->xceiv = usb_get_phy();
> +	musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
>  	if (!musb->xceiv) {
>  		pr_err("HS USB OTG: no transceiver configured\n");
>  		return -ENODEV;
> diff --git a/drivers/usb/otg/ab8500-usb.c b/drivers/usb/otg/ab8500-usb.c
> index 672e28c..ae8ad56 100644
> --- a/drivers/usb/otg/ab8500-usb.c
> +++ b/drivers/usb/otg/ab8500-usb.c
> @@ -529,7 +529,7 @@ static int __devinit ab8500_usb_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto fail0;
>  
> -	err = usb_add_phy(&ab->phy);
> +	err = usb_add_phy(&ab->phy, USB_PHY_TYPE_USB2);
>  	if (err) {
>  		dev_err(&pdev->dev, "Can't register transceiver\n");
>  		goto fail1;
> @@ -556,7 +556,7 @@ static int __devexit ab8500_usb_remove(struct platform_device *pdev)
>  
>  	cancel_work_sync(&ab->phy_dis_work);
>  
> -	usb_add_phy(NULL);
> +	usb_remove_phy(&ab->phy);
>  
>  	ab8500_usb_host_phy_dis(ab);
>  	ab8500_usb_peri_phy_dis(ab);
> diff --git a/drivers/usb/otg/fsl_otg.c b/drivers/usb/otg/fsl_otg.c
> index 73561ed..b68b610 100644
> --- a/drivers/usb/otg/fsl_otg.c
> +++ b/drivers/usb/otg/fsl_otg.c
> @@ -806,7 +806,7 @@ static int fsl_otg_conf(struct platform_device *pdev)
>  	fsl_otg_dev = fsl_otg_tc;
>  
>  	/* Store the otg transceiver */
> -	status = usb_add_phy(&fsl_otg_tc->phy);
> +	status = usb_add_phy(&fsl_otg_tc->phy, USB_PHY_TYPE_USB2);
>  	if (status) {
>  		pr_warn(FSL_OTG_NAME ": unable to register OTG transceiver.\n");
>  		goto err;
> @@ -1134,7 +1134,7 @@ static int __devexit fsl_otg_remove(struct platform_device *pdev)
>  {
>  	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
>  
> -	usb_add_phy(NULL);
> +	usb_remove_phy(&fsl_otg_dev->phy);
>  	free_irq(fsl_otg_dev->irq, fsl_otg_dev);
>  
>  	iounmap((void *)usb_dr_regs);
> diff --git a/drivers/usb/otg/gpio_vbus.c b/drivers/usb/otg/gpio_vbus.c
> index 9b3c264..a67ffe2 100644
> --- a/drivers/usb/otg/gpio_vbus.c
> +++ b/drivers/usb/otg/gpio_vbus.c
> @@ -320,7 +320,7 @@ static int __init gpio_vbus_probe(struct platform_device *pdev)
>  	}
>  
>  	/* only active when a gadget is registered */
> -	err = usb_add_phy(&gpio_vbus->phy);
> +	err = usb_add_phy(&gpio_vbus->phy, USB_PHY_TYPE_USB2);
>  	if (err) {
>  		dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
>  			err);
> @@ -354,7 +354,7 @@ static int __exit gpio_vbus_remove(struct platform_device *pdev)
>  	cancel_delayed_work_sync(&gpio_vbus->work);
>  	regulator_put(gpio_vbus->vbus_draw);
>  
> -	usb_add_phy(NULL);
> +	usb_remove_phy(&gpio_vbus->phy);
>  
>  	free_irq(gpio_vbus->irq, pdev);
>  	if (gpio_is_valid(pdata->gpio_pullup))
> diff --git a/drivers/usb/otg/isp1301_omap.c b/drivers/usb/otg/isp1301_omap.c
> index 70f9950..9b8bfc1 100644
> --- a/drivers/usb/otg/isp1301_omap.c
> +++ b/drivers/usb/otg/isp1301_omap.c
> @@ -1610,7 +1610,7 @@ isp1301_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
>  	dev_dbg(&i2c->dev, "scheduled timer, %d min\n", TIMER_MINUTES);
>  #endif
>  
> -	status = usb_add_phy(&isp->phy);
> +	status = usb_add_phy(&isp->phy, USB_PHY_TYPE_USB2);
>  	if (status < 0)
>  		dev_err(&i2c->dev, "can't register transceiver, %d\n",
>  			status);
> @@ -1649,7 +1649,7 @@ subsys_initcall(isp_init);
>  static void __exit isp_exit(void)
>  {
>  	if (the_transceiver)
> -		usb_add_phy(NULL);
> +		usb_remove_phy(&the_transceiver->phy);
>  	i2c_del_driver(&isp1301_driver);
>  }
>  module_exit(isp_exit);
> diff --git a/drivers/usb/otg/msm_otg.c b/drivers/usb/otg/msm_otg.c
> index 037d87c..dd281ca 100644
> --- a/drivers/usb/otg/msm_otg.c
> +++ b/drivers/usb/otg/msm_otg.c
> @@ -1555,7 +1555,7 @@ static int __init msm_otg_probe(struct platform_device *pdev)
>  	phy->otg->set_host = msm_otg_set_host;
>  	phy->otg->set_peripheral = msm_otg_set_peripheral;
>  
> -	ret = usb_add_phy(&motg->phy);
> +	ret = usb_add_phy(&motg->phy, USB_PHY_TYPE_USB2);
>  	if (ret) {
>  		dev_err(&pdev->dev, "usb_set_transceiver failed\n");
>  		goto free_irq;
> @@ -1624,7 +1624,7 @@ static int __devexit msm_otg_remove(struct platform_device *pdev)
>  	device_init_wakeup(&pdev->dev, 0);
>  	pm_runtime_disable(&pdev->dev);
>  
> -	usb_add_phy(NULL);
> +	usb_remove_phy(phy);
>  	free_irq(motg->irq, motg);
>  
>  	/*
> diff --git a/drivers/usb/otg/mv_otg.c b/drivers/usb/otg/mv_otg.c
> index 18e90fe..1c5ed8e 100644
> --- a/drivers/usb/otg/mv_otg.c
> +++ b/drivers/usb/otg/mv_otg.c
> @@ -690,7 +690,7 @@ int mv_otg_remove(struct platform_device *pdev)
>  	for (clk_i = 0; clk_i <= mvotg->clknum; clk_i++)
>  		clk_put(mvotg->clk[clk_i]);
>  
> -	usb_add_phy(NULL);
> +	usb_remove_phy(&mvotg->phy);
>  	platform_set_drvdata(pdev, NULL);
>  
>  	kfree(mvotg->phy.otg);
> @@ -880,7 +880,7 @@ static int mv_otg_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_set_transceiver:
> -	usb_add_phy(NULL);
> +	usb_remove_phy(&mvotg->phy);
>  err_free_irq:
>  	free_irq(mvotg->irq, mvotg);
>  err_disable_clk:
> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
> index 33000da..803f958 100644
> --- a/drivers/usb/otg/nop-usb-xceiv.c
> +++ b/drivers/usb/otg/nop-usb-xceiv.c
> @@ -117,7 +117,7 @@ static int __devinit nop_usb_xceiv_probe(struct platform_device *pdev)
>  	nop->phy.otg->set_host		= nop_set_host;
>  	nop->phy.otg->set_peripheral	= nop_set_peripheral;
>  
> -	err = usb_add_phy(&nop->phy);
> +	err = usb_add_phy(&nop->phy, USB_PHY_TYPE_USB2);
>  	if (err) {
>  		dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
>  			err);
> @@ -139,7 +139,7 @@ static int __devexit nop_usb_xceiv_remove(struct platform_device *pdev)
>  {
>  	struct nop_usb_xceiv *nop = platform_get_drvdata(pdev);
>  
> -	usb_add_phy(NULL);
> +	usb_remove_phy(&nop->phy);
>  
>  	platform_set_drvdata(pdev, NULL);
>  	kfree(nop->phy.otg);
> diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
> index 300a995..a230658 100644
> --- a/drivers/usb/otg/otg.c
> +++ b/drivers/usb/otg/otg.c
> @@ -11,14 +11,32 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/export.h>
> +#include <linux/err.h>
>  #include <linux/device.h>
>  
>  #include <linux/usb/otg.h>
>  
> -static struct usb_phy *phy;
> +static LIST_HEAD(phy_list);
> +static DEFINE_SPINLOCK(phy_lock);
> +
> +static struct usb_phy *__usb_find_phy(struct list_head *list,
> +	enum usb_phy_type type)
> +{
> +	struct usb_phy  *phy = NULL;
> +
> +	list_for_each_entry(phy, list, head) {
> +		if (phy->type != type)
> +			continue;
> +
> +		return phy;
> +	}
> +
> +	return ERR_PTR(-ENODEV);
> +}
>  
>  /**
> - * usb_get_phy - find the (single) USB PHY
> + * usb_get_phy - find the USB PHY
> + * @type - the type of the phy the controller requires
>   *
>   * Returns the phy driver, after getting a refcount to it; or
>   * null if there is no such phy.  The caller is responsible for
> @@ -26,16 +44,30 @@ static struct usb_phy *phy;
>   *
>   * For use by USB host and peripheral drivers.
>   */
> -struct usb_phy *usb_get_phy(void)
> +struct usb_phy *usb_get_phy(enum usb_phy_type type)
>  {
> -	if (phy)
> -		get_device(phy->dev);
> +	struct usb_phy	*phy = NULL;
> +	unsigned long	flags;
> +
> +	spin_lock_irqsave(&phy_lock, flags);
> +
> +	phy = __usb_find_phy(&phy_list, type);
> +	if (IS_ERR(phy)) {
> +		pr_err("unable to find transceiver of type %s\n",
> +			usb_phy_type_string(type));

missing spin_unlock_irqrestore(&phy_lock, flags);
(or goto exit)

Marc

> +		return phy;
> +	}
> +
> +	get_device(phy->dev);
> +
> +	spin_unlock_irqrestore(&phy_lock, flags);
> +
>  	return phy;
>  }
>  EXPORT_SYMBOL(usb_get_phy);
>  
>  /**
> - * usb_put_phy - release the (single) USB PHY
> + * usb_put_phy - release the USB PHY
>   * @x: the phy returned by usb_get_phy()
>   *
>   * Releases a refcount the caller received from usb_get_phy().
> @@ -50,22 +82,62 @@ void usb_put_phy(struct usb_phy *x)
>  EXPORT_SYMBOL(usb_put_phy);
>  
>  /**
> - * usb_add_phy - declare the (single) USB PHY
> + * usb_add_phy - declare the USB PHY
>   * @x: the USB phy to be used; or NULL
> + * @type - the type of this PHY
>   *
>   * This call is exclusively for use by phy drivers, which
>   * coordinate the activities of drivers for host and peripheral
>   * controllers, and in some cases for VBUS current regulation.
>   */
> -int usb_add_phy(struct usb_phy *x)
> +int usb_add_phy(struct usb_phy *x, enum usb_phy_type type)
>  {
> -	if (phy && x)
> -		return -EBUSY;
> -	phy = x;
> -	return 0;
> +	int		ret = 0;
> +	unsigned long	flags;
> +	struct usb_phy	*phy;
> +
> +	if (x && x->type != USB_PHY_TYPE_UNDEFINED) {
> +		dev_err(x->dev, "not accepting initialized PHY %s\n", x->label);
> +		return -EINVAL;
> +	}
> +
> +	spin_lock_irqsave(&phy_lock, flags);
> +
> +	list_for_each_entry(phy, &phy_list, head) {
> +		if (phy->type == type) {
> +			ret = -EBUSY;
> +			dev_err(x->dev, "transceiver type %s already exists\n",
> +						usb_phy_type_string(type));
> +			goto out;
> +		}
> +	}
> +
> +	x->type = type;
> +	list_add_tail(&x->head, &phy_list);
> +
> +out:
> +	spin_unlock_irqrestore(&phy_lock, flags);
> +	return ret;
>  }
>  EXPORT_SYMBOL(usb_add_phy);
>  
> +/**
> + * usb_remove_phy - remove the OTG PHY
> + * @x: the USB OTG PHY to be removed;
> + *
> + * This reverts the effects of usb_add_phy
> + */
> +void usb_remove_phy(struct usb_phy *x)
> +{
> +	unsigned long	flags;
> +
> +	spin_lock_irqsave(&phy_lock, flags);
> +	if (x)
> +		list_del(&x->head);
> +	spin_unlock_irqrestore(&phy_lock, flags);
> +}
> +EXPORT_SYMBOL(usb_remove_phy);
> +
>  const char *otg_state_string(enum usb_otg_state state)
>  {
>  	switch (state) {
> diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
> index 3e313fc..f8139b3 100644
> --- a/drivers/usb/otg/twl4030-usb.c
> +++ b/drivers/usb/otg/twl4030-usb.c
> @@ -633,7 +633,7 @@ static int __devinit twl4030_usb_probe(struct platform_device *pdev)
>  		kfree(twl);
>  		return err;
>  	}
> -	usb_add_phy(&twl->phy);
> +	usb_add_phy(&twl->phy, USB_PHY_TYPE_USB2);
>  
>  	platform_set_drvdata(pdev, twl);
>  	if (device_create_file(&pdev->dev, &dev_attr_vbus))
> diff --git a/drivers/usb/otg/twl6030-usb.c b/drivers/usb/otg/twl6030-usb.c
> index a8be208..dbee00a 100644
> --- a/drivers/usb/otg/twl6030-usb.c
> +++ b/drivers/usb/otg/twl6030-usb.c
> @@ -443,7 +443,7 @@ static int __devinit twl6030_usb_probe(struct platform_device *pdev)
>  		kfree(twl);
>  		return err;
>  	}
> -	usb_add_phy(&twl->phy);
> +	usb_add_phy(&twl->phy, USB_PHY_TYPE_USB2);
>  
>  	platform_set_drvdata(pdev, twl);
>  	if (device_create_file(&pdev->dev, &dev_attr_vbus))
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index 0e739c8..1def65f 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -43,6 +43,13 @@ enum usb_phy_events {
>  	USB_EVENT_ENUMERATED,   /* gadget driver enumerated */
>  };
>  
> +/* associate a type with PHY */
> +enum usb_phy_type {
> +	USB_PHY_TYPE_UNDEFINED,
> +	USB_PHY_TYPE_USB2,
> +	USB_PHY_TYPE_USB3,
> +};
> +
>  struct usb_phy;
>  
>  /* for transceivers connected thru an ULPI interface, the user must
> @@ -89,6 +96,7 @@ struct usb_phy {
>  	const char		*label;
>  	unsigned int		 flags;
>  
> +	enum usb_phy_type	type;
>  	enum usb_otg_state	state;
>  	enum usb_phy_events	last_event;
>  
> @@ -105,6 +113,9 @@ struct usb_phy {
>  	u16			port_status;
>  	u16			port_change;
>  
> +	/* to support controllers that have multiple transceivers */
> +	struct list_head	head;
> +
>  	/* initialize/shutdown the OTG controller */
>  	int	(*init)(struct usb_phy *x);
>  	void	(*shutdown)(struct usb_phy *x);
> @@ -121,7 +132,8 @@ struct usb_phy {
>  
>  
>  /* for board-specific init logic */
> -extern int usb_add_phy(struct usb_phy *);
> +extern int usb_add_phy(struct usb_phy *, enum usb_phy_type type);
> +extern void usb_remove_phy(struct usb_phy *);
>  
>  #if defined(CONFIG_NOP_USB_XCEIV) || (defined(CONFIG_NOP_USB_XCEIV_MODULE) && defined(MODULE))
>  /* sometimes transceivers are accessed only through e.g. ULPI */
> @@ -172,11 +184,11 @@ usb_phy_shutdown(struct usb_phy *x)
>  
>  /* for usb host and peripheral controller drivers */
>  #ifdef CONFIG_USB_OTG_UTILS
> -extern struct usb_phy *usb_get_phy(void);
> +extern struct usb_phy *usb_get_phy(enum usb_phy_type type);
>  extern void usb_put_phy(struct usb_phy *);
>  extern const char *otg_state_string(enum usb_otg_state state);
>  #else
> -static inline struct usb_phy *usb_get_phy(void)
> +static inline struct usb_phy *usb_get_phy(enum usb_phy_type type)
>  {
>  	return NULL;
>  }
> @@ -276,4 +288,15 @@ usb_unregister_notifier(struct usb_phy *x, struct notifier_block *nb)
>  /* for OTG controller drivers (and maybe other stuff) */
>  extern int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num);
>  
> +static inline const char *usb_phy_type_string(enum usb_phy_type type)
> +{
> +	switch (type) {
> +	case USB_PHY_TYPE_USB2:
> +		return "USB2 PHY";
> +	case USB_PHY_TYPE_USB3:
> +		return "USB3 PHY";
> +	default:
> +		return "UNKNOWN PHY TYPE";
> +	}
> +}
>  #endif /* __LINUX_USB_OTG_H */


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux