| Message ID | 1387316678-10174-3-git-send-email-wsa@the-dreams.de |
|---|---|
| State | Superseded |
| Headers | show |
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-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >@@ -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?
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-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I am unsure of the historical reason for this but it does > seem to be consistent. Okay, I'll follow that.
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]),