* Re: [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order
2013-11-19 9:05 [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order Kuninori Morimoto
@ 2013-11-19 17:56 ` Sergei Shtylyov
2013-11-20 0:24 ` Simon Horman
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2013-11-19 17:56 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order
2013-11-19 9:05 [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order Kuninori Morimoto
2013-11-19 17:56 ` Sergei Shtylyov
@ 2013-11-20 0:24 ` Simon Horman
2013-11-20 18:26 ` Sergei Shtylyov
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2013-11-20 0:24 UTC (permalink / raw)
To: linux-sh
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order
2013-11-19 9:05 [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order Kuninori Morimoto
2013-11-19 17:56 ` Sergei Shtylyov
2013-11-20 0:24 ` Simon Horman
@ 2013-11-20 18:26 ` Sergei Shtylyov
2013-11-21 1:35 ` Kuninori Morimoto
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2013-11-20 18:26 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order
2013-11-19 9:05 [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order Kuninori Morimoto
` (2 preceding siblings ...)
2013-11-20 18:26 ` Sergei Shtylyov
@ 2013-11-21 1:35 ` Kuninori Morimoto
2013-11-21 16:22 ` Sergei Shtylyov
2013-11-25 0:17 ` Kuninori Morimoto
5 siblings, 0 replies; 7+ messages in thread
From: Kuninori Morimoto @ 2013-11-21 1:35 UTC (permalink / raw)
To: linux-sh
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.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order
2013-11-19 9:05 [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order Kuninori Morimoto
` (3 preceding siblings ...)
2013-11-21 1:35 ` Kuninori Morimoto
@ 2013-11-21 16:22 ` Sergei Shtylyov
2013-11-25 0:17 ` Kuninori Morimoto
5 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2013-11-21 16:22 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order
2013-11-19 9:05 [PATCH 5/6] ARM: shmobile: r8a7790: tidyup clock table order Kuninori Morimoto
` (4 preceding siblings ...)
2013-11-21 16:22 ` Sergei Shtylyov
@ 2013-11-25 0:17 ` Kuninori Morimoto
5 siblings, 0 replies; 7+ messages in thread
From: Kuninori Morimoto @ 2013-11-25 0:17 UTC (permalink / raw)
To: linux-sh
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 ?
^ permalink raw reply [flat|nested] 7+ messages in thread