| Message ID | 8738msep7z.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | 372bc732cdad3f728f7225bd8427b55d9ee9358a |
| Headers | show |
Hello. On 19-11-2013 13:05, Kuninori Morimoto wrote: > SuperH lookups clock is using CLKDEV_CON/DEV/ICK_ID() macro > for a long term. > But in these days, the ICK clock is defined in random place. > This patch arranges it. > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > arch/arm/mach-shmobile/clock-r8a7790.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c > index 36e1a2d..d962436 100644 > --- a/arch/arm/mach-shmobile/clock-r8a7790.c > +++ b/arch/arm/mach-shmobile/clock-r8a7790.c > @@ -266,11 +266,6 @@ static struct clk_lookup lookups[] = { > CLKDEV_CON_ID("ssprs", &div6_clks[DIV6_SSPRS]), > > /* MSTP */ > - CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), > - CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), > - CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), > - CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), > - CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), > CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP204]), > CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP203]), > CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP206]), > @@ -302,7 +297,15 @@ static struct clk_lookup lookups[] = { > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP704]), > + > + /* ICK */ How this corresponds to the above MSTP comment? > CLKDEV_ICK_ID("usbhs", "usb_phy_rcar_gen2", &mstp_clks[MSTP704]), > + CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), > + CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), > + CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), > + CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), > + CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), > + > }; 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
On Tue, Nov 19, 2013 at 09:56:52PM +0400, Sergei Shtylyov wrote: > Hello. > > On 19-11-2013 13:05, Kuninori Morimoto wrote: > > >SuperH lookups clock is using CLKDEV_CON/DEV/ICK_ID() macro > >for a long term. > >But in these days, the ICK clock is defined in random place. > >This patch arranges it. > > >Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > >--- > > arch/arm/mach-shmobile/clock-r8a7790.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > >diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c > >index 36e1a2d..d962436 100644 > >--- a/arch/arm/mach-shmobile/clock-r8a7790.c > >+++ b/arch/arm/mach-shmobile/clock-r8a7790.c > >@@ -266,11 +266,6 @@ static struct clk_lookup lookups[] = { > > CLKDEV_CON_ID("ssprs", &div6_clks[DIV6_SSPRS]), > > > > /* MSTP */ > >- CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), > >- CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), > >- CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), > >- CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), > >- CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), > > CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP204]), > > CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP203]), > > CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP206]), > >@@ -302,7 +297,15 @@ static struct clk_lookup lookups[] = { > > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > > CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > > CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP704]), > >+ > >+ /* ICK */ > > How this corresponds to the above MSTP comment? My understanding is that the motivation of this patch is to group together ICK clock initialisation as it is done using the CLKDEV_ICK_ID() macro rather than the CLKDEV_ICK_ID() macro used for other MSTP clocks. In this context I think the comment makes sense. > > > CLKDEV_ICK_ID("usbhs", "usb_phy_rcar_gen2", &mstp_clks[MSTP704]), > >+ CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), > >+ CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), > >+ CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), > >+ CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), > >+ CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), > >+ > > }; > > 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 > -- 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
Hello. On 20-11-2013 4:24, Simon Horman wrote: >>> SuperH lookups clock is using CLKDEV_CON/DEV/ICK_ID() macro >>> for a long term. >>> But in these days, the ICK clock is defined in random place. >>> This patch arranges it. >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> >>> --- >>> arch/arm/mach-shmobile/clock-r8a7790.c | 13 ++++++++----- >>> 1 file changed, 8 insertions(+), 5 deletions(-) >>> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c >>> index 36e1a2d..d962436 100644 >>> --- a/arch/arm/mach-shmobile/clock-r8a7790.c >>> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c >>> @@ -266,11 +266,6 @@ static struct clk_lookup lookups[] = { >>> CLKDEV_CON_ID("ssprs", &div6_clks[DIV6_SSPRS]), >>> >>> /* MSTP */ >>> - CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), >>> - CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), >>> - CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), >>> - CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), >>> - CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), >>> CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP204]), >>> CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP203]), >>> CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP206]), >>> @@ -302,7 +297,15 @@ static struct clk_lookup lookups[] = { >>> CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), >>> CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), >>> CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP704]), >>> + >>> + /* ICK */ >> How this corresponds to the above MSTP comment? > My understanding is that the motivation of this patch is to > group together ICK clock initialisation as it is > done using the CLKDEV_ICK_ID() macro rather than the > CLKDEV_ICK_ID() macro used for other MSTP clocks. What? :-) > In this context I think the comment makes sense. I disagree because the clocks declared via CLKDEV_ICK_ID() are by nature the same MSTP clocks as the others. This patch looks dangerously close to the needless churn IMHO. >>> CLKDEV_ICK_ID("usbhs", "usb_phy_rcar_gen2", &mstp_clks[MSTP704]), >>> + CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), >>> + CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), >>> + CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), >>> + CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), >>> + CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), >>> + >>> }; 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
Hi > > In this context I think the comment makes sense. > > I disagree because the clocks declared via CLKDEV_ICK_ID() are by nature > the same MSTP clocks as the others. This patch looks dangerously close to the > needless churn IMHO. From MSTP grouping point of view, you are correct. From PM-Runtime/Clock API point of view, this patch is sane. -- 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
Hello. On 21-11-2013 5:35, Kuninori Morimoto wrote: >>> In this context I think the comment makes sense. >> I disagree because the clocks declared via CLKDEV_ICK_ID() are by nature >> the same MSTP clocks as the others. This patch looks dangerously close to the >> needless churn IMHO. > From MSTP grouping point of view, you are correct. > From PM-Runtime/Clock API point of view, this patch is sane. Please explain what it adds from this PoV. And if this patch is going to be accepted, I at least would like the dubious "ICK" comment to be removed. 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
Hi Sergei > >> I disagree because the clocks declared via CLKDEV_ICK_ID() are by nature > >> the same MSTP clocks as the others. This patch looks dangerously close to the > >> needless churn IMHO. > > > From MSTP grouping point of view, you are correct. > > From PM-Runtime/Clock API point of view, this patch is sane. > > Please explain what it adds from this PoV. And if this patch is going to > be accepted, I at least would like the dubious "ICK" comment to be removed. Your opinion which is focusing about MSTP grouping is only for r8a7790. This patch-set is focusing whole SH-ARM SoC. We don't need minor rule which breaks source code uniformity. I don't know what is the best comment instead of "ICK", but, kernal hacker can understand without full explanation ? -- 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
diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c index 36e1a2d..d962436 100644 --- a/arch/arm/mach-shmobile/clock-r8a7790.c +++ b/arch/arm/mach-shmobile/clock-r8a7790.c @@ -266,11 +266,6 @@ static struct clk_lookup lookups[] = { CLKDEV_CON_ID("ssprs", &div6_clks[DIV6_SSPRS]), /* MSTP */ - CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), - CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), - CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), - CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), - CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), CLKDEV_DEV_ID("sh-sci.0", &mstp_clks[MSTP204]), CLKDEV_DEV_ID("sh-sci.1", &mstp_clks[MSTP203]), CLKDEV_DEV_ID("sh-sci.2", &mstp_clks[MSTP206]), @@ -302,7 +297,15 @@ static struct clk_lookup lookups[] = { CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), CLKDEV_DEV_ID("renesas_usbhs", &mstp_clks[MSTP704]), + + /* ICK */ CLKDEV_ICK_ID("usbhs", "usb_phy_rcar_gen2", &mstp_clks[MSTP704]), + CLKDEV_ICK_ID("lvds.0", "rcar-du-r8a7790", &mstp_clks[MSTP726]), + CLKDEV_ICK_ID("lvds.1", "rcar-du-r8a7790", &mstp_clks[MSTP725]), + CLKDEV_ICK_ID("du.0", "rcar-du-r8a7790", &mstp_clks[MSTP724]), + CLKDEV_ICK_ID("du.1", "rcar-du-r8a7790", &mstp_clks[MSTP723]), + CLKDEV_ICK_ID("du.2", "rcar-du-r8a7790", &mstp_clks[MSTP722]), + }; #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31) \
SuperH lookups clock is using CLKDEV_CON/DEV/ICK_ID() macro for a long term. But in these days, the ICK clock is defined in random place. This patch arranges it. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- arch/arm/mach-shmobile/clock-r8a7790.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)