diff mbox

[2/5] arm: shmobile: r7s72100: add i2c clocks

Message ID 1387316678-10174-3-git-send-email-wsa@the-dreams.de (mailing list archive)
State Superseded
Headers show

Commit Message

Wolfram Sang Dec. 17, 2013, 9:44 p.m. UTC
From: Wolfram Sang <wsa@sang-engineering.com>

Tested with RIIC2 on a genmai board. Others untested but hopefully
trivial enough to be added.

Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
Acked-by: Magnus Damm <damm@opensource.se>
---
 arch/arm/mach-shmobile/clock-r7s72100.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Sergei Shtylyov Dec. 18, 2013, 11:38 a.m. UTC | #1
Hello.

On 18-12-2013 1:44, Wolfram Sang wrote:

> From: Wolfram Sang <wsa@sang-engineering.com>

> Tested with RIIC2 on a genmai board. Others untested but hopefully
> trivial enough to be added.

> Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
> Acked-by: Magnus Damm <damm@opensource.se>
> ---
>   arch/arm/mach-shmobile/clock-r7s72100.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)

> diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
> index 7b457ae..770316d 100644
> --- a/arch/arm/mach-shmobile/clock-r7s72100.c
> +++ b/arch/arm/mach-shmobile/clock-r7s72100.c
[...]
> @@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
>   	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
>
>   	/* ICK */
> +	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
> +	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
> +	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
> +	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),

    These belong to some other place, the group marked by /* ICK */ is only 
for CLKDEV_ICK_ID().

>   	CLKDEV_ICK_ID("sci_fck", "sh-sci.0", &mstp_clks[MSTP47]),
>   	CLKDEV_ICK_ID("sci_fck", "sh-sci.1", &mstp_clks[MSTP46]),
>   	CLKDEV_ICK_ID("sci_fck", "sh-sci.2", &mstp_clks[MSTP45]),

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Dec. 18, 2013, 11:43 a.m. UTC | #2
> >@@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
> >  	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
> >
> >  	/* ICK */
> >+	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
> >+	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
> >+	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
> >+	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
> 
>    These belong to some other place, the group marked by /* ICK */
> is only for CLKDEV_ICK_ID().

So, I'll create a /* DEV */ prefix?
Sergei Shtylyov Dec. 18, 2013, 11:53 a.m. UTC | #3
On 18-12-2013 15:43, Wolfram Sang wrote:

>>> @@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
>>>   	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),

>>>   	/* ICK */
>>> +	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
>>> +	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
>>> +	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
>>> +	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),

>>     These belong to some other place, the group marked by /* ICK */
>> is only for CLKDEV_ICK_ID().

> So, I'll create a /* DEV */ prefix?

    I really don't know. Other places have /* MSTP */ comment in this case 
despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are really MSTP 
clocks. I considered the idea of separating CLKDEV_ICK_ID() under /* ICK */ 
comment silly from the very start but Simon didn't listen to me.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Dec. 18, 2013, 12:15 p.m. UTC | #4
On Wed, Dec 18, 2013 at 03:53:45PM +0400, Sergei Shtylyov wrote:
> On 18-12-2013 15:43, Wolfram Sang wrote:
> 
> >>>@@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
> >>>  	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
> 
> >>>  	/* ICK */
> >>>+	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
> >>>+	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
> >>>+	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
> >>>+	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
> 
> >>    These belong to some other place, the group marked by /* ICK */
> >>is only for CLKDEV_ICK_ID().
> 
> >So, I'll create a /* DEV */ prefix?
> 
>    I really don't know. Other places have /* MSTP */ comment in this
> case despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are
> really MSTP clocks. I considered the idea of separating
> CLKDEV_ICK_ID() under /* ICK */ comment silly from the very start
> but Simon didn't listen to me.

I am puzzled, too. ICK is a type of registration and not a clock domain.
Also, there is 'mtu2_fck' which is under ICK as well as MSTP? Looks
wrong. From what I understand now, removing the /* ICK */ comment would
be easiest and proper?
Simon Horman Dec. 18, 2013, 1:49 p.m. UTC | #5
On Wed, Dec 18, 2013 at 01:15:42PM +0100, Wolfram Sang wrote:
> 
> On Wed, Dec 18, 2013 at 03:53:45PM +0400, Sergei Shtylyov wrote:
> > On 18-12-2013 15:43, Wolfram Sang wrote:
> > 
> > >>>@@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
> > >>>  	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
> > 
> > >>>  	/* ICK */
> > >>>+	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
> > >>>+	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
> > >>>+	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
> > >>>+	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
> > 
> > >>    These belong to some other place, the group marked by /* ICK */
> > >>is only for CLKDEV_ICK_ID().
> > 
> > >So, I'll create a /* DEV */ prefix?
> > 
> >    I really don't know. Other places have /* MSTP */ comment in this
> > case despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are
> > really MSTP clocks. I considered the idea of separating
> > CLKDEV_ICK_ID() under /* ICK */ comment silly from the very start
> > but Simon didn't listen to me.
> 
> I am puzzled, too. ICK is a type of registration and not a clock domain.
> Also, there is 'mtu2_fck' which is under ICK as well as MSTP? Looks
> wrong. From what I understand now, removing the /* ICK */ comment would
> be easiest and proper?

I'm not sure that I really understand what all the fuss is about.

As I understand things the convention that prevails for
MSTP clocks under mach-shmobile is as follows:

1. Clocks not registered by CLKDEV_ICK_ID() are grouped together
   under /* MSTP */ followed by:
2. Clocks registered using CLKDEV_ICK_ID() are grouped together
   under /* ICK */

I am unsure of the historical reason for this but it does
seem to be consistent.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Dec. 18, 2013, 2:02 p.m. UTC | #6
Hello.

On 18-12-2013 17:49, Simon Horman wrote:

>>>>>> @@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
>>>>>>   	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),

>>>>>>   	/* ICK */
>>>>>> +	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
>>>>>> +	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
>>>>>> +	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
>>>>>> +	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),

>>>>>     These belong to some other place, the group marked by /* ICK */
>>>>> is only for CLKDEV_ICK_ID().

>>>> So, I'll create a /* DEV */ prefix?

>>>     I really don't know. Other places have /* MSTP */ comment in this
>>> case despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are
>>> really MSTP clocks. I considered the idea of separating
>>> CLKDEV_ICK_ID() under /* ICK */ comment silly from the very start
>>> but Simon didn't listen to me.

>> I am puzzled, too. ICK is a type of registration and not a clock domain.
>> Also, there is 'mtu2_fck' which is under ICK as well as MSTP? Looks
>> wrong. From what I understand now, removing the /* ICK */ comment would
>> be easiest and proper?

> I'm not sure that I really understand what all the fuss is about.

> As I understand things the convention that prevails for
> MSTP clocks under mach-shmobile is as follows:

> 1. Clocks not registered by CLKDEV_ICK_ID() are grouped together
>     under /* MSTP */ followed by:
> 2. Clocks registered using CLKDEV_ICK_ID() are grouped together
>     under /* ICK */

> I am unsure of the historical reason for this

    Recent patches by Morimoto-san.

> but it does seem to be consistent.

    No, it doesn't. These comments are *clearly* not consistent and should be 
removed at least.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Magnus Damm Dec. 18, 2013, 2:44 p.m. UTC | #7
On Wed, Dec 18, 2013 at 11:02 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 18-12-2013 17:49, Simon Horman wrote:
>
>>>>>>> @@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
>>>>>>>         CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
>
>
>>>>>>>         /* ICK */
>>>>>>> +       CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
>>>>>>> +       CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
>>>>>>> +       CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
>>>>>>> +       CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
>
>
>>>>>>     These belong to some other place, the group marked by /* ICK */
>>>>>> is only for CLKDEV_ICK_ID().
>
>
>>>>> So, I'll create a /* DEV */ prefix?
>
>
>>>>     I really don't know. Other places have /* MSTP */ comment in this
>>>> case despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are
>>>> really MSTP clocks. I considered the idea of separating
>>>> CLKDEV_ICK_ID() under /* ICK */ comment silly from the very start
>>>> but Simon didn't listen to me.
>
>
>>> I am puzzled, too. ICK is a type of registration and not a clock domain.
>>> Also, there is 'mtu2_fck' which is under ICK as well as MSTP? Looks
>>> wrong. From what I understand now, removing the /* ICK */ comment would
>>> be easiest and proper?
>
>
>> I'm not sure that I really understand what all the fuss is about.
>
>
>> As I understand things the convention that prevails for
>> MSTP clocks under mach-shmobile is as follows:
>
>
>> 1. Clocks not registered by CLKDEV_ICK_ID() are grouped together
>>     under /* MSTP */ followed by:
>> 2. Clocks registered using CLKDEV_ICK_ID() are grouped together
>>     under /* ICK */
>
>
>> I am unsure of the historical reason for this
>
>
>    Recent patches by Morimoto-san.
>
>
>> but it does seem to be consistent.
>
>
>    No, it doesn't. These comments are *clearly* not consistent and should be
> removed at least.

Feel free to contribute patches!

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Dec. 18, 2013, 3:13 p.m. UTC | #8
Hello.

On 18-12-2013 18:44, Magnus Damm wrote:

>>>>>>>> @@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
>>>>>>>>          CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),

>>>>>>>>          /* ICK */
>>>>>>>> +       CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
>>>>>>>> +       CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
>>>>>>>> +       CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
>>>>>>>> +       CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),

>>>>>>>      These belong to some other place, the group marked by /* ICK */
>>>>>>> is only for CLKDEV_ICK_ID().

>>>>>> So, I'll create a /* DEV */ prefix?

>>>>>      I really don't know. Other places have /* MSTP */ comment in this
>>>>> case despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are
>>>>> really MSTP clocks. I considered the idea of separating
>>>>> CLKDEV_ICK_ID() under /* ICK */ comment silly from the very start
>>>>> but Simon didn't listen to me.

>>>> I am puzzled, too. ICK is a type of registration and not a clock domain.
>>>> Also, there is 'mtu2_fck' which is under ICK as well as MSTP? Looks
>>>> wrong. From what I understand now, removing the /* ICK */ comment would
>>>> be easiest and proper?

>>> I'm not sure that I really understand what all the fuss is about.

>>> As I understand things the convention that prevails for
>>> MSTP clocks under mach-shmobile is as follows:

>>> 1. Clocks not registered by CLKDEV_ICK_ID() are grouped together
>>>      under /* MSTP */ followed by:
>>> 2. Clocks registered using CLKDEV_ICK_ID() are grouped together
>>>      under /* ICK */

>>> I am unsure of the historical reason for this

>>     Recent patches by Morimoto-san.

>>> but it does seem to be consistent.

>>     No, it doesn't. These comments are *clearly* not consistent and should be
>> removed at least.

> Feel free to contribute patches!

    Of course, in my copious free time. I was against these ICKy comments (and 
the patches introducing them) in the first place but my opinion didn't count. 
I'm not sure it will count if I go and submit the patches (but the time will 
be lost).

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang Dec. 18, 2013, 9 p.m. UTC | #9
> I am unsure of the historical reason for this but it does
> seem to be consistent.

Okay, I'll follow that.
diff mbox

Patch

diff --git a/arch/arm/mach-shmobile/clock-r7s72100.c b/arch/arm/mach-shmobile/clock-r7s72100.c
index 7b457ae..770316d 100644
--- a/arch/arm/mach-shmobile/clock-r7s72100.c
+++ b/arch/arm/mach-shmobile/clock-r7s72100.c
@@ -27,6 +27,7 @@ 
 #define FRQCR2		0xfcfe0014
 #define STBCR3		0xfcfe0420
 #define STBCR4		0xfcfe0424
+#define STBCR9		0xfcfe0438
 
 #define PLL_RATE 30
 
@@ -144,10 +145,15 @@  struct clk div4_clks[DIV4_NR] = {
 					| CLK_ENABLE_ON_INIT),
 };
 
-enum { MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
+enum {	MSTP97, MSTP96, MSTP95, MSTP94,
+	MSTP47, MSTP46, MSTP45, MSTP44, MSTP43, MSTP42, MSTP41, MSTP40,
 	MSTP33,	MSTP_NR };
 
 static struct clk mstp_clks[MSTP_NR] = {
+	[MSTP97] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 7, 0), /* RIIC0 */
+	[MSTP96] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 6, 0), /* RIIC1 */
+	[MSTP95] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 5, 0), /* RIIC2 */
+	[MSTP94] = SH_CLK_MSTP8(&peripheral0_clk, STBCR9, 4, 0), /* RIIC3 */
 	[MSTP47] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 7, 0), /* SCIF0 */
 	[MSTP46] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 6, 0), /* SCIF1 */
 	[MSTP45] = SH_CLK_MSTP8(&peripheral1_clk, STBCR4, 5, 0), /* SCIF2 */
@@ -173,6 +179,10 @@  static struct clk_lookup lookups[] = {
 	CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),
 
 	/* ICK */
+	CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
+	CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
+	CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
+	CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),
 	CLKDEV_ICK_ID("sci_fck", "sh-sci.0", &mstp_clks[MSTP47]),
 	CLKDEV_ICK_ID("sci_fck", "sh-sci.1", &mstp_clks[MSTP46]),
 	CLKDEV_ICK_ID("sci_fck", "sh-sci.2", &mstp_clks[MSTP45]),