lkml.org 
[lkml]   [2014]   [Oct]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code
    Date

    Hi,

    On Monday, October 20, 2014 01:52:01 PM dinguyen@opensource.altera.com wrote:
    > From: Dinh Nguyen <dinguyen@opensource.altera.com>
    >
    > 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@opensource.altera.com>
    > Acked-by: Paul Zimmerman <paulz@synopsys.com>
    > ---
    > 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@simtec.co.uk>");
    > -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 = &params_bcm2835 },
    > { .compatible = "rockchip,rk3066-usb", .data = &params_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,



    \
     
     \ /
      Last update: 2014-10-22 13:41    [W:2.275 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site