diff mbox

[v2,1/4] dwc3: exynos: Add support for SCLK present on Exynos7

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

Commit Message

Vivek Gautam Oct. 7, 2014, 10:19 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

Felipe Balbi Oct. 7, 2014, 2:11 p.m. UTC | #1
On Tue, Oct 07, 2014 at 03:49:33PM +0530, 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>

I'll take this one once -rc1 is tagged. The others have no direct
dependency on this so I'll leave them to Kishon.
Vivek Gautam Oct. 8, 2014, 3:01 a.m. UTC | #2
On Tue, Oct 7, 2014 at 7:41 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Tue, Oct 07, 2014 at 03:49:33PM +0530, 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>
>
> I'll take this one once -rc1 is tagged. The others have no direct
> dependency on this so I'll leave them to Kishon.

Thanks Felipe !
Anton Tikhomirov Oct. 13, 2014, 4:54 a.m. UTC | #3
Hi Vivek,

> Exynos7 also has a separate special gate clock going to the IP
> apart from the usual AHB clock. So add support for the same.

As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
by the driver. Adding only sclk is not enough. 

> 
> 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 3951a65..7dc6a98 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;

The clock "clk" in Exynos5 just gated all that above 7 clocks, which
we should control separately now in Exynos7.

> +	struct clk		*sclk;
>  	struct regulator	*vdd33;
>  	struct regulator	*vdd10;
>  };
> @@ -139,10 +140,21 @@ static int dwc3_exynos_probe(struct
> platform_device *pdev)
>  		return -EINVAL;
>  	}
> 
> +	/*
> +	 * Exynos7 has a special gate clock going to this IP,
> +	 * which in earlier SoCs was probably concealed.
> +	 */
> +	exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
> +	if (IS_ERR(exynos->sclk)) {
> +		dev_info(dev, "no sclk specified\n");
> +		exynos->sclk = NULL;
> +	}
> +
>  	exynos->dev	= dev;
>  	exynos->clk	= clk;
> 
>  	clk_prepare_enable(exynos->clk);
> +	clk_prepare_enable(exynos->sclk);
> 
>  	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>  	if (IS_ERR(exynos->vdd33)) {
> @@ -185,6 +197,7 @@ err4:
>  err3:
>  	regulator_disable(exynos->vdd33);
>  err2:
> +	clk_disable_unprepare(exynos->sclk);
>  	clk_disable_unprepare(clk);
>  	return ret;
>  }
> @@ -197,6 +210,7 @@ static int dwc3_exynos_remove(struct
> platform_device *pdev)
>  	platform_device_unregister(exynos->usb2_phy);
>  	platform_device_unregister(exynos->usb3_phy);
> 
> +	clk_disable_unprepare(exynos->sclk);
>  	clk_disable_unprepare(exynos->clk);
> 
>  	regulator_disable(exynos->vdd33);
> @@ -218,6 +232,7 @@ static int dwc3_exynos_suspend(struct device *dev)
>  {
>  	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
> 
> +	clk_disable(exynos->sclk);
>  	clk_disable(exynos->clk);
> 
>  	regulator_disable(exynos->vdd33);
> @@ -243,6 +258,7 @@ static int dwc3_exynos_resume(struct device *dev)
>  	}
> 
>  	clk_enable(exynos->clk);
> +	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 linux-usb" 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-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi Oct. 13, 2014, 10:44 p.m. UTC | #4
Hi,

On Mon, Oct 13, 2014 at 01:54:59PM +0900, Anton Tikhomirov wrote:
> Hi Vivek,
> 
> > Exynos7 also has a separate special gate clock going to the IP
> > apart from the usual AHB clock. So add support for the same.
> 
> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> by the driver. Adding only sclk is not enough. 
> 
> > 
> > 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 3951a65..7dc6a98 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;
> 
> The clock "clk" in Exynos5 just gated all that above 7 clocks, which
> we should control separately now in Exynos7.
> 

should I drop this patch for now ?
Tomasz Figa Oct. 13, 2014, 10:51 p.m. UTC | #5
Hi Anton,

On 13.10.2014 06:54, Anton Tikhomirov wrote:
> Hi Vivek,
> 
>> Exynos7 also has a separate special gate clock going to the IP
>> apart from the usual AHB clock. So add support for the same.
> 
> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> by the driver. Adding only sclk is not enough. 
> 

I'm quite interested in this discussion. Has it happened on mailing lists?

In general, previous SoCs also gave the possibility of controlling all
the bus clocks separately, in addition to bulk gates, but there was no
real advantage in using those, while burdening the clock tree with
numerous clocks. Isn't Exynos7 similar in this aspect?

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anton Tikhomirov Oct. 14, 2014, 1:26 a.m. UTC | #6
Hello,

> Hi Anton,
> 
> On 13.10.2014 06:54, Anton Tikhomirov wrote:
> > Hi Vivek,
> >
> >> Exynos7 also has a separate special gate clock going to the IP
> >> apart from the usual AHB clock. So add support for the same.
> >
> > As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> > by the driver. Adding only sclk is not enough.
> >
> 
> I'm quite interested in this discussion. Has it happened on mailing
> lists?

No, we used company messenger for the discussion.

> 
> In general, previous SoCs also gave the possibility of controlling all
> the bus clocks separately, in addition to bulk gates, but there was no

correct

> real advantage in using those, while burdening the clock tree with
> numerous clocks. Isn't Exynos7 similar in this aspect?

Exynos7 doesn't have "Gating all clocks for USBDRD30" bit. The clocks
should be controlled separately.

> 
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" 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-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vivek Gautam Oct. 14, 2014, 4:53 a.m. UTC | #7
Hi Tomasz,


On Tue, Oct 14, 2014 at 6:56 AM, Anton Tikhomirov
<av.tikhomirov@samsung.com> wrote:
> Hello,
>
>> Hi Anton,
>>
>> On 13.10.2014 06:54, Anton Tikhomirov wrote:
>> > Hi Vivek,
>> >
>> >> Exynos7 also has a separate special gate clock going to the IP
>> >> apart from the usual AHB clock. So add support for the same.
>> >
>> > As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
>> > by the driver. Adding only sclk is not enough.
>> >
>>
>> I'm quite interested in this discussion. Has it happened on mailing
>> lists?
>
> No, we used company messenger for the discussion.

Yea, we head a round of discussion at our end regarding this, and we are
going to get more clarity on this from our H/W team too, this week.

>
>>
>> In general, previous SoCs also gave the possibility of controlling all
>> the bus clocks separately, in addition to bulk gates, but there was no
>
> correct
>
>> real advantage in using those, while burdening the clock tree with
>> numerous clocks. Isn't Exynos7 similar in this aspect?
>
> Exynos7 doesn't have "Gating all clocks for USBDRD30" bit. The clocks
> should be controlled separately.

true, on Exynos7 we have separate gates for the available clocks going to
USB-DRD block. So we will have to add these basic required number of
clocks.
Vivek Gautam Oct. 14, 2014, 4:55 a.m. UTC | #8
Hi Felipe,


On Tue, Oct 14, 2014 at 4:14 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, Oct 13, 2014 at 01:54:59PM +0900, Anton Tikhomirov wrote:
>> Hi Vivek,
>>
>> > Exynos7 also has a separate special gate clock going to the IP
>> > apart from the usual AHB clock. So add support for the same.
>>
>> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
>> by the driver. Adding only sclk is not enough.
>>
>> >
>> > 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 3951a65..7dc6a98 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;
>>
>> The clock "clk" in Exynos5 just gated all that above 7 clocks, which
>> we should control separately now in Exynos7.
>>
>
> should I drop this patch for now ?

Yes, better to hold this for some time till we get more clarity
from our h/w team.
Felipe Balbi Oct. 15, 2014, 2:50 p.m. UTC | #9
Hi,

On Tue, Oct 14, 2014 at 10:25:00AM +0530, Vivek Gautam wrote:
> Hi Felipe,
> 
> 
> On Tue, Oct 14, 2014 at 4:14 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Mon, Oct 13, 2014 at 01:54:59PM +0900, Anton Tikhomirov wrote:
> >> Hi Vivek,
> >>
> >> > Exynos7 also has a separate special gate clock going to the IP
> >> > apart from the usual AHB clock. So add support for the same.
> >>
> >> As we discussed before, Exynos7 SoCs have 7 clocks to be controlled
> >> by the driver. Adding only sclk is not enough.
> >>
> >> >
> >> > 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 3951a65..7dc6a98 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;
> >>
> >> The clock "clk" in Exynos5 just gated all that above 7 clocks, which
> >> we should control separately now in Exynos7.
> >>
> >
> > should I drop this patch for now ?
> 
> Yes, better to hold this for some time till we get more clarity
> from our h/w team.

now dropped. Please a new one if needed.
diff mbox

Patch

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 3951a65..7dc6a98 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;
 };
@@ -139,10 +140,21 @@  static int dwc3_exynos_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	/*
+	 * Exynos7 has a special gate clock going to this IP,
+	 * which in earlier SoCs was probably concealed.
+	 */
+	exynos->sclk = devm_clk_get(dev, "usbdrd30_sclk");
+	if (IS_ERR(exynos->sclk)) {
+		dev_info(dev, "no sclk specified\n");
+		exynos->sclk = NULL;
+	}
+
 	exynos->dev	= dev;
 	exynos->clk	= clk;
 
 	clk_prepare_enable(exynos->clk);
+	clk_prepare_enable(exynos->sclk);
 
 	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
 	if (IS_ERR(exynos->vdd33)) {
@@ -185,6 +197,7 @@  err4:
 err3:
 	regulator_disable(exynos->vdd33);
 err2:
+	clk_disable_unprepare(exynos->sclk);
 	clk_disable_unprepare(clk);
 	return ret;
 }
@@ -197,6 +210,7 @@  static int dwc3_exynos_remove(struct platform_device *pdev)
 	platform_device_unregister(exynos->usb2_phy);
 	platform_device_unregister(exynos->usb3_phy);
 
+	clk_disable_unprepare(exynos->sclk);
 	clk_disable_unprepare(exynos->clk);
 
 	regulator_disable(exynos->vdd33);
@@ -218,6 +232,7 @@  static int dwc3_exynos_suspend(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 
+	clk_disable(exynos->sclk);
 	clk_disable(exynos->clk);
 
 	regulator_disable(exynos->vdd33);
@@ -243,6 +258,7 @@  static int dwc3_exynos_resume(struct device *dev)
 	}
 
 	clk_enable(exynos->clk);
+	clk_enable(exynos->sclk);
 
 	/* runtime set active to reflect active state. */
 	pm_runtime_disable(dev);