diff mbox series

[v3,1/1] clk: jz4725b: fix mmc0 clock gating

Message ID 20220205094531.676371-2-lis8215@gmail.com (mailing list archive)
State Superseded
Headers show
Series clk: jz4725b: fix mmc0 clock gating | expand

Commit Message

Siarhei Volkau Feb. 5, 2022, 9:45 a.m. UTC
The mmc0 clock gate bit was mistakenly assigned to "i2s" clock.
You can find that the same bit is assigned to "mmc0" too.
It leads to mmc0 hang for a long time after any sound activity
also it  prevented PM_SLEEP to work properly.
I guess it was introduced by copy-paste from jz4740 driver
where it is really controls I2S clock gate.

Fixes: 226dfa4726eb ("clk: Add Ingenic jz4725b CGU driver")
Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
Tested-by: Siarhei Volkau <lis8215@gmail.com>
Reviewed-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/clk/ingenic/jz4725b-cgu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Greg KH Feb. 5, 2022, 9:59 a.m. UTC | #1
On Sat, Feb 05, 2022 at 12:45:31PM +0300, Siarhei Volkau wrote:
> The mmc0 clock gate bit was mistakenly assigned to "i2s" clock.
> You can find that the same bit is assigned to "mmc0" too.
> It leads to mmc0 hang for a long time after any sound activity
> also it  prevented PM_SLEEP to work properly.
> I guess it was introduced by copy-paste from jz4740 driver
> where it is really controls I2S clock gate.
> 
> Fixes: 226dfa4726eb ("clk: Add Ingenic jz4725b CGU driver")
> Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> Tested-by: Siarhei Volkau <lis8215@gmail.com>
> Reviewed-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/clk/ingenic/jz4725b-cgu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/ingenic/jz4725b-cgu.c b/drivers/clk/ingenic/jz4725b-cgu.c
> index 744d136..15d6179 100644
> --- a/drivers/clk/ingenic/jz4725b-cgu.c
> +++ b/drivers/clk/ingenic/jz4725b-cgu.c
> @@ -139,11 +139,10 @@ static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
>  	},
>  
>  	[JZ4725B_CLK_I2S] = {
> -		"i2s", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
> +		"i2s", CGU_CLK_MUX | CGU_CLK_DIV,
>  		.parents = { JZ4725B_CLK_EXT, JZ4725B_CLK_PLL_HALF, -1, -1 },
>  		.mux = { CGU_REG_CPCCR, 31, 1 },
>  		.div = { CGU_REG_I2SCDR, 0, 1, 9, -1, -1, -1 },
> -		.gate = { CGU_REG_CLKGR, 6 },
>  	},
>  
>  	[JZ4725B_CLK_SPI] = {
> -- 
> 2.35.1
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>
Paul Cercueil Feb. 5, 2022, 12:15 p.m. UTC | #2
Hi Greg,

Le sam., févr. 5 2022 at 10:59:50 +0100, Greg KH 
<gregkh@linuxfoundation.org> a écrit :
> On Sat, Feb 05, 2022 at 12:45:31PM +0300, Siarhei Volkau wrote:
>>  The mmc0 clock gate bit was mistakenly assigned to "i2s" clock.
>>  You can find that the same bit is assigned to "mmc0" too.
>>  It leads to mmc0 hang for a long time after any sound activity
>>  also it  prevented PM_SLEEP to work properly.
>>  I guess it was introduced by copy-paste from jz4740 driver
>>  where it is really controls I2S clock gate.
>> 
>>  Fixes: 226dfa4726eb ("clk: Add Ingenic jz4725b CGU driver")
>>  Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
>>  Tested-by: Siarhei Volkau <lis8215@gmail.com>
>>  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/clk/ingenic/jz4725b-cgu.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>>  diff --git a/drivers/clk/ingenic/jz4725b-cgu.c 
>> b/drivers/clk/ingenic/jz4725b-cgu.c
>>  index 744d136..15d6179 100644
>>  --- a/drivers/clk/ingenic/jz4725b-cgu.c
>>  +++ b/drivers/clk/ingenic/jz4725b-cgu.c
>>  @@ -139,11 +139,10 @@ static const struct ingenic_cgu_clk_info 
>> jz4725b_cgu_clocks[] = {
>>   	},
>> 
>>   	[JZ4725B_CLK_I2S] = {
>>  -		"i2s", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
>>  +		"i2s", CGU_CLK_MUX | CGU_CLK_DIV,
>>   		.parents = { JZ4725B_CLK_EXT, JZ4725B_CLK_PLL_HALF, -1, -1 },
>>   		.mux = { CGU_REG_CPCCR, 31, 1 },
>>   		.div = { CGU_REG_I2SCDR, 0, 1, 9, -1, -1, -1 },
>>  -		.gate = { CGU_REG_CLKGR, 6 },
>>   	},
>> 
>>   	[JZ4725B_CLK_SPI] = {
>>  --
>>  2.35.1
>> 
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

What's wrong with this patch exactly? It looks good to me.

Cheers,
-Paul
Greg KH Feb. 5, 2022, 12:48 p.m. UTC | #3
On Sat, Feb 05, 2022 at 12:15:15PM +0000, Paul Cercueil wrote:
> Hi Greg,
> 
> Le sam., févr. 5 2022 at 10:59:50 +0100, Greg KH
> <gregkh@linuxfoundation.org> a écrit :
> > On Sat, Feb 05, 2022 at 12:45:31PM +0300, Siarhei Volkau wrote:
> > >  The mmc0 clock gate bit was mistakenly assigned to "i2s" clock.
> > >  You can find that the same bit is assigned to "mmc0" too.
> > >  It leads to mmc0 hang for a long time after any sound activity
> > >  also it  prevented PM_SLEEP to work properly.
> > >  I guess it was introduced by copy-paste from jz4740 driver
> > >  where it is really controls I2S clock gate.
> > > 
> > >  Fixes: 226dfa4726eb ("clk: Add Ingenic jz4725b CGU driver")
> > >  Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> > >  Tested-by: Siarhei Volkau <lis8215@gmail.com>
> > >  Reviewed-by: Paul Cercueil <paul@crapouillou.net>
> > >  ---
> > >   drivers/clk/ingenic/jz4725b-cgu.c | 3 +--
> > >   1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > >  diff --git a/drivers/clk/ingenic/jz4725b-cgu.c
> > > b/drivers/clk/ingenic/jz4725b-cgu.c
> > >  index 744d136..15d6179 100644
> > >  --- a/drivers/clk/ingenic/jz4725b-cgu.c
> > >  +++ b/drivers/clk/ingenic/jz4725b-cgu.c
> > >  @@ -139,11 +139,10 @@ static const struct ingenic_cgu_clk_info
> > > jz4725b_cgu_clocks[] = {
> > >   	},
> > > 
> > >   	[JZ4725B_CLK_I2S] = {
> > >  -		"i2s", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
> > >  +		"i2s", CGU_CLK_MUX | CGU_CLK_DIV,
> > >   		.parents = { JZ4725B_CLK_EXT, JZ4725B_CLK_PLL_HALF, -1, -1 },
> > >   		.mux = { CGU_REG_CPCCR, 31, 1 },
> > >   		.div = { CGU_REG_I2SCDR, 0, 1, 9, -1, -1, -1 },
> > >  -		.gate = { CGU_REG_CLKGR, 6 },
> > >   	},
> > > 
> > >   	[JZ4725B_CLK_SPI] = {
> > >  --
> > >  2.35.1
> > > 
> > 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> What's wrong with this patch exactly? It looks good to me.

No "Cc: stable@..." in the signed-off-by area.
diff mbox series

Patch

diff --git a/drivers/clk/ingenic/jz4725b-cgu.c b/drivers/clk/ingenic/jz4725b-cgu.c
index 744d136..15d6179 100644
--- a/drivers/clk/ingenic/jz4725b-cgu.c
+++ b/drivers/clk/ingenic/jz4725b-cgu.c
@@ -139,11 +139,10 @@  static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
 	},
 
 	[JZ4725B_CLK_I2S] = {
-		"i2s", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
+		"i2s", CGU_CLK_MUX | CGU_CLK_DIV,
 		.parents = { JZ4725B_CLK_EXT, JZ4725B_CLK_PLL_HALF, -1, -1 },
 		.mux = { CGU_REG_CPCCR, 31, 1 },
 		.div = { CGU_REG_I2SCDR, 0, 1, 9, -1, -1, -1 },
-		.gate = { CGU_REG_CLKGR, 6 },
 	},
 
 	[JZ4725B_CLK_SPI] = {