linux-kernel - Re: [PATCH v1 1/1] pwrap: mediatek: fix wait time out issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 29 Mar 2022 16:44:00 +0200
From:   Matthias Brugger <matthias.bgg@...il.com>
To:     Zhiyong Tao <zhiyong.tao@...iatek.com>, lee.jones@...aro.org,
        robh+dt@...nel.org, lgirdwood@...il.com, broonie@...nel.org,
        eddie.huang@...iatek.com, a.zummo@...ertech.it,
        alexandre.belloni@...tlin.com, fshao@...omium.org
Cc:     srv_heupstream@...iatek.com, hui.liu@...iatek.com,
        hsin-hsiung.wang@...iatek.com, sean.wang@...iatek.com,
        macpaul.lin@...iatek.com, wen.su@...iatek.com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-rtc@...r.kernel.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v1 1/1] pwrap: mediatek: fix wait time out issue



On 29/03/2022 13:58, Zhiyong Tao wrote:
> From: "Zhiyong.Tao" <zhiyong.tao@...iatek.com>
> 
> add sleep delay before read "jiffies"
> 
> Signed-off-by: Zhiyong.Tao <zhiyong.tao@...iatek.com>
> ---
>   drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>   mode change 100644 => 100755 drivers/soc/mediatek/mtk-pmic-wrap.c
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> old mode 100644
> new mode 100755
> index 952bc554f443..ac7139a67e87
> --- 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);
> +

You need to explain this change. Why is it needed, are you sure it does not 
break other platforms etc.

I can guess why you need that patch here. In any case in the code you provide, 
it would make sense to move the first if out of the do loop and only after this 
wait 10 us before reading again. Right now the code does usleep only after every 
other read.

Regards,
Matthias

> +		if (time_after(jiffies, timeout))
> +			return fp(wrp) ? 0 : -ETIMEDOUT;
>   	} while (1);
>   }
>