Re: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code
From: Bartlomiej Zolnierkiewicz
Date: Wed Oct 22 2014 - 07:16:52 EST
Hi,
On Monday, October 20, 2014 01:52:01 PM dinguyen@xxxxxxxxxxxxxxxxxxxxx wrote:
> From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
>
> This patch will aggregate the probing of gadget/hcd driver into platform.c.
> The gadget probe funtion is converted into gadget_init that is now only
> responsible for gadget only initialization. All the gadget resources is now
> handled by platform.c
>
> Since the host workqueue will not get initialized if the driver is configured
> for peripheral mode only. Thus we need to check for wq_otg before calling
> queue_work().
>
> Also, we move spin_lock_init to common location for both host and gadget that
> is either in platform.c or pci.c.
>
> We also ove suspend/resume code to common platform code, and update it to use
> the new PM API (struct dev_pm_ops).
>
> Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.
This patch seems to break bisectability. It moves all the gadget probing
to platform.c but Kconfig/Makefile are not updated (platform.c will be
compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
> Acked-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> v5: Reworked by squashing the following commits into this one:
> * [PATCHv4 02/12] usb: dwc2: move "samsung,s3c6400-hsotg" into common platform
> * [PATCHv4 03/12] usb: dwc2: Update the gadget driver to use common dwc2_hsotg
> structure
> * [PATCHv4 09/12] usb: dwc2: initialize the spin_lock for both host and gadget
> * [PATCHv4 10/12] usb: dwc2: Add suspend/resume for gadget
> * [PATCHv4 11/12] usb: dwc2: check that the host work queue is valid
> Also use IS_ENABLED instead of #if defined
> ---
> drivers/usb/dwc2/core.h | 34 +++++++++++++++-
> drivers/usb/dwc2/core_intr.c | 8 ++--
> drivers/usb/dwc2/gadget.c | 97 +++++++++-----------------------------------
> drivers/usb/dwc2/hcd.c | 1 -
> drivers/usb/dwc2/hcd.h | 10 -----
> drivers/usb/dwc2/pci.c | 1 +
> drivers/usb/dwc2/platform.c | 32 +++++++++++++++
> 7 files changed, 91 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 5412f57..b21aace 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -667,7 +667,6 @@ struct dwc2_hsotg {
> struct usb_phy *uphy;
> struct s3c_hsotg_plat *plat;
>
> - int irq;
> struct clk *clk;
>
> struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
> @@ -961,4 +960,37 @@ extern void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
> */
> extern u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg);
>
> +/* Gadget defines */
> +#if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> +extern int s3c_hsotg_remove(struct dwc2_hsotg *hsotg);
> +extern int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2);
> +extern int s3c_hsotg_resume(struct dwc2_hsotg *dwc2);
> +extern int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq);
> +#else
> +static inline int s3c_hsotg_remove(struct dwc2_hsotg *dwc2)
> +{ return 0; }
> +static inline int s3c_hsotg_suspend(struct dwc2_hsotg *dwc2)
> +{ return 0; }
> +static inline int s3c_hsotg_resume(struct dwc2_hsotg *dwc2)
> +{ return 0; }
> +static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> +{ return 0; }
> +#endif
> +
> +#if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> +extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
> +extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
> +extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> +#else
> +static inline void dwc2_set_all_params(struct dwc2_core_params *params, int value) {}
> +static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
> +{ return 0; }
> +static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg) {}
> +static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
> +static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
> +static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
> + const struct dwc2_core_params *params)
> +{ return 0; }
> +#endif
> +
> #endif /* __DWC2_CORE_H__ */
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index c93918b..b176c2f 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -287,9 +287,11 @@ static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
> * Release lock before scheduling workq as it holds spinlock during
> * scheduling.
> */
> - spin_unlock(&hsotg->lock);
> - queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> - spin_lock(&hsotg->lock);
> + if (hsotg->wq_otg) {
> + spin_unlock(&hsotg->lock);
> + queue_work(hsotg->wq_otg, &hsotg->wf_otg);
> + spin_lock(&hsotg->lock);
> + }
>
> /* Clear interrupt */
> writel(GINTSTS_CONIDSTSCHNG, hsotg->regs + GINTSTS);
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 6611ea3..c407d33 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -3404,26 +3404,21 @@ static void s3c_hsotg_delete_debug(struct dwc2_hsotg *hsotg)
> }
>
> /**
> - * s3c_hsotg_probe - probe function for hsotg driver
> - * @pdev: The platform information for the driver
> + * dwc2_gadget_init - init function for gadget
> + * @dwc2: The data structure for the DWC2 driver.
> + * @irq: The IRQ number for the controller.
> */
> -static int s3c_hsotg_probe(struct platform_device *pdev)
> +int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> {
> - struct s3c_hsotg_plat *plat = dev_get_platdata(&pdev->dev);
> + struct device *dev = hsotg->dev;
> + struct s3c_hsotg_plat *plat = dev->platform_data;
> struct phy *phy;
> struct usb_phy *uphy;
> - struct device *dev = &pdev->dev;
> struct s3c_hsotg_ep *eps;
> - struct dwc2_hsotg *hsotg;
> - struct resource *res;
> int epnum;
> int ret;
> int i;
>
> - hsotg = devm_kzalloc(&pdev->dev, sizeof(struct dwc2_hsotg), GFP_KERNEL);
> - if (!hsotg)
> - return -ENOMEM;
> -
> /* Set default UTMI width */
> hsotg->phyif = GUSBCFG_PHYIF16;
>
> @@ -3431,14 +3426,14 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> * Attempt to find a generic PHY, then look for an old style
> * USB PHY, finally fall back to pdata
> */
> - phy = devm_phy_get(&pdev->dev, "usb2-phy");
> + phy = devm_phy_get(dev, "usb2-phy");
> if (IS_ERR(phy)) {
> uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> if (IS_ERR(uphy)) {
> /* Fallback for pdata */
> - plat = dev_get_platdata(&pdev->dev);
> + plat = dev_get_platdata(dev);
> if (!plat) {
> - dev_err(&pdev->dev,
> + dev_err(dev,
> "no platform data or transceiver defined\n");
> return -EPROBE_DEFER;
> }
> @@ -3455,36 +3450,12 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> hsotg->phyif = GUSBCFG_PHYIF8;
> }
>
> - hsotg->dev = dev;
> -
> - hsotg->clk = devm_clk_get(&pdev->dev, "otg");
> + hsotg->clk = devm_clk_get(dev, "otg");
> if (IS_ERR(hsotg->clk)) {
> dev_err(dev, "cannot get otg clock\n");
> return PTR_ERR(hsotg->clk);
> }
>
> - platform_set_drvdata(pdev, hsotg);
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> - hsotg->regs = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(hsotg->regs)) {
> - ret = PTR_ERR(hsotg->regs);
> - goto err_clk;
> - }
> -
> - ret = platform_get_irq(pdev, 0);
> - if (ret < 0) {
> - dev_err(dev, "cannot find IRQ\n");
> - goto err_clk;
> - }
> -
> - spin_lock_init(&hsotg->lock);
> -
> - hsotg->irq = ret;
> -
> - dev_info(dev, "regs %p, irq %d\n", hsotg->regs, hsotg->irq);
> -
> hsotg->gadget.max_speed = USB_SPEED_HIGH;
> hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
> hsotg->gadget.name = dev_name(dev);
> @@ -3501,7 +3472,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(hsotg->supplies),
> hsotg->supplies);
> if (ret) {
> - dev_err(hsotg->dev, "failed to request supplies: %d\n", ret);
> + dev_err(dev, "failed to request supplies: %d\n", ret);
> goto err_clk;
> }
>
> @@ -3520,7 +3491,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> s3c_hsotg_hw_cfg(hsotg);
> s3c_hsotg_init(hsotg);
>
> - ret = devm_request_irq(&pdev->dev, hsotg->irq, s3c_hsotg_irq, 0,
> + ret = devm_request_irq(dev, irq, s3c_hsotg_irq, 0,
> dev_name(dev), hsotg);
> if (ret < 0) {
> s3c_hsotg_phy_disable(hsotg);
> @@ -3572,7 +3543,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
> ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
> hsotg->supplies);
> if (ret) {
> - dev_err(&pdev->dev, "failed to disable supplies: %d\n", ret);
> + dev_err(dev, "failed to disable supplies: %d\n", ret);
> goto err_ep_mem;
> }
>
> @@ -3597,15 +3568,14 @@ err_clk:
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(dwc2_gadget_init);
>
> /**
> * s3c_hsotg_remove - remove function for hsotg driver
> * @pdev: The platform information for the driver
> */
> -static int s3c_hsotg_remove(struct platform_device *pdev)
> +int s3c_hsotg_remove(struct dwc2_hsotg *hsotg)
> {
> - struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
> -
> usb_del_gadget_udc(&hsotg->gadget);
>
> s3c_hsotg_delete_debug(hsotg);
> @@ -3619,10 +3589,10 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(s3c_hsotg_remove);
>
> -static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
> +int s3c_hsotg_suspend(struct dwc2_hsotg *hsotg)
> {
> - struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
> unsigned long flags;
> int ret = 0;
>
> @@ -3648,10 +3618,10 @@ static int s3c_hsotg_suspend(struct platform_device *pdev, pm_message_t state)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(s3c_hsotg_suspend);
>
> -static int s3c_hsotg_resume(struct platform_device *pdev)
> +int s3c_hsotg_resume(struct dwc2_hsotg *hsotg)
> {
> - struct dwc2_hsotg *hsotg = platform_get_drvdata(pdev);
> unsigned long flags;
> int ret = 0;
>
> @@ -3672,31 +3642,4 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
>
> return ret;
> }
> -
> -#ifdef CONFIG_OF
> -static const struct of_device_id s3c_hsotg_of_ids[] = {
> - { .compatible = "samsung,s3c6400-hsotg", },
> - { .compatible = "snps,dwc2", },
> - { /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
> -#endif
> -
> -static struct platform_driver s3c_hsotg_driver = {
> - .driver = {
> - .name = "s3c-hsotg",
> - .owner = THIS_MODULE,
> - .of_match_table = of_match_ptr(s3c_hsotg_of_ids),
> - },
> - .probe = s3c_hsotg_probe,
> - .remove = s3c_hsotg_remove,
> - .suspend = s3c_hsotg_suspend,
> - .resume = s3c_hsotg_resume,
> -};
> -
> -module_platform_driver(s3c_hsotg_driver);
> -
> -MODULE_DESCRIPTION("Samsung S3C USB High-speed/OtG device");
> -MODULE_AUTHOR("Ben Dooks <ben@xxxxxxxxxxxx>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:s3c-hsotg");
> +EXPORT_SYMBOL_GPL(s3c_hsotg_resume);
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 0a0e6f0..4a3cce0 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -2839,7 +2839,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
>
> hcd->has_tt = 1;
>
> - spin_lock_init(&hsotg->lock);
> ((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
> hsotg->priv = hcd;
>
> diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
> index a12bb15..e69a843 100644
> --- a/drivers/usb/dwc2/hcd.h
> +++ b/drivers/usb/dwc2/hcd.h
> @@ -668,9 +668,6 @@ extern irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg);
> */
> extern void dwc2_hcd_stop(struct dwc2_hsotg *hsotg);
>
> -extern void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> -extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
> -
> /**
> * dwc2_hcd_is_b_host() - Returns 1 if core currently is acting as B host,
> * and 0 otherwise
> @@ -680,13 +677,6 @@ extern void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg);
> extern int dwc2_hcd_is_b_host(struct dwc2_hsotg *hsotg);
>
> /**
> - * dwc2_hcd_get_frame_number() - Returns current frame number
> - *
> - * @hsotg: The DWC2 HCD
> - */
> -extern int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
> -
> -/**
> * dwc2_hcd_dump_state() - Dumps hsotg state
> *
> * @hsotg: The DWC2 HCD
> diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
> index c291fca..6d33ecf 100644
> --- a/drivers/usb/dwc2/pci.c
> +++ b/drivers/usb/dwc2/pci.c
> @@ -141,6 +141,7 @@ static int dwc2_driver_probe(struct pci_dev *dev,
>
> pci_set_master(dev);
>
> + spin_lock_init(&hsotg->lock);
> retval = dwc2_hcd_init(hsotg, dev->irq, &dwc2_module_params);
> if (retval) {
> pci_disable_device(dev);
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 121dbda..5783ed0 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -121,6 +121,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
> struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
>
> dwc2_hcd_remove(hsotg);
> + s3c_hsotg_remove(hsotg);
>
> return 0;
> }
> @@ -129,6 +130,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
> { .compatible = "brcm,bcm2835-usb", .data = ¶ms_bcm2835 },
> { .compatible = "rockchip,rk3066-usb", .data = ¶ms_rk3066 },
> { .compatible = "snps,dwc2", .data = NULL },
> + { .compatible = "samsung,s3c6400-hsotg", .data = NULL},
> {},
> };
> MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
> @@ -204,6 +206,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
>
> hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
>
> + spin_lock_init(&hsotg->lock);
> + retval = dwc2_gadget_init(hsotg, irq);
> + if (retval)
> + return retval;
> retval = dwc2_hcd_init(hsotg, irq, params);
> if (retval)
> return retval;
> @@ -213,10 +219,36 @@ static int dwc2_driver_probe(struct platform_device *dev)
> return retval;
> }
>
> +static int dwc2_suspend(struct device *dev)
> +{
> + struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (dwc2_is_device_mode(dwc2))
> + ret = s3c_hsotg_suspend(dwc2);
> + return ret;
> +}
> +
> +static int dwc2_resume(struct device *dev)
> +{
> + struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (dwc2_is_device_mode(dwc2))
> + ret = s3c_hsotg_resume(dwc2);
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops dwc2_dev_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(dwc2_suspend, dwc2_resume)
> +};
> +
> static struct platform_driver dwc2_platform_driver = {
> .driver = {
> .name = dwc2_driver_name,
> .of_match_table = dwc2_of_match_table,
> + .pm = &dwc2_dev_pm_ops,
> },
> .probe = dwc2_driver_probe,
> .remove = dwc2_driver_remove,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/