From: Matthias Brugger <matthias.bgg@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org,
linux-kernel@vger.kernel.org, nfraprado@collabora.com,
rex-bc.chen@mediatek.com, zhiyong.tao@mediatek.com
Subject: Re: [PATCH v3 1/5] soc: mediatek: pwrap: Use readx_poll_timeout() instead of custom function
Date: Tue, 17 May 2022 11:25:57 +0200 [thread overview]
Message-ID: <f8acbc75-970c-62fb-ad0d-914e512104a7@gmail.com> (raw)
In-Reply-To: <20220516124659.69484-2-angelogioacchino.delregno@collabora.com>
On 16/05/2022 14:46, AngeloGioacchino Del Regno wrote:
> Function pwrap_wait_for_state() is a function that polls an address
> through a helper function, but this is the very same operation that
> the readx_poll_timeout macro means to do.
> Convert all instances of calling pwrap_wait_for_state() to instead
> use the read_poll_timeout macro.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> drivers/soc/mediatek/mtk-pmic-wrap.c | 60 +++++++++++++++-------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index bf39a64f3ecc..54a5300ab72b 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -13,6 +13,9 @@
> #include <linux/regmap.h>
> #include <linux/reset.h>
>
> +#define PWRAP_POLL_DELAY_US 10
> +#define PWRAP_POLL_TIMEOUT_US 10000
> +
> #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN 0x4
> #define PWRAP_MT8135_BRIDGE_WACS3_EN 0x10
> #define PWRAP_MT8135_BRIDGE_INIT_DONE3 0x14
> @@ -1241,27 +1244,14 @@ static bool pwrap_is_fsm_idle_and_sync_idle(struct pmic_wrapper *wrp)
> (val & PWRAP_STATE_SYNC_IDLE0);
> }
>
> -static int pwrap_wait_for_state(struct pmic_wrapper *wrp,
> - bool (*fp)(struct pmic_wrapper *))
> -{
> - unsigned long timeout;
> -
> - timeout = jiffies + usecs_to_jiffies(10000);
> -
> - do {
> - if (time_after(jiffies, timeout))
> - return fp(wrp) ? 0 : -ETIMEDOUT;
> - if (fp(wrp))
> - return 0;
> - } while (1);
> -}
> -
> static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> {
> + bool tmp;
> int ret;
> u32 val;
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
hm, if we make the cond (tmp > 0) that would help to understand the code. At
least I had to think about it for a moment. But I leave it to you if you think
it's worth the effort.
Regards,
Matthias
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1273,7 +1263,8 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> val = (adr >> 1) << 16;
> pwrap_writel(wrp, val, PWRAP_WACS2_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
> + ret = readx_poll_timeout(pwrap_is_fsm_vldclr, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret)
> return ret;
>
> @@ -1290,11 +1281,14 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
>
> static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> {
> + bool tmp;
> int ret, msb;
>
> *rdata = 0;
> for (msb = 0; msb < 2; msb++) {
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> +
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1303,7 +1297,8 @@ static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> pwrap_writel(wrp, ((msb << 30) | (adr << 16)),
> PWRAP_WACS2_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
> + ret = readx_poll_timeout(pwrap_is_fsm_vldclr, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret)
> return ret;
>
> @@ -1323,9 +1318,11 @@ static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
>
> static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
> {
> + bool tmp;
> int ret;
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1344,10 +1341,12 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
>
> static int pwrap_write32(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
> {
> + bool tmp;
> int ret, msb, rdata;
>
> for (msb = 0; msb < 2; msb++) {
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1388,6 +1387,7 @@ static int pwrap_regmap_write(void *context, u32 adr, u32 wdata)
>
> static int pwrap_reset_spislave(struct pmic_wrapper *wrp)
> {
> + bool tmp;
> int ret, i;
>
> pwrap_writel(wrp, 0, PWRAP_HIPRIO_ARB_EN);
> @@ -1407,7 +1407,8 @@ static int pwrap_reset_spislave(struct pmic_wrapper *wrp)
> pwrap_writel(wrp, wrp->master->spi_w | PWRAP_MAN_CMD_OP_OUTS,
> PWRAP_MAN_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
> return ret;
> @@ -1458,14 +1459,15 @@ static int pwrap_init_sidly(struct pmic_wrapper *wrp)
> static int pwrap_init_dual_io(struct pmic_wrapper *wrp)
> {
> int ret;
> + bool tmp;
> u32 rdata;
>
> /* Enable dual IO mode */
> pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_DIO_EN], 1);
>
> /* Check IDLE & INIT_DONE in advance */
> - ret = pwrap_wait_for_state(wrp,
> - pwrap_is_fsm_idle_and_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle_and_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
> return ret;
> @@ -1570,6 +1572,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> + bool tmp;
> u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> @@ -1624,14 +1627,16 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> }
>
> /* wait for cipher data ready@AP */
> - ret = pwrap_wait_for_state(wrp, pwrap_is_cipher_ready);
> + ret = readx_poll_timeout(pwrap_is_cipher_ready, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "cipher data ready@AP fail, ret=%d\n", ret);
> return ret;
> }
>
> /* wait for cipher data ready@PMIC */
> - ret = pwrap_wait_for_state(wrp, pwrap_is_pmic_cipher_ready);
> + ret = readx_poll_timeout(pwrap_is_pmic_cipher_ready, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev,
> "timeout waiting for cipher data ready@PMIC\n");
> @@ -1640,7 +1645,8 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>
> /* wait for cipher mode idle */
> pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_MODE], 0x1);
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle_and_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle_and_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "cipher mode idle fail, ret=%d\n", ret);
> return ret;
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Matthias Brugger <matthias.bgg@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org,
linux-kernel@vger.kernel.org, nfraprado@collabora.com,
rex-bc.chen@mediatek.com, zhiyong.tao@mediatek.com
Subject: Re: [PATCH v3 1/5] soc: mediatek: pwrap: Use readx_poll_timeout() instead of custom function
Date: Tue, 17 May 2022 11:25:57 +0200 [thread overview]
Message-ID: <f8acbc75-970c-62fb-ad0d-914e512104a7@gmail.com> (raw)
In-Reply-To: <20220516124659.69484-2-angelogioacchino.delregno@collabora.com>
On 16/05/2022 14:46, AngeloGioacchino Del Regno wrote:
> Function pwrap_wait_for_state() is a function that polls an address
> through a helper function, but this is the very same operation that
> the readx_poll_timeout macro means to do.
> Convert all instances of calling pwrap_wait_for_state() to instead
> use the read_poll_timeout macro.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> drivers/soc/mediatek/mtk-pmic-wrap.c | 60 +++++++++++++++-------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index bf39a64f3ecc..54a5300ab72b 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -13,6 +13,9 @@
> #include <linux/regmap.h>
> #include <linux/reset.h>
>
> +#define PWRAP_POLL_DELAY_US 10
> +#define PWRAP_POLL_TIMEOUT_US 10000
> +
> #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN 0x4
> #define PWRAP_MT8135_BRIDGE_WACS3_EN 0x10
> #define PWRAP_MT8135_BRIDGE_INIT_DONE3 0x14
> @@ -1241,27 +1244,14 @@ static bool pwrap_is_fsm_idle_and_sync_idle(struct pmic_wrapper *wrp)
> (val & PWRAP_STATE_SYNC_IDLE0);
> }
>
> -static int pwrap_wait_for_state(struct pmic_wrapper *wrp,
> - bool (*fp)(struct pmic_wrapper *))
> -{
> - unsigned long timeout;
> -
> - timeout = jiffies + usecs_to_jiffies(10000);
> -
> - do {
> - if (time_after(jiffies, timeout))
> - return fp(wrp) ? 0 : -ETIMEDOUT;
> - if (fp(wrp))
> - return 0;
> - } while (1);
> -}
> -
> static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> {
> + bool tmp;
> int ret;
> u32 val;
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
hm, if we make the cond (tmp > 0) that would help to understand the code. At
least I had to think about it for a moment. But I leave it to you if you think
it's worth the effort.
Regards,
Matthias
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1273,7 +1263,8 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> val = (adr >> 1) << 16;
> pwrap_writel(wrp, val, PWRAP_WACS2_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
> + ret = readx_poll_timeout(pwrap_is_fsm_vldclr, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret)
> return ret;
>
> @@ -1290,11 +1281,14 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
>
> static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> {
> + bool tmp;
> int ret, msb;
>
> *rdata = 0;
> for (msb = 0; msb < 2; msb++) {
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> +
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1303,7 +1297,8 @@ static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> pwrap_writel(wrp, ((msb << 30) | (adr << 16)),
> PWRAP_WACS2_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
> + ret = readx_poll_timeout(pwrap_is_fsm_vldclr, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret)
> return ret;
>
> @@ -1323,9 +1318,11 @@ static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
>
> static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
> {
> + bool tmp;
> int ret;
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1344,10 +1341,12 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
>
> static int pwrap_write32(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
> {
> + bool tmp;
> int ret, msb, rdata;
>
> for (msb = 0; msb < 2; msb++) {
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1388,6 +1387,7 @@ static int pwrap_regmap_write(void *context, u32 adr, u32 wdata)
>
> static int pwrap_reset_spislave(struct pmic_wrapper *wrp)
> {
> + bool tmp;
> int ret, i;
>
> pwrap_writel(wrp, 0, PWRAP_HIPRIO_ARB_EN);
> @@ -1407,7 +1407,8 @@ static int pwrap_reset_spislave(struct pmic_wrapper *wrp)
> pwrap_writel(wrp, wrp->master->spi_w | PWRAP_MAN_CMD_OP_OUTS,
> PWRAP_MAN_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
> return ret;
> @@ -1458,14 +1459,15 @@ static int pwrap_init_sidly(struct pmic_wrapper *wrp)
> static int pwrap_init_dual_io(struct pmic_wrapper *wrp)
> {
> int ret;
> + bool tmp;
> u32 rdata;
>
> /* Enable dual IO mode */
> pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_DIO_EN], 1);
>
> /* Check IDLE & INIT_DONE in advance */
> - ret = pwrap_wait_for_state(wrp,
> - pwrap_is_fsm_idle_and_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle_and_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
> return ret;
> @@ -1570,6 +1572,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> + bool tmp;
> u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> @@ -1624,14 +1627,16 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> }
>
> /* wait for cipher data ready@AP */
> - ret = pwrap_wait_for_state(wrp, pwrap_is_cipher_ready);
> + ret = readx_poll_timeout(pwrap_is_cipher_ready, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "cipher data ready@AP fail, ret=%d\n", ret);
> return ret;
> }
>
> /* wait for cipher data ready@PMIC */
> - ret = pwrap_wait_for_state(wrp, pwrap_is_pmic_cipher_ready);
> + ret = readx_poll_timeout(pwrap_is_pmic_cipher_ready, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev,
> "timeout waiting for cipher data ready@PMIC\n");
> @@ -1640,7 +1645,8 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>
> /* wait for cipher mode idle */
> pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_MODE], 0x1);
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle_and_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle_and_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "cipher mode idle fail, ret=%d\n", ret);
> return ret;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Matthias Brugger <matthias.bgg@gmail.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
nfraprado@collabora.com, rex-bc.chen@mediatek.com,
zhiyong.tao@mediatek.com
Subject: Re: [PATCH v3 1/5] soc: mediatek: pwrap: Use readx_poll_timeout() instead of custom function
Date: Tue, 17 May 2022 11:25:57 +0200 [thread overview]
Message-ID: <f8acbc75-970c-62fb-ad0d-914e512104a7@gmail.com> (raw)
In-Reply-To: <20220516124659.69484-2-angelogioacchino.delregno@collabora.com>
On 16/05/2022 14:46, AngeloGioacchino Del Regno wrote:
> Function pwrap_wait_for_state() is a function that polls an address
> through a helper function, but this is the very same operation that
> the readx_poll_timeout macro means to do.
> Convert all instances of calling pwrap_wait_for_state() to instead
> use the read_poll_timeout macro.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> drivers/soc/mediatek/mtk-pmic-wrap.c | 60 +++++++++++++++-------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index bf39a64f3ecc..54a5300ab72b 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -13,6 +13,9 @@
> #include <linux/regmap.h>
> #include <linux/reset.h>
>
> +#define PWRAP_POLL_DELAY_US 10
> +#define PWRAP_POLL_TIMEOUT_US 10000
> +
> #define PWRAP_MT8135_BRIDGE_IORD_ARB_EN 0x4
> #define PWRAP_MT8135_BRIDGE_WACS3_EN 0x10
> #define PWRAP_MT8135_BRIDGE_INIT_DONE3 0x14
> @@ -1241,27 +1244,14 @@ static bool pwrap_is_fsm_idle_and_sync_idle(struct pmic_wrapper *wrp)
> (val & PWRAP_STATE_SYNC_IDLE0);
> }
>
> -static int pwrap_wait_for_state(struct pmic_wrapper *wrp,
> - bool (*fp)(struct pmic_wrapper *))
> -{
> - unsigned long timeout;
> -
> - timeout = jiffies + usecs_to_jiffies(10000);
> -
> - do {
> - if (time_after(jiffies, timeout))
> - return fp(wrp) ? 0 : -ETIMEDOUT;
> - if (fp(wrp))
> - return 0;
> - } while (1);
> -}
> -
> static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> {
> + bool tmp;
> int ret;
> u32 val;
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
hm, if we make the cond (tmp > 0) that would help to understand the code. At
least I had to think about it for a moment. But I leave it to you if you think
it's worth the effort.
Regards,
Matthias
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1273,7 +1263,8 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> val = (adr >> 1) << 16;
> pwrap_writel(wrp, val, PWRAP_WACS2_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
> + ret = readx_poll_timeout(pwrap_is_fsm_vldclr, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret)
> return ret;
>
> @@ -1290,11 +1281,14 @@ static int pwrap_read16(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
>
> static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> {
> + bool tmp;
> int ret, msb;
>
> *rdata = 0;
> for (msb = 0; msb < 2; msb++) {
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> +
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1303,7 +1297,8 @@ static int pwrap_read32(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
> pwrap_writel(wrp, ((msb << 30) | (adr << 16)),
> PWRAP_WACS2_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_vldclr);
> + ret = readx_poll_timeout(pwrap_is_fsm_vldclr, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret)
> return ret;
>
> @@ -1323,9 +1318,11 @@ static int pwrap_read(struct pmic_wrapper *wrp, u32 adr, u32 *rdata)
>
> static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
> {
> + bool tmp;
> int ret;
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1344,10 +1341,12 @@ static int pwrap_write16(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
>
> static int pwrap_write32(struct pmic_wrapper *wrp, u32 adr, u32 wdata)
> {
> + bool tmp;
> int ret, msb, rdata;
>
> for (msb = 0; msb < 2; msb++) {
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> pwrap_leave_fsm_vldclr(wrp);
> return ret;
> @@ -1388,6 +1387,7 @@ static int pwrap_regmap_write(void *context, u32 adr, u32 wdata)
>
> static int pwrap_reset_spislave(struct pmic_wrapper *wrp)
> {
> + bool tmp;
> int ret, i;
>
> pwrap_writel(wrp, 0, PWRAP_HIPRIO_ARB_EN);
> @@ -1407,7 +1407,8 @@ static int pwrap_reset_spislave(struct pmic_wrapper *wrp)
> pwrap_writel(wrp, wrp->master->spi_w | PWRAP_MAN_CMD_OP_OUTS,
> PWRAP_MAN_CMD);
>
> - ret = pwrap_wait_for_state(wrp, pwrap_is_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
> return ret;
> @@ -1458,14 +1459,15 @@ static int pwrap_init_sidly(struct pmic_wrapper *wrp)
> static int pwrap_init_dual_io(struct pmic_wrapper *wrp)
> {
> int ret;
> + bool tmp;
> u32 rdata;
>
> /* Enable dual IO mode */
> pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_DIO_EN], 1);
>
> /* Check IDLE & INIT_DONE in advance */
> - ret = pwrap_wait_for_state(wrp,
> - pwrap_is_fsm_idle_and_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle_and_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "%s fail, ret=%d\n", __func__, ret);
> return ret;
> @@ -1570,6 +1572,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> + bool tmp;
> u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> @@ -1624,14 +1627,16 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> }
>
> /* wait for cipher data ready@AP */
> - ret = pwrap_wait_for_state(wrp, pwrap_is_cipher_ready);
> + ret = readx_poll_timeout(pwrap_is_cipher_ready, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "cipher data ready@AP fail, ret=%d\n", ret);
> return ret;
> }
>
> /* wait for cipher data ready@PMIC */
> - ret = pwrap_wait_for_state(wrp, pwrap_is_pmic_cipher_ready);
> + ret = readx_poll_timeout(pwrap_is_pmic_cipher_ready, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev,
> "timeout waiting for cipher data ready@PMIC\n");
> @@ -1640,7 +1645,8 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>
> /* wait for cipher mode idle */
> pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_CIPHER_MODE], 0x1);
> - ret = pwrap_wait_for_state(wrp, pwrap_is_fsm_idle_and_sync_idle);
> + ret = readx_poll_timeout(pwrap_is_fsm_idle_and_sync_idle, wrp, tmp, tmp,
> + PWRAP_POLL_DELAY_US, PWRAP_POLL_TIMEOUT_US);
> if (ret) {
> dev_err(wrp->dev, "cipher mode idle fail, ret=%d\n", ret);
> return ret;
next prev parent reply other threads:[~2022-05-17 9:26 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-16 12:46 [PATCH v3 0/5] MediaTek PMIC Wrap improvements and cleanups AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` [PATCH v3 1/5] soc: mediatek: pwrap: Use readx_poll_timeout() instead of custom function AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-17 9:25 ` Matthias Brugger [this message]
2022-05-17 9:25 ` Matthias Brugger
2022-05-17 9:25 ` Matthias Brugger
2022-05-17 9:41 ` AngeloGioacchino Del Regno
2022-05-17 9:41 ` AngeloGioacchino Del Regno
2022-05-17 9:41 ` AngeloGioacchino Del Regno
2022-05-17 9:44 ` Matthias Brugger
2022-05-17 9:44 ` Matthias Brugger
2022-05-17 9:44 ` Matthias Brugger
2022-05-16 12:46 ` [PATCH v3 2/5] soc: mediatek: pwrap: Switch to devm_platform_ioremap_resource_byname() AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` [PATCH v3 3/5] soc: mediatek: pwrap: Move and check return value of platform_get_irq() AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-17 9:18 ` Matthias Brugger
2022-05-17 9:18 ` Matthias Brugger
2022-05-17 9:18 ` Matthias Brugger
2022-05-17 9:34 ` AngeloGioacchino Del Regno
2022-05-17 9:34 ` AngeloGioacchino Del Regno
2022-05-17 9:34 ` AngeloGioacchino Del Regno
2022-05-17 9:49 ` Matthias Brugger
2022-05-17 9:49 ` Matthias Brugger
2022-05-17 9:49 ` Matthias Brugger
2022-05-17 10:35 ` AngeloGioacchino Del Regno
2022-05-17 10:35 ` AngeloGioacchino Del Regno
2022-05-17 10:35 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` [PATCH v3 4/5] soc: mediatek: pwrap: Move IO pointers to new structure AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` [PATCH v3 5/5] soc: mediatek: pwrap: Compress of_device_id entries to one line AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-16 12:46 ` AngeloGioacchino Del Regno
2022-05-17 9:23 ` Matthias Brugger
2022-05-17 9:23 ` Matthias Brugger
2022-05-17 9:23 ` Matthias Brugger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f8acbc75-970c-62fb-ad0d-914e512104a7@gmail.com \
--to=matthias.bgg@gmail.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=nfraprado@collabora.com \
--cc=rex-bc.chen@mediatek.com \
--cc=zhiyong.tao@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.