| Message ID | 1409212920-28526-2-git-send-email-gautam.vivek@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: > Exynos7 also has a separate special gate clock going to the IP > apart from the usual AHB clock. So add support for the same. > > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > --- > drivers/usb/dwc3/dwc3-exynos.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > index f9fb8ad..bab6395 100644 > --- a/drivers/usb/dwc3/dwc3-exynos.c > +++ b/drivers/usb/dwc3/dwc3-exynos.c > @@ -35,6 +35,7 @@ struct dwc3_exynos { > struct device *dev; > > struct clk *clk; > + struct clk *sclk; > struct regulator *vdd33; > struct regulator *vdd10; > }; > @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > return -EINVAL; > } > > + /* Exynos7 has a special gate clock going to this IP */ > + exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk"); > + if (IS_ERR(exynos->sclk)) > + dev_warn(dev, "couldn't get sclk\n"); Doesn't this introduce a pointless warning for Exynos SoCs other than Exynos7? > + > exynos->dev = dev; > exynos->clk = clk; > > clk_prepare_enable(exynos->clk); > + if (!IS_ERR(exynos->sclk)) > + clk_prepare_enable(exynos->sclk); If you replaced the returned err value with NULL you could avoid these IS_ERR cases. Mark. > > exynos->vdd33 = devm_regulator_get(dev, "vdd33"); > if (IS_ERR(exynos->vdd33)) { > @@ -187,6 +195,8 @@ err4: > err3: > regulator_disable(exynos->vdd33); > err2: > + if (!IS_ERR(exynos->sclk)) > + clk_disable_unprepare(exynos->sclk); > clk_disable_unprepare(clk); > return ret; > } > @@ -199,6 +209,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev) > platform_device_unregister(exynos->usb2_phy); > platform_device_unregister(exynos->usb3_phy); > > + if (!IS_ERR(exynos->sclk)) > + clk_disable_unprepare(exynos->sclk); > clk_disable_unprepare(exynos->clk); > > regulator_disable(exynos->vdd33); > @@ -220,6 +232,8 @@ static int dwc3_exynos_suspend(struct device *dev) > { > struct dwc3_exynos *exynos = dev_get_drvdata(dev); > > + if (!IS_ERR(exynos->sclk)) > + clk_disable(exynos->sclk); > clk_disable(exynos->clk); > > regulator_disable(exynos->vdd33); > @@ -245,6 +259,8 @@ static int dwc3_exynos_resume(struct device *dev) > } > > clk_enable(exynos->clk); > + if (!IS_ERR(exynos->sclk)) > + clk_enable(exynos->sclk); > > /* runtime set active to reflect active state. */ > pm_runtime_disable(dev); > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi, On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: >> Exynos7 also has a separate special gate clock going to the IP >> apart from the usual AHB clock. So add support for the same. >> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >> --- >> drivers/usb/dwc3/dwc3-exynos.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c >> index f9fb8ad..bab6395 100644 >> --- a/drivers/usb/dwc3/dwc3-exynos.c >> +++ b/drivers/usb/dwc3/dwc3-exynos.c >> @@ -35,6 +35,7 @@ struct dwc3_exynos { >> struct device *dev; >> >> struct clk *clk; >> + struct clk *sclk; >> struct regulator *vdd33; >> struct regulator *vdd10; >> }; >> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) >> return -EINVAL; >> } >> >> + /* Exynos7 has a special gate clock going to this IP */ >> + exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk"); >> + if (IS_ERR(exynos->sclk)) >> + dev_warn(dev, "couldn't get sclk\n"); > > Doesn't this introduce a pointless warning for Exynos SoCs other than > Exynos7? True, it will introduce an unnecessary warning for non-Exynos7 systems. I initially thought of introducing a compatible check for Exynos7-dwc3, but that way we may end up adding such checks for future SoCs which have similar controller but have some clock difference or some other small change, no ? > >> + >> exynos->dev = dev; >> exynos->clk = clk; >> >> clk_prepare_enable(exynos->clk); >> + if (!IS_ERR(exynos->sclk)) >> + clk_prepare_enable(exynos->sclk); > > If you replaced the returned err value with NULL you could avoid these > IS_ERR cases. Right, point taken. [snip]
On Tue, Sep 02, 2014 at 11:39:08AM +0100, Vivek Gautam wrote: > Hi, > > > On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: > >> Exynos7 also has a separate special gate clock going to the IP > >> apart from the usual AHB clock. So add support for the same. > >> > >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > >> --- > >> drivers/usb/dwc3/dwc3-exynos.c | 16 ++++++++++++++++ > >> 1 file changed, 16 insertions(+) > >> > >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > >> index f9fb8ad..bab6395 100644 > >> --- a/drivers/usb/dwc3/dwc3-exynos.c > >> +++ b/drivers/usb/dwc3/dwc3-exynos.c > >> @@ -35,6 +35,7 @@ struct dwc3_exynos { > >> struct device *dev; > >> > >> struct clk *clk; > >> + struct clk *sclk; > >> struct regulator *vdd33; > >> struct regulator *vdd10; > >> }; > >> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > >> return -EINVAL; > >> } > >> > >> + /* Exynos7 has a special gate clock going to this IP */ > >> + exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk"); > >> + if (IS_ERR(exynos->sclk)) > >> + dev_warn(dev, "couldn't get sclk\n"); > > > > Doesn't this introduce a pointless warning for Exynos SoCs other than > > Exynos7? > > True, it will introduce an unnecessary warning for non-Exynos7 systems. > I initially thought of introducing a compatible check for Exynos7-dwc3, but that > way we may end up adding such checks for future SoCs which have similar > controller but have some clock difference or some other small change, no ? Perhaps. I don't know what your future hardware will look like. Is the usbdrd30_sclk input unique to Exynos7, or was it previously there but just without an input? If the latter you could just reduce this to: dev_info(dev, "no sclk specified"); Mark.
On Tue, Sep 02, 2014 at 04:09:08PM +0530, Vivek Gautam wrote: > Hi, > > > On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland <mark.rutland@arm.com> wrote: > > On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: > >> Exynos7 also has a separate special gate clock going to the IP > >> apart from the usual AHB clock. So add support for the same. > >> > >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > >> --- > >> drivers/usb/dwc3/dwc3-exynos.c | 16 ++++++++++++++++ > >> 1 file changed, 16 insertions(+) > >> > >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c > >> index f9fb8ad..bab6395 100644 > >> --- a/drivers/usb/dwc3/dwc3-exynos.c > >> +++ b/drivers/usb/dwc3/dwc3-exynos.c > >> @@ -35,6 +35,7 @@ struct dwc3_exynos { > >> struct device *dev; > >> > >> struct clk *clk; > >> + struct clk *sclk; > >> struct regulator *vdd33; > >> struct regulator *vdd10; > >> }; > >> @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) > >> return -EINVAL; > >> } > >> > >> + /* Exynos7 has a special gate clock going to this IP */ > >> + exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk"); > >> + if (IS_ERR(exynos->sclk)) > >> + dev_warn(dev, "couldn't get sclk\n"); > > > > Doesn't this introduce a pointless warning for Exynos SoCs other than > > Exynos7? > > True, it will introduce an unnecessary warning for non-Exynos7 systems. > I initially thought of introducing a compatible check for Exynos7-dwc3, but that > way we may end up adding such checks for future SoCs which have similar > controller but have some clock difference or some other small change, no ? maybe dev_dbg() is what you want ? :-)
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index f9fb8ad..bab6395 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -35,6 +35,7 @@ struct dwc3_exynos { struct device *dev; struct clk *clk; + struct clk *sclk; struct regulator *vdd33; struct regulator *vdd10; }; @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) return -EINVAL; } + /* Exynos7 has a special gate clock going to this IP */ + exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk"); + if (IS_ERR(exynos->sclk)) + dev_warn(dev, "couldn't get sclk\n"); + exynos->dev = dev; exynos->clk = clk; clk_prepare_enable(exynos->clk); + if (!IS_ERR(exynos->sclk)) + clk_prepare_enable(exynos->sclk); exynos->vdd33 = devm_regulator_get(dev, "vdd33"); if (IS_ERR(exynos->vdd33)) { @@ -187,6 +195,8 @@ err4: err3: regulator_disable(exynos->vdd33); err2: + if (!IS_ERR(exynos->sclk)) + clk_disable_unprepare(exynos->sclk); clk_disable_unprepare(clk); return ret; } @@ -199,6 +209,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev) platform_device_unregister(exynos->usb2_phy); platform_device_unregister(exynos->usb3_phy); + if (!IS_ERR(exynos->sclk)) + clk_disable_unprepare(exynos->sclk); clk_disable_unprepare(exynos->clk); regulator_disable(exynos->vdd33); @@ -220,6 +232,8 @@ static int dwc3_exynos_suspend(struct device *dev) { struct dwc3_exynos *exynos = dev_get_drvdata(dev); + if (!IS_ERR(exynos->sclk)) + clk_disable(exynos->sclk); clk_disable(exynos->clk); regulator_disable(exynos->vdd33); @@ -245,6 +259,8 @@ static int dwc3_exynos_resume(struct device *dev) } clk_enable(exynos->clk); + if (!IS_ERR(exynos->sclk)) + clk_enable(exynos->sclk); /* runtime set active to reflect active state. */ pm_runtime_disable(dev);
Exynos7 also has a separate special gate clock going to the IP apart from the usual AHB clock. So add support for the same. Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> --- drivers/usb/dwc3/dwc3-exynos.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)