[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <A2CA0424C0A6F04399FB9E1CD98E0304844E6AEE@US01WEMBX2.internal.synopsys.com>
Date: Wed, 29 Oct 2014 01:30:12 +0000
From: Paul Zimmerman <Paul.Zimmerman@...opsys.com>
To: "dinguyen@...nsource.altera.com" <dinguyen@...nsource.altera.com>,
"balbi@...com" <balbi@...com>
CC: "dinh.linux@...il.com" <dinh.linux@...il.com>,
"swarren@...dotorg.org" <swarren@...dotorg.org>,
"b.zolnierkie@...sung.com" <b.zolnierkie@...sung.com>,
"matthijs@...in.nl" <matthijs@...in.nl>,
"r.baldyga@...sung.com" <r.baldyga@...sung.com>,
"jg1.han@...sung.com" <jg1.han@...sung.com>,
"sachin.kamat@...aro.org" <sachin.kamat@...aro.org>,
"ben-linux@...ff.org" <ben-linux@...ff.org>,
"dianders@...omium.org" <dianders@...omium.org>,
"kever.yang@...k-chips.com" <kever.yang@...k-chips.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCHv6 6/8] usb: dwc2: gadget: Do not fail probe if there
isn't a clock node
> From: dinguyen@...nsource.altera.com [mailto:dinguyen@...nsource.altera.com]
> Sent: Tuesday, October 28, 2014 4:26 PM
>
> Since the dwc2 hcd driver is currently not looking for a clock node during
> init, we should not completely fail if there isn't a clock provided.
> For dual-role mode, we will only fail init for a non-clock node error. We
> then update the HCD to only call gadget funtions if there is a proper clock
> node.
>
> Signed-off-by: Dinh Nguyen <dinguyen@...nsource.altera.com>
> ---
> v5: reworked to not access gadget functions from the hcd.
> ---
> drivers/usb/dwc2/core.h | 3 +--
> drivers/usb/dwc2/core_intr.c | 9 ++++++---
> drivers/usb/dwc2/hcd.c | 3 ++-
> drivers/usb/dwc2/platform.c | 19 +++++++++++++++----
> 4 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index ec70862..48120c8 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -660,6 +660,7 @@ struct dwc2_hsotg {
> #endif
> #endif /* CONFIG_USB_DWC2_HOST || CONFIG_USB_DWC2_DUAL_ROLE */
>
> + struct clk *clk;
> #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
> /* Gadget structures */
> struct usb_gadget_driver *driver;
> @@ -667,8 +668,6 @@ struct dwc2_hsotg {
> struct usb_phy *uphy;
> struct s3c_hsotg_plat *plat;
>
> - struct clk *clk;
> -
> struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
>
> u32 phyif;
> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
> index 1240875..1608037 100644
> --- a/drivers/usb/dwc2/core_intr.c
> +++ b/drivers/usb/dwc2/core_intr.c
> @@ -339,7 +339,8 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
> }
> /* Change to L0 state */
> hsotg->lx_state = DWC2_L0;
> - call_gadget(hsotg, resume);
> + if (!IS_ERR(hsotg->clk))
> + call_gadget(hsotg, resume);
> } else {
> if (hsotg->lx_state != DWC2_L1) {
> u32 pcgcctl = readl(hsotg->regs + PCGCTL);
> @@ -400,7 +401,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg)
> "DSTS.Suspend Status=%d HWCFG4.Power Optimize=%d\n",
> !!(dsts & DSTS_SUSPSTS),
> hsotg->hw_params.power_optimized);
> - call_gadget(hsotg, suspend);
> + if (!IS_ERR(hsotg->clk))
> + call_gadget(hsotg, suspend);
> } else {
> if (hsotg->op_state == OTG_STATE_A_PERIPHERAL) {
> dev_dbg(hsotg->dev, "a_peripheral->a_host\n");
> @@ -477,7 +479,8 @@ irqreturn_t dwc2_handle_common_intr(int irq, void *dev)
> spin_lock(&hsotg->lock);
>
> if (dwc2_is_device_mode(hsotg))
> - retval = s3c_hsotg_irq(irq, dev);
> + if (!IS_ERR(hsotg->clk))
> + retval = s3c_hsotg_irq(irq, dev);
>
> gintsts = dwc2_read_common_intr(hsotg);
> if (gintsts & ~GINTSTS_PRTINT)
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 44c609f..fa49c72 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -1371,7 +1371,8 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
> hsotg->op_state = OTG_STATE_B_PERIPHERAL;
> dwc2_core_init(hsotg, false, -1);
> dwc2_enable_global_interrupts(hsotg);
> - s3c_hsotg_core_init(hsotg);
> + if (!IS_ERR(hsotg->clk))
> + s3c_hsotg_core_init(hsotg);
> } else {
> /* A-Device connector (Host Mode) */
> dev_dbg(hsotg->dev, "connId A\n");
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 72f32f7..77c8417 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -217,8 +217,17 @@ static int dwc2_driver_probe(struct platform_device *dev)
>
> spin_lock_init(&hsotg->lock);
> retval = dwc2_gadget_init(hsotg, irq);
> - if (retval)
> - return retval;
> + if (retval) {
> + /*
> + * We will not fail the driver initialization for dual-role
> + * if no clock node is supplied. However, all gadget
> + * functionality will be disabled if a clock node is not
> + * provided. Host functionality will continue.
> + * TO-DO: make clock node a requirement for the HCD.
> + */
> + if (!IS_ERR(hsotg->clk))
> + return retval;
> + }
> retval = dwc2_hcd_init(hsotg, irq, params);
> if (retval)
> return retval;
> @@ -234,7 +243,8 @@ static int dwc2_suspend(struct device *dev)
> int ret = 0;
>
> if (dwc2_is_device_mode(dwc2))
> - ret = s3c_hsotg_suspend(dwc2);
> + if (!IS_ERR(dwc2->clk))
> + ret = s3c_hsotg_suspend(dwc2);
> return ret;
> }
>
> @@ -244,7 +254,8 @@ static int dwc2_resume(struct device *dev)
> int ret = 0;
>
> if (dwc2_is_device_mode(dwc2))
> - ret = s3c_hsotg_resume(dwc2);
> + if (!IS_ERR(dwc2->clk))
> + ret = s3c_hsotg_resume(dwc2);
>
> return ret;
> }
Hi Felipe,
I don't feel qualified to ack/nak this patch, since I don't fully
understand the clock subsystem. Can you review this one and see if it
looks OK?
--
Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists