diff mbox

[1/5] usb: dwc3: exynos: Add support for SCLK present on Exynos7

Message ID 1409212920-28526-2-git-send-email-gautam.vivek@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vivek Gautam Aug. 28, 2014, 8:01 a.m. UTC
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(+)

Comments

Mark Rutland Aug. 28, 2014, 6:48 p.m. UTC | #1
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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vivek Gautam Sept. 2, 2014, 10:39 a.m. UTC | #2
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]
Mark Rutland Sept. 2, 2014, 11:01 a.m. UTC | #3
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi Sept. 2, 2014, 2:35 p.m. UTC | #4
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 mbox

Patch

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);