This series is meant to improve the mtk-pmic-wrap driver;
that's done by removing the custom pwrap_wait_for_state() function
and correctly using the readx_poll_timeout macro instead, which is
doing exactly the same as the former.
As also shown in a patch [1] by Zhiyong Tao (MediaTek), performing
a tight loop is not desired: because of the operation timing in the
SPMI PMICs on these platforms, it makes more sense to wait for some
microseconds before trying to read again, reducing CPU busy time
around these state waits. For this purpose, a ~10uS delay was chosen.
While at it, I also took the occasion to tidy up this driver a
little by optimizing its probe() function.
[1]: https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
Changes in v2:
- Fixed a critical typo in patch 1/3. Thanks Nicolas!
AngeloGioacchino Del Regno (3):
soc: mediatek: pwrap: Use readx_poll_timeout() instead of custom
function
soc: mediatek: pwrap: Switch to
devm_platform_ioremap_resource_byname()
soc: mediatek: pwrap: Move and check return value of
platform_get_irq()
drivers/soc/mediatek/mtk-pmic-wrap.c | 73 +++++++++++++++-------------
1 file changed, 39 insertions(+), 34 deletions(-)
--
2.35.1
In order to simplify ioremapping resources, instead of calling
platform_get_resource_byname() and then devm_ioremap_resource(),
simply call devm_platform_ioremap_resource_byname().
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 54a5300ab72b..852514366f1f 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -2191,7 +2191,6 @@ static int pwrap_probe(struct platform_device *pdev)
struct pmic_wrapper *wrp;
struct device_node *np = pdev->dev.of_node;
const struct of_device_id *of_slave_id = NULL;
- struct resource *res;
if (np->child)
of_slave_id = of_match_node(of_slave_match_tbl, np->child);
@@ -2211,8 +2210,7 @@ static int pwrap_probe(struct platform_device *pdev)
wrp->slave = of_slave_id->data;
wrp->dev = &pdev->dev;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwrap");
- wrp->base = devm_ioremap_resource(wrp->dev, res);
+ wrp->base = devm_platform_ioremap_resource_byname(pdev, "pwrap");
if (IS_ERR(wrp->base))
return PTR_ERR(wrp->base);
@@ -2226,9 +2224,7 @@ static int pwrap_probe(struct platform_device *pdev)
}
if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
- "pwrap-bridge");
- wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);
+ wrp->bridge_base = devm_platform_ioremap_resource_byname(pdev, "pwrap-bridge");
if (IS_ERR(wrp->bridge_base))
return PTR_ERR(wrp->bridge_base);
--
2.35.1
Move the call to platform_get_irq() earlier in the probe function
and check for its return value: if no interrupt is specified, it
wouldn't make sense to try to call devm_request_irq() so, in that
case, we can simply return early.
Moving the platform_get_irq() call also makes it possible to use
one less goto, as clocks aren't required at that stage.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/soc/mediatek/mtk-pmic-wrap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 852514366f1f..332cbcabc299 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -2204,6 +2204,10 @@ static int pwrap_probe(struct platform_device *pdev)
if (!wrp)
return -ENOMEM;
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
platform_set_drvdata(pdev, wrp);
wrp->master = of_device_get_match_data(&pdev->dev);
@@ -2316,7 +2320,6 @@ static int pwrap_probe(struct platform_device *pdev)
if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
- irq = platform_get_irq(pdev, 0);
ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
IRQF_TRIGGER_HIGH,
"mt-pmic-pwrap", wrp);
--
2.35.1
On Thu, Mar 31, 2022 at 09:58:16AM +0200, AngeloGioacchino Del Regno wrote:
> In order to simplify ioremapping resources, instead of calling
> platform_get_resource_byname() and then devm_ioremap_resource(),
> simply call devm_platform_ioremap_resource_byname().
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: N?colas F. R. A. Prado <[email protected]>
Tested-by: N?colas F. R. A. Prado <[email protected]>
> ---
> drivers/soc/mediatek/mtk-pmic-wrap.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 54a5300ab72b..852514366f1f 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -2191,7 +2191,6 @@ static int pwrap_probe(struct platform_device *pdev)
> struct pmic_wrapper *wrp;
> struct device_node *np = pdev->dev.of_node;
> const struct of_device_id *of_slave_id = NULL;
> - struct resource *res;
>
> if (np->child)
> of_slave_id = of_match_node(of_slave_match_tbl, np->child);
> @@ -2211,8 +2210,7 @@ static int pwrap_probe(struct platform_device *pdev)
> wrp->slave = of_slave_id->data;
> wrp->dev = &pdev->dev;
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwrap");
> - wrp->base = devm_ioremap_resource(wrp->dev, res);
> + wrp->base = devm_platform_ioremap_resource_byname(pdev, "pwrap");
> if (IS_ERR(wrp->base))
> return PTR_ERR(wrp->base);
>
> @@ -2226,9 +2224,7 @@ static int pwrap_probe(struct platform_device *pdev)
> }
>
> if (HAS_CAP(wrp->master->caps, PWRAP_CAP_BRIDGE)) {
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - "pwrap-bridge");
> - wrp->bridge_base = devm_ioremap_resource(wrp->dev, res);
> + wrp->bridge_base = devm_platform_ioremap_resource_byname(pdev, "pwrap-bridge");
> if (IS_ERR(wrp->bridge_base))
> return PTR_ERR(wrp->bridge_base);
>
> --
> 2.35.1
>
On Thu, Mar 31, 2022 at 09:58:17AM +0200, AngeloGioacchino Del Regno wrote:
> Move the call to platform_get_irq() earlier in the probe function
> and check for its return value: if no interrupt is specified, it
> wouldn't make sense to try to call devm_request_irq() so, in that
> case, we can simply return early.
>
> Moving the platform_get_irq() call also makes it possible to use
> one less goto, as clocks aren't required at that stage.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: N?colas F. R. A. Prado <[email protected]>
Tested-by: N?colas F. R. A. Prado <[email protected]>
> ---
> drivers/soc/mediatek/mtk-pmic-wrap.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 852514366f1f..332cbcabc299 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -2204,6 +2204,10 @@ static int pwrap_probe(struct platform_device *pdev)
> if (!wrp)
> return -ENOMEM;
>
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> platform_set_drvdata(pdev, wrp);
>
> wrp->master = of_device_get_match_data(&pdev->dev);
> @@ -2316,7 +2320,6 @@ static int pwrap_probe(struct platform_device *pdev)
> if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN))
> pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>
> - irq = platform_get_irq(pdev, 0);
> ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
> IRQF_TRIGGER_HIGH,
> "mt-pmic-pwrap", wrp);
> --
> 2.35.1
>