diff mbox series

[v3,1/1] pwrap: mediatek: fix FSM timeout issue

Message ID 20220424025738.32271-2-zhiyong.tao@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Mediatek PMIC patch | expand

Commit Message

zhiyong.tao April 24, 2022, 2:57 a.m. UTC
From: "Zhiyong.Tao" <zhiyong.tao@mediatek.com>

Fix pwrap FSM timeout issue which leads the system crash on GFX VSRAM
power on.
Add a usleep delay to avoid busy read for the H/W status.
For avoiding the system behavior(ex. disable interrupt in suspend/resume
flow, schedule block task)cause if (time_after()) be turn first,
we change it after sleep delay.
Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and
MT8173 SoCs")

Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Rex-BC Chen (陳柏辰) April 25, 2022, 2:05 a.m. UTC | #1
On Sun, 2022-04-24 at 10:57 +0800, Zhiyong Tao wrote:
> From: "Zhiyong.Tao" <zhiyong.tao@mediatek.com>
> 
> Fix pwrap FSM timeout issue which leads the system crash on GFX VSRAM
> power on.
> Add a usleep delay to avoid busy read for the H/W status.
> For avoiding the system behavior(ex. disable interrupt in
> suspend/resume
> flow, schedule block task)cause if (time_after()) be turn first,
> we change it after sleep delay.
> Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and
> MT8173 SoCs")
> 
> Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
> ---

Hello Zhiyong,

IMO, commit messages should be

Fix pwrap FSM timeout issue....
...we change it after sleep delay.

(=> one blank)
Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and
MT8173 SoCs")
(=> oneline)

Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>

>  drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c
> b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 952bc554f443..ac7139a67e87 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -12,6 +12,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
> +#include <linux/delay.h>

Sorry, I do not notice this in previous version. It will proper order
the header in alphabet.

 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>

BRs,
Rex
>  
>  #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN		0x4
>  #define PWRAP_MT8135_BRIDGE_WACS3_EN		0x10
> @@ -1197,10 +1198,13 @@ static int pwrap_wait_for_state(struct
> pmic_wrapper *wrp,
>  	timeout = jiffies + usecs_to_jiffies(10000);
>  
>  	do {
> -		if (time_after(jiffies, timeout))
> -			return fp(wrp) ? 0 : -ETIMEDOUT;
>  		if (fp(wrp))
>  			return 0;
> +
> +		usleep_range(10, 11);
> +
> +		if (time_after(jiffies, timeout))
> +			return fp(wrp) ? 0 : -ETIMEDOUT;
>  	} while (1);
>  }
>
Matthias Brugger April 25, 2022, 10:50 a.m. UTC | #2
On 25/04/2022 04:05, Rex-BC Chen wrote:
> On Sun, 2022-04-24 at 10:57 +0800, Zhiyong Tao wrote:
>> From: "Zhiyong.Tao" <zhiyong.tao@mediatek.com>
>>
>> Fix pwrap FSM timeout issue which leads the system crash on GFX VSRAM
>> power on.
>> Add a usleep delay to avoid busy read for the H/W status.
>> For avoiding the system behavior(ex. disable interrupt in
>> suspend/resume
>> flow, schedule block task)cause if (time_after()) be turn first,
>> we change it after sleep delay.

I'm not sure what you mean with this sentence. But it sound's like we want to 
paper about some issue. Can you please explain better?

Thanks,
Matthias

>> Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and
>> MT8173 SoCs")
>>
>> Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
>> ---
> 
> Hello Zhiyong,
> 
> IMO, commit messages should be
> 
> Fix pwrap FSM timeout issue....
> ...we change it after sleep delay.
> 
> (=> one blank)
> Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and
> MT8173 SoCs")
> (=> oneline)
> 
> Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
> 
>>   drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c
>> b/drivers/soc/mediatek/mtk-pmic-wrap.c
>> index 952bc554f443..ac7139a67e87 100644
>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/platform_device.h>
>>   #include <linux/regmap.h>
>>   #include <linux/reset.h>
>> +#include <linux/delay.h>
> 
> Sorry, I do not notice this in previous version. It will proper order
> the header in alphabet.
> 
>   #include <linux/delay.h>
>   #include <linux/platform_device.h>
>   #include <linux/regmap.h>
>   #include <linux/reset.h>
> 
> BRs,
> Rex
>>   
>>   #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN		0x4
>>   #define PWRAP_MT8135_BRIDGE_WACS3_EN		0x10
>> @@ -1197,10 +1198,13 @@ static int pwrap_wait_for_state(struct
>> pmic_wrapper *wrp,
>>   	timeout = jiffies + usecs_to_jiffies(10000);
>>   
>>   	do {
>> -		if (time_after(jiffies, timeout))
>> -			return fp(wrp) ? 0 : -ETIMEDOUT;
>>   		if (fp(wrp))
>>   			return 0;
>> +
>> +		usleep_range(10, 11);
>> +
>> +		if (time_after(jiffies, timeout))
>> +			return fp(wrp) ? 0 : -ETIMEDOUT;
>>   	} while (1);
>>   }
>>   
>
zhiyong.tao April 27, 2022, 1:51 a.m. UTC | #3
On Mon, 2022-04-25 at 10:05 +0800, Rex-BC Chen wrote:
> On Sun, 2022-04-24 at 10:57 +0800, Zhiyong Tao wrote:
> > From: "Zhiyong.Tao" <zhiyong.tao@mediatek.com>
> > 
> > Fix pwrap FSM timeout issue which leads the system crash on GFX
> > VSRAM
> > power on.
> > Add a usleep delay to avoid busy read for the H/W status.
> > For avoiding the system behavior(ex. disable interrupt in
> > suspend/resume
> > flow, schedule block task)cause if (time_after()) be turn first,
> > we change it after sleep delay.
> > Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135
> > and
> > MT8173 SoCs")
> > 
> > Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
> > ---
> 
> Hello Zhiyong,
> 
> IMO, commit messages should be
> 
> Fix pwrap FSM timeout issue....
> ...we change it after sleep delay.
> 
> (=> one blank)
> Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and
> MT8173 SoCs")
> (=> oneline)
> 
we will change it in v4
> Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
> 
> >  drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c
> > b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > index 952bc554f443..ac7139a67e87 100644
> > --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> > +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> >  #include <linux/reset.h>
> > +#include <linux/delay.h>
> 
> Sorry, I do not notice this in previous version. It will proper order
> the header in alphabet.
we will change it in v4
> 
>  #include <linux/delay.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
> 
> BRs,
> Rex
> >  
> >  #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN		0x4
> >  #define PWRAP_MT8135_BRIDGE_WACS3_EN		0x10
> > @@ -1197,10 +1198,13 @@ static int pwrap_wait_for_state(struct
> > pmic_wrapper *wrp,
> >  	timeout = jiffies + usecs_to_jiffies(10000);
> >  
> >  	do {
> > -		if (time_after(jiffies, timeout))
> > -			return fp(wrp) ? 0 : -ETIMEDOUT;
> >  		if (fp(wrp))
> >  			return 0;
> > +
> > +		usleep_range(10, 11);
> > +
> > +		if (time_after(jiffies, timeout))
> > +			return fp(wrp) ? 0 : -ETIMEDOUT;
> >  	} while (1);
> >  }
> >  
> 
>
diff mbox series

Patch

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 952bc554f443..ac7139a67e87 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -12,6 +12,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
+#include <linux/delay.h>
 
 #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN		0x4
 #define PWRAP_MT8135_BRIDGE_WACS3_EN		0x10
@@ -1197,10 +1198,13 @@  static int pwrap_wait_for_state(struct pmic_wrapper *wrp,
 	timeout = jiffies + usecs_to_jiffies(10000);
 
 	do {
-		if (time_after(jiffies, timeout))
-			return fp(wrp) ? 0 : -ETIMEDOUT;
 		if (fp(wrp))
 			return 0;
+
+		usleep_range(10, 11);
+
+		if (time_after(jiffies, timeout))
+			return fp(wrp) ? 0 : -ETIMEDOUT;
 	} while (1);
 }