[PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current cpg_sd01_div_table is missing
"0x0100: x 1/8" division ratio.
This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.

Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
>> Simon
Actually,
	linux/arch/arm/mach-shmobile/clock-r8a7790.c : div4_clks
	linux/arch/arm/mach-shmobile/clock-r8a7791.c : div4_clks
have same issue, but I ignored

 drivers/clk/shmobile/clk-rcar-gen2.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
index dff7f79..e996425 100644
--- a/drivers/clk/shmobile/clk-rcar-gen2.c
+++ b/drivers/clk/shmobile/clk-rcar-gen2.c
@@ -202,6 +202,7 @@ static const struct clk_div_table cpg_sdh_div_table[] = {
 };
 
 static const struct clk_div_table cpg_sd01_div_table[] = {
+	{  4,  8 },
 	{  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
 	{ 10, 36 }, { 11, 48 }, { 12, 10 }, {  0,  0 },
 };
-- 
1.7.9.5

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Simon Horman]

On Tue, Aug 05, 2014 at 12:09:26AM -0700, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current cpg_sd01_div_table is missing
> "0x0100: x 1/8" division ratio.
> This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.
> 
> Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> >> Simon
> Actually,
> 	linux/arch/arm/mach-shmobile/clock-r8a7790.c : div4_clks
> 	linux/arch/arm/mach-shmobile/clock-r8a7791.c : div4_clks
> have same issue, but I ignored
> 
>  drivers/clk/shmobile/clk-rcar-gen2.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
> index dff7f79..e996425 100644
> --- a/drivers/clk/shmobile/clk-rcar-gen2.c
> +++ b/drivers/clk/shmobile/clk-rcar-gen2.c
> @@ -202,6 +202,7 @@ static const struct clk_div_table cpg_sdh_div_table[] = {
>  };
>  
>  static const struct clk_div_table cpg_sd01_div_table[] = {
> +	{  4,  8 },
>  	{  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
>  	{ 10, 36 }, { 11, 48 }, { 12, 10 }, {  0,  0 },
>  };

Thanks. I have a few questions about this.

1. Could you provide information on the patch that added this problem
   and the kernel version it was added in?

2. Is this a bug-fix?
   If so, does it manifest in mainline?
   If so what happens?

3. Should we also fix clock-r8a7790.c and clock-r8a7791.c?
   If so I can make a patch or I am happy for you to do so.

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Geert Uytterhoeven]

On Tue, Aug 5, 2014 at 9:09 AM, Kuninori Morimoto
<kuninori.morimoto.gx@gmail.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Current cpg_sd01_div_table is missing
> "0x0100: x 1/8" division ratio.
> This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.
>
> Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

BTW, has this been verified by testing, or is it purely based on the datasheet?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Kuninori Morimoto]

Hi Simon

Thank you for your feedback

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Current cpg_sd01_div_table is missing
> > "0x0100: x 1/8" division ratio.
> > This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.
> > 
> > Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
(snip)
> >  static const struct clk_div_table cpg_sd01_div_table[] = {
> > +	{  4,  8 },
> >  	{  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
> >  	{ 10, 36 }, { 11, 48 }, { 12, 10 }, {  0,  0 },
> >  };
> 
> Thanks. I have a few questions about this.
> 
> 1. Could you provide information on the patch that added this problem
>    and the kernel version it was added in?

I see.
It is added on v3.14.
I can send v2 patch (or can you add it on log area ?)

> 2. Is this a bug-fix?
>    If so, does it manifest in mainline?
>    If so what happens?

This fixes hidden bug.
No board/SoC doesn't have issue without this patch.

> 3. Should we also fix clock-r8a7790.c and clock-r8a7791.c?
>    If so I can make a patch or I am happy for you to do so.

I can do it, but is it possible ?
I'm afraid that clock-r8axxx is used for legacy (= non-DT)
But, I'm happy to send it if you say "Yes"

Best regards
---
Kuninori Morimoto

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Kuninori Morimoto]

Hi Geert

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >
> > Current cpg_sd01_div_table is missing
> > "0x0100: x 1/8" division ratio.
> > This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.
> >
> > Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> BTW, has this been verified by testing, or is it purely based on the datasheet?

Thank you for your feedback.
I can understand your distrust :)
I just checked datasheet, but, BSP team had tested it.

Best regards
---
Kuninori Morimoto

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Laurent Pinchart]

Hi Morimoto-san,

Thank you for the patch.

On Tuesday 05 August 2014 00:09:26 Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current cpg_sd01_div_table is missing
> "0x0100: x 1/8" division ratio.
> This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.
> 
> Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
> >> Simon
> 
> Actually,
> 	linux/arch/arm/mach-shmobile/clock-r8a7790.c : div4_clks
> 	linux/arch/arm/mach-shmobile/clock-r8a7791.c : div4_clks
> have same issue, but I ignored
> 
>  drivers/clk/shmobile/clk-rcar-gen2.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c
> b/drivers/clk/shmobile/clk-rcar-gen2.c index dff7f79..e996425 100644
> --- a/drivers/clk/shmobile/clk-rcar-gen2.c
> +++ b/drivers/clk/shmobile/clk-rcar-gen2.c
> @@ -202,6 +202,7 @@ static const struct clk_div_table cpg_sdh_div_table[] > { };
> 
>  static const struct clk_div_table cpg_sd01_div_table[] = {
> +	{  4,  8 },
>  	{  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
>  	{ 10, 36 }, { 11, 48 }, { 12, 10 }, {  0,  0 },
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Simon Horman]

On Tue, Aug 05, 2014 at 01:51:12AM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> Thank you for your feedback
> 
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > 
> > > Current cpg_sd01_div_table is missing
> > > "0x0100: x 1/8" division ratio.
> > > This patch is based on R-Car H2 v0.7, R-Car M2 v0.9.
> > > 
> > > Reported-by: Yusuke Goda <yusuke.goda.sx@renesas.com>
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > ---
> (snip)
> > >  static const struct clk_div_table cpg_sd01_div_table[] = {
> > > +	{  4,  8 },
> > >  	{  5, 12 }, {  6, 16 }, {  7, 18 }, {  8, 24 },
> > >  	{ 10, 36 }, { 11, 48 }, { 12, 10 }, {  0,  0 },
> > >  };
> > 
> > Thanks. I have a few questions about this.
> > 
> > 1. Could you provide information on the patch that added this problem
> >    and the kernel version it was added in?
> 
> I see.
> It is added on v3.14.
> I can send v2 patch (or can you add it on log area ?)
> 
> > 2. Is this a bug-fix?
> >    If so, does it manifest in mainline?
> >    If so what happens?
> 
> This fixes hidden bug.
> No board/SoC doesn't have issue without this patch.

Thanks. Please send v2 with the above information.
Also please include the id and subject of the commit that added
the problem. e.g. 0123456789abcdef ("clk: shmobile: blah").

And I think that this patch needs to go via
Mike Turquette <mturquette@linaro.org> (CCed),
so please include him when you post v2.

> > 3. Should we also fix clock-r8a7790.c and clock-r8a7791.c?
> >    If so I can make a patch or I am happy for you to do so.
> 
> I can do it, but is it possible ?
> I'm afraid that clock-r8axxx is used for legacy (= non-DT)
> But, I'm happy to send it if you say "Yes"

Good point.

My not very strong feeling at this point is that if it is a fix then we
should add it.  Even if it is legacy code.

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Kuninori Morimoto]

Hi Simon

> > It is added on v3.14.
> > I can send v2 patch (or can you add it on log area ?)
(snip)
> > This fixes hidden bug.
> > No board/SoC doesn't have issue without this patch.
> 
> Thanks. Please send v2 with the above information.
> Also please include the id and subject of the commit that added
> the problem. e.g. 0123456789abcdef ("clk: shmobile: blah").
> 
> And I think that this patch needs to go via
> Mike Turquette <mturquette@linaro.org> (CCed),
> so please include him when you post v2.
> 
> > > 3. Should we also fix clock-r8a7790.c and clock-r8a7791.c?
> > >    If so I can make a patch or I am happy for you to do so.
> > 
> > I can do it, but is it possible ?
> > I'm afraid that clock-r8axxx is used for legacy (= non-DT)
> > But, I'm happy to send it if you say "Yes"
> 
> Good point.
> 
> My not very strong feeling at this point is that if it is a fix then we
> should add it.  Even if it is legacy code.

I see, will send v2 which includes legacy side fix patch

Best regards
---
Kuninori Morimoto

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Simon Horman]

On Tue, Aug 05, 2014 at 05:39:44PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > It is added on v3.14.
> > > I can send v2 patch (or can you add it on log area ?)
> (snip)
> > > This fixes hidden bug.
> > > No board/SoC doesn't have issue without this patch.
> > 
> > Thanks. Please send v2 with the above information.
> > Also please include the id and subject of the commit that added
> > the problem. e.g. 0123456789abcdef ("clk: shmobile: blah").
> > 
> > And I think that this patch needs to go via
> > Mike Turquette <mturquette@linaro.org> (CCed),
> > so please include him when you post v2.
> > 
> > > > 3. Should we also fix clock-r8a7790.c and clock-r8a7791.c?
> > > >    If so I can make a patch or I am happy for you to do so.
> > > 
> > > I can do it, but is it possible ?
> > > I'm afraid that clock-r8axxx is used for legacy (= non-DT)
> > > But, I'm happy to send it if you say "Yes"
> > 
> > Good point.
> > 
> > My not very strong feeling at this point is that if it is a fix then we
> > should add it.  Even if it is legacy code.
> 
> I see, will send v2 which includes legacy side fix patch

Thanks. Please send separate patches for each file that updated.
I believe they should all be independent of each other.

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Kuninori Morimoto]

Hi Simon

> > I see, will send v2 which includes legacy side fix patch
> 
> Thanks. Please send separate patches for each file that updated.
> I believe they should all be independent of each other.

Bad timing :<
I already posted v2 patch few minute ago

Re: [PATCH] clk: shmobile: add missing 0x0100 for SDCKCR

[Simon Horman]

On Tue, Aug 05, 2014 at 06:32:20PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > I see, will send v2 which includes legacy side fix patch
> > 
> > Thanks. Please send separate patches for each file that updated.
> > I believe they should all be independent of each other.
> 
> Bad timing :<
> I already posted v2 patch few minute ago

Thanks, it looks good :)