Patchwork [GIT,PULL] Renesas ARM-based SoC: r8a7740 SoC for 3.7

login
register
mail settings
Submitter Simon Horman
Date Aug. 30, 2012, 5:52 a.m.
Message ID <1346305969-27110-1-git-send-email-horms@verge.net.au>
Download mbox
Permalink /patch/180800/
State New
Headers show

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git r8a7740

Comments

Simon Horman - Aug. 30, 2012, 5:52 a.m.
Hi Olof, Hi Arnd,

please consider the following documentation enhancement for the
r8a7740 SoC from Morimoto-san for inclusion in 3.7.

----------------------------------------------------------------
The following changes since commit fea7a08acb13524b47711625eebea40a0ede69a0:

  Linux 3.6-rc3 (2012-08-22 13:29:06 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git r8a7740

for you to fetch changes up to a6e7c110f972d7ca5c6eb288f5e64d5659b44dc0:

  ARM: shmobile: r8a7740: add USB24 clock explain and sample code (2012-08-27 17:43:22 +0900)

----------------------------------------------------------------
Kuninori Morimoto (1):
      ARM: shmobile: r8a7740: add USB24 clock explain and sample code

 arch/arm/mach-shmobile/clock-r8a7740.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
Olof Johansson - Sept. 5, 2012, 10:54 p.m.
Hi,

A couple of comments below.

On Thu, Aug 30, 2012 at 02:52:49PM +0900, Simon Horman wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> USBCKCR is controlling USB parent clock and divide rate.
> This parent clock is used as a "usb24s" from other devices,
> but the "divide rate" is not used.
> Further, this clock itself is known as "usb24".
> So, to set this clock is a little confusable.
> This patch adds quick explain and sample code for this clock.
> 
> Cc: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> ---
>  arch/arm/mach-shmobile/clock-r8a7740.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm/mach-shmobile/clock-r8a7740.c b/arch/arm/mach-shmobile/clock-r8a7740.c
> index ad5fccc..a6a7583 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7740.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7740.c
> @@ -188,6 +188,22 @@ static struct clk pllc1_div2_clk = {
>  };
>  
>  /* USB clock */
> +/*
> + * USBCKCR is controlling usb24 clock
> + * bit[7] : parent clock
> + * bit[6] : clock divide rate

The divisor is either 1 or 2 in this case, for bit[6] values of 0 and 1?

> + * And this bit[7] is used as a "usb24s" from other devices.

Sorry, I don't follow the explanation here.

This is how I interpret the current code, can you confirm if it's accurate?

usb24 is the (potentially) divided output from usb24s, while usb24s is
at a fixed pass-through rate from the parent if enabled. Both share the
same enable bit. In other words, usb24 is a directly derived, potentially
divided-by-two rate from usb24s?

> + * (Video clock / Sub clock / SPU clock)
> + * You can controll this clock as a below.
> + *
> + * struct clk *usb24	= clk_get(dev,  "usb24");
> + * struct clk *usb24s	= clk_get(NULL, "usb24s");
> + * struct clk *system	= clk_get(NULL, "system_clk");
> + * int rate = clk_get_rate(system);
> + *
> + * clk_set_parent(usb24s, system);  // for bit[7]
> + * clk_set_rate(usb24, rate / 2);   // for bit[6]

This is just standard clk infrastructure usage, I don't think there's need to
include the example code here.


-Olof
Kuninori Morimoto - Sept. 7, 2012, 1:23 a.m.
Hi Olof

Thank you for checking patch

> On Thu, Aug 30, 2012 at 02:52:49PM +0900, Simon Horman wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > USBCKCR is controlling USB parent clock and divide rate.
> > This parent clock is used as a "usb24s" from other devices,
> > but the "divide rate" is not used.
> > Further, this clock itself is known as "usb24".
> > So, to set this clock is a little confusable.
> > This patch adds quick explain and sample code for this clock.
> > 
> > Cc: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > ---
> >  arch/arm/mach-shmobile/clock-r8a7740.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/arm/mach-shmobile/clock-r8a7740.c b/arch/arm/mach-shmobile/clock-r8a7740.c
> > index ad5fccc..a6a7583 100644
> > --- a/arch/arm/mach-shmobile/clock-r8a7740.c
> > +++ b/arch/arm/mach-shmobile/clock-r8a7740.c
> > @@ -188,6 +188,22 @@ static struct clk pllc1_div2_clk = {
> >  };
> >  
> >  /* USB clock */
> > +/*
> > + * USBCKCR is controlling usb24 clock
> > + * bit[7] : parent clock
> > + * bit[6] : clock divide rate
> 
> The divisor is either 1 or 2 in this case, for bit[6] values of 0 and 1?

Yes.
if (0 == bit[6]) clock is 1/2
if (1 == bit[6]) clock is 1/1

> > + * And this bit[7] is used as a "usb24s" from other devices.
> 
> Sorry, I don't follow the explanation here.
> 
> This is how I interpret the current code, can you confirm if it's accurate?
> 
> usb24 is the (potentially) divided output from usb24s, while usb24s is
> at a fixed pass-through rate from the parent if enabled. Both share the
> same enable bit. In other words, usb24 is a directly derived, potentially
> divided-by-two rate from usb24s?

clcok A ---+  bit[7]                 bit[6]
           |- select -> usb24s -+--> divide (1/1 or 1/2) -> usb24
clock B ---+                    |
                                +--> other clocks ->


> > + * (Video clock / Sub clock / SPU clock)
> > + * You can controll this clock as a below.
> > + *
> > + * struct clk *usb24	= clk_get(dev,  "usb24");
> > + * struct clk *usb24s	= clk_get(NULL, "usb24s");
> > + * struct clk *system	= clk_get(NULL, "system_clk");
> > + * int rate = clk_get_rate(system);
> > + *
> > + * clk_set_parent(usb24s, system);  // for bit[7]
> > + * clk_set_rate(usb24, rate / 2);   // for bit[6]
> 
> This is just standard clk infrastructure usage, I don't think there's need to
> include the example code here.

Ahh.. 50% yes.

_If_ this usb24/usb24s explanation on datasheet is same as other clocks,
it is easy to understand and can use clk_set_rate().

But this setting style of these 2 clock is very special case on our datasheet.
Maybe first case.
So, I added sample setting code here,
since it is difficult to understand how to set the clock from
datasheet/clock tree and clock-r8a7740.c

Best regards
---
Kuninori Morimoto