clk: renesas: r8a774c0: Fix LAST_DT_CORE_CLK

Commit Message

Fabrizio Castro Feb. 20, 2019, 4:09 p.m. UTC

Enum LAST_DT_CORE_CLK needs updating as R8A774C0_CLK_CANFD
was recently added and it's the core clock with the highest
index.

Signed-off-by: Fabrizio Castro <[e-mail]>
---
 drivers/clk/renesas/r8a774c0-cpg-mssr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Paterson Feb. 20, 2019, 8:39 p.m. UTC | #1

> From: Fabrizio Castro <[e-mail]>
> Sent: 20 February 2019 16:09
> To: Geert Uytterhoeven <[e-mail]>
> Cc: Fabrizio Castro <[e-mail]>; Michael Turquette
> <[e-mail]>; Stephen Boyd <[e-mail]>; linux-
> [e-mail]; [e-mail]; Simon Horman
> <[e-mail]>; Chris Paterson <[e-mail]>;
> Biju Das <[e-mail]>
> Subject: [PATCH] clk: renesas: r8a774c0: Fix LAST_DT_CORE_CLK
> 
> Enum LAST_DT_CORE_CLK needs updating as R8A774C0_CLK_CANFD
> was recently added and it's the core clock with the highest
> index.
> 
> Signed-off-by: Fabrizio Castro <[e-mail]>
Reviewed-by: Chris Paterson <[e-mail]>

> ---
>  drivers/clk/renesas/r8a774c0-cpg-mssr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/renesas/r8a774c0-cpg-mssr.c
> b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
> index 4f3111b..48a578a 100644
> --- a/drivers/clk/renesas/r8a774c0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
> @@ -22,7 +22,7 @@
> 
>  enum clk_ids {
>  	/* Core Clock Outputs exported to DT */
> -	LAST_DT_CORE_CLK = R8A774C0_CLK_CPEX,
> +	LAST_DT_CORE_CLK = R8A774C0_CLK_CANFD,
> 
>  	/* External Input Clocks */
>  	CLK_EXTAL,
> --
> 2.7.4

Geert Uytterhoeven Feb. 21, 2019, 6:33 p.m. UTC | #2

On Wed, Feb 20, 2019 at 5:09 PM Fabrizio Castro
<[e-mail]> wrote:
> Enum LAST_DT_CORE_CLK needs updating as R8A774C0_CLK_CANFD
> was recently added and it's the core clock with the highest
> index.
>
> Signed-off-by: Fabrizio Castro <[e-mail]>

Nice catch!
You only notice this kind of bug when you start referencing the clock
from DT, right? At registration time, the clock number space is shared with
internal core clocks, so no error is detected.
Can we improve that?

Fixes: 2a6efbc6da5d248c ("clk: renesas: r8a774c0: Add missing CANFD clock")
Reviewed-by: Geert Uytterhoeven <[e-mail]>

Mike/Stephen: can you please take this one directly, as it is a fix for
clk-next?

Fabrizio: Looks like R8A774A1 has the same bug. Care to fix that as well?

Thanks!

Gr{oetje,eeting}s,

                        Geert

Stephen Boyd Feb. 21, 2019, 9:10 p.m. UTC | #3

Quoting Geert Uytterhoeven (2019-02-21 10:33:42)
> On Wed, Feb 20, 2019 at 5:09 PM Fabrizio Castro
> <[e-mail]> wrote:
> > Enum LAST_DT_CORE_CLK needs updating as R8A774C0_CLK_CANFD
> > was recently added and it's the core clock with the highest
> > index.
> >
> > Signed-off-by: Fabrizio Castro <[e-mail]>
> 
> Nice catch!
> You only notice this kind of bug when you start referencing the clock
> from DT, right? At registration time, the clock number space is shared with
> internal core clocks, so no error is detected.
> Can we improve that?
> 
> Fixes: 2a6efbc6da5d248c ("clk: renesas: r8a774c0: Add missing CANFD clock")
> Reviewed-by: Geert Uytterhoeven <[e-mail]>
> 
> Mike/Stephen: can you please take this one directly, as it is a fix for
> clk-next?
> 

Yep. Applied to clk-renesas and merged up.

Fabrizio Castro Feb. 22, 2019, 10:12 a.m. UTC | #4

Hello Geert,

> From: [e-mail] <[e-mail]> On Behalf Of Geert Uytterhoeven
> Sent: 21 February 2019 18:34
> To: Fabrizio Castro <[e-mail]>
> Subject: Re: [PATCH] clk: renesas: r8a774c0: Fix LAST_DT_CORE_CLK
> 
> On Wed, Feb 20, 2019 at 5:09 PM Fabrizio Castro
> <[e-mail]> wrote:
> > Enum LAST_DT_CORE_CLK needs updating as R8A774C0_CLK_CANFD
> > was recently added and it's the core clock with the highest
> > index.
> >
> > Signed-off-by: Fabrizio Castro <[e-mail]>
> 
> Nice catch!
> You only notice this kind of bug when you start referencing the clock
> from DT, right? At registration time, the clock number space is shared with
> internal core clocks, so no error is detected.
> Can we improve that?

I can look into it

> 
> Fixes: 2a6efbc6da5d248c ("clk: renesas: r8a774c0: Add missing CANFD clock")

Commit 2a6efbc6da5d248c is not in a release or in a rc, is it ok to refer to it with a fixes tag?

> Reviewed-by: Geert Uytterhoeven <[e-mail]>
> 
> Mike/Stephen: can you please take this one directly, as it is a fix for
> clk-next?
> 
> Fabrizio: Looks like R8A774A1 has the same bug. Care to fix that as well?

Will do

Thanks,
Fab

> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Fabrizio Castro Feb. 22, 2019, 12:03 p.m. UTC | #5

Hello Geert,

> From: [e-mail] <[e-mail]> On Behalf Of Fabrizio Castro
> Sent: 22 February 2019 10:12
> Subject: RE: [PATCH] clk: renesas: r8a774c0: Fix LAST_DT_CORE_CLK
> 
> Hello Geert,
> 
> > From: [e-mail] <[e-mail]> On Behalf Of Geert Uytterhoeven
> > Sent: 21 February 2019 18:34
> > To: Fabrizio Castro <[e-mail]>
> > Subject: Re: [PATCH] clk: renesas: r8a774c0: Fix LAST_DT_CORE_CLK
> >
> > On Wed, Feb 20, 2019 at 5:09 PM Fabrizio Castro
> > <[e-mail]> wrote:
> > > Enum LAST_DT_CORE_CLK needs updating as R8A774C0_CLK_CANFD
> > > was recently added and it's the core clock with the highest
> > > index.
> > >
> > > Signed-off-by: Fabrizio Castro <[e-mail]>
> >
> > Nice catch!
> > You only notice this kind of bug when you start referencing the clock
> > from DT, right? At registration time, the clock number space is shared with
> > internal core clocks, so no error is detected.
> > Can we improve that?
> 
> I can look into it

Actually, it seems like I am a bit tight on time, not sure I am going to have the chance to look into this, sorry Geert

Thanks,
Fab

> 
> >
> > Fixes: 2a6efbc6da5d248c ("clk: renesas: r8a774c0: Add missing CANFD clock")
> 
> Commit 2a6efbc6da5d248c is not in a release or in a rc, is it ok to refer to it with a fixes tag?
> 
> > Reviewed-by: Geert Uytterhoeven <[e-mail]>
> >
> > Mike/Stephen: can you please take this one directly, as it is a fix for
> > clk-next?
> >
> > Fabrizio: Looks like R8A774A1 has the same bug. Care to fix that as well?
> 
> Will do
> 
> Thanks,
> Fab
> 
> >
> > Thanks!
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]
> >
> > In personal conversations with technical people, I call myself a hacker. But
> > when I'm talking to journalists I just say "programmer" or something like that.
> >                                 -- Linus Torvalds

Patch

diff --git a/drivers/clk/renesas/r8a774c0-cpg-mssr.c b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
index 4f3111b..48a578a 100644
--- a/drivers/clk/renesas/r8a774c0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
@@ -22,7 +22,7 @@ 
 
 enum clk_ids {
 	/* Core Clock Outputs exported to DT */
-	LAST_DT_CORE_CLK = R8A774C0_CLK_CPEX,
+	LAST_DT_CORE_CLK = R8A774C0_CLK_CANFD,
 
 	/* External Input Clocks */
 	CLK_EXTAL,