[PATCH 0/4] soc: mediatek: pwrap: Constify some data and other improvements

[PATCH 0/4] soc: mediatek: pwrap: Constify some data and other improvements

[Christophe JAILLET]

This series is motivated by patch 1. The 3 other patches are some
additionnal goodies spotted while looking at the code.

Patch 1 constifies struct pmic_wrapper_type to move some data to a
read-only section, in order to increase safety.

Patch 2 does the same for some int arrays. This helps move about 7 ko of
data to a read-only section. Not that bad!

Patch 3 simplifies code related to clk management. It also fixes an
issue if the driver is unloaded.

Patch 4 is just a clean-up of some messages.

Christophe JAILLET (4):
  soc: mediatek: pwrap: Constify struct pmic_wrapper_type
  soc: mediatek: pwrap: Constify some struct int[]
  soc: mediatek: pwrap: Use devm_clk_get_[optional_]enabled()
  soc: mediatek: pwrap: Simplify some error messages

 drivers/soc/mediatek/mtk-pmic-wrap.c | 125 +++++++++------------------
 1 file changed, 43 insertions(+), 82 deletions(-)

-- 
2.45.2

[PATCH 1/4] soc: mediatek: pwrap: Constify struct pmic_wrapper_type

[Christophe JAILLET]

'struct pmic_wrapper_type' is not modified in this driver.

Constifying this structure moves some data to a read-only section, so
increase overall security.

On a x86_64, with allmodconfig, as an example:
Before:
======
   text	   data	    bss	    dec	    hex	filename
  45336	   8724	     16	  54076	   d33c	drivers/soc/mediatek/mtk-pmic-wrap.o

After:
=====
   text	   data	    bss	    dec	    hex	filename
  45528	   8532	     16	  54076	   d33c	drivers/soc/mediatek/mtk-pmic-wrap.o

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index efd9cae212dc..0da0cdec5050 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -2397,7 +2397,7 @@ static const struct pmic_wrapper_type pwrap_mt8183 = {
 	.init_soc_specific = pwrap_mt8183_init_soc_specific,
 };
 
-static struct pmic_wrapper_type pwrap_mt8195 = {
+static const struct pmic_wrapper_type pwrap_mt8195 = {
 	.regs = mt8195_regs,
 	.type = PWRAP_MT8195,
 	.arb_en_all = 0x777f, /* NEED CONFIRM */
@@ -2423,7 +2423,7 @@ static const struct pmic_wrapper_type pwrap_mt8365 = {
 	.init_soc_specific = NULL,
 };
 
-static struct pmic_wrapper_type pwrap_mt8516 = {
+static const struct pmic_wrapper_type pwrap_mt8516 = {
 	.regs = mt8516_regs,
 	.type = PWRAP_MT8516,
 	.arb_en_all = 0xff,
@@ -2435,7 +2435,7 @@ static struct pmic_wrapper_type pwrap_mt8516 = {
 	.init_soc_specific = NULL,
 };
 
-static struct pmic_wrapper_type pwrap_mt8186 = {
+static const struct pmic_wrapper_type pwrap_mt8186 = {
 	.regs = mt8186_regs,
 	.type = PWRAP_MT8186,
 	.arb_en_all = 0xfb27f,
-- 
2.45.2

[PATCH 2/4] soc: mediatek: pwrap: Constify some struct int[]

[Christophe JAILLET]

These arrays are not modified in this driver.

Constifying this structure moves some data to a read-only section, so
increase overall security.

On a x86_64, with allmodconfig, as an example:
Before:
======
   text	   data	    bss	    dec	    hex	filename
  45528	   8532	     16	  54076	   d33c	drivers/soc/mediatek/mtk-pmic-wrap.o

After:
=====
   text	   data	    bss	    dec	    hex	filename
  52664	   1384	     16	  54064	   d330	drivers/soc/mediatek/mtk-pmic-wrap.o

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 30 ++++++++++++++--------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 0da0cdec5050..d57553486383 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -483,7 +483,7 @@ enum pwrap_regs {
 	PWRAP_MSB_FIRST,
 };
 
-static int mt2701_regs[] = {
+static const int mt2701_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -569,7 +569,7 @@ static int mt2701_regs[] = {
 	[PWRAP_ADC_RDATA_ADDR2] =	0x154,
 };
 
-static int mt6765_regs[] = {
+static const int mt6765_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -601,7 +601,7 @@ static int mt6765_regs[] = {
 	[PWRAP_DCM_DBC_PRD] =		0x1E0,
 };
 
-static int mt6779_regs[] = {
+static const int mt6779_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -640,7 +640,7 @@ static int mt6779_regs[] = {
 	[PWRAP_WACS2_VLDCLR] =		0xC28,
 };
 
-static int mt6795_regs[] = {
+static const int mt6795_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -725,7 +725,7 @@ static int mt6795_regs[] = {
 	[PWRAP_EXT_CK] =		0x14c,
 };
 
-static int mt6797_regs[] = {
+static const int mt6797_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -758,7 +758,7 @@ static int mt6797_regs[] = {
 	[PWRAP_DCM_DBC_PRD] =		0x1D4,
 };
 
-static int mt6873_regs[] = {
+static const int mt6873_regs[] = {
 	[PWRAP_INIT_DONE2] =		0x0,
 	[PWRAP_TIMER_EN] =		0x3E0,
 	[PWRAP_INT_EN] =		0x448,
@@ -769,7 +769,7 @@ static int mt6873_regs[] = {
 	[PWRAP_WACS2_RDATA] =		0xCA8,
 };
 
-static int mt7622_regs[] = {
+static const int mt7622_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -881,7 +881,7 @@ static int mt7622_regs[] = {
 	[PWRAP_SPI2_CTRL] =		0x244,
 };
 
-static int mt8135_regs[] = {
+static const int mt8135_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -954,7 +954,7 @@ static int mt8135_regs[] = {
 	[PWRAP_DCM_DBC_PRD] =		0x160,
 };
 
-static int mt8173_regs[] = {
+static const int mt8173_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -1036,7 +1036,7 @@ static int mt8173_regs[] = {
 	[PWRAP_DCM_DBC_PRD] =		0x148,
 };
 
-static int mt8183_regs[] = {
+static const int mt8183_regs[] = {
 	[PWRAP_MUX_SEL] =			0x0,
 	[PWRAP_WRAP_EN] =			0x4,
 	[PWRAP_DIO_EN] =			0x8,
@@ -1087,7 +1087,7 @@ static int mt8183_regs[] = {
 	[PWRAP_WACS2_VLDCLR] =			0xC28,
 };
 
-static int mt8195_regs[] = {
+static const int mt8195_regs[] = {
 	[PWRAP_INIT_DONE2] =		0x0,
 	[PWRAP_STAUPD_CTRL] =		0x4C,
 	[PWRAP_TIMER_EN] =		0x3E4,
@@ -1104,7 +1104,7 @@ static int mt8195_regs[] = {
 	[PWRAP_WACS2_RDATA] =		0x8A8,
 };
 
-static int mt8365_regs[] = {
+static const int mt8365_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -1166,7 +1166,7 @@ static int mt8365_regs[] = {
 	[PWRAP_WDT_SRC_EN_1] =		0xf8,
 };
 
-static int mt8516_regs[] = {
+static const int mt8516_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -1251,7 +1251,7 @@ static int mt8516_regs[] = {
 	[PWRAP_MSB_FIRST] =		0x170,
 };
 
-static int mt8186_regs[] = {
+static const int mt8186_regs[] = {
 	[PWRAP_MUX_SEL] =		0x0,
 	[PWRAP_WRAP_EN] =		0x4,
 	[PWRAP_DIO_EN] =		0x8,
@@ -1377,7 +1377,7 @@ struct pmic_wrapper {
 };
 
 struct pmic_wrapper_type {
-	int *regs;
+	const int *regs;
 	enum pwrap_type type;
 	u32 arb_en_all;
 	u32 int_en_all;
-- 
2.45.2

[PATCH 3/4] soc: mediatek: pwrap: Use devm_clk_get_[optional_]enabled()

[Christophe JAILLET]

Use devm_clk_get_enabled() and devm_clk_get_optional_enabled() to simplify
the code and to make sure that clk_disable_unprepare() is called if the
driver is unloaded.

Fixes: 55924157da8c ("soc: mediatek: pwrap: add support for sys & tmr clocks")
Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and MT8173 SoCs")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 85 ++++++++--------------------
 1 file changed, 25 insertions(+), 60 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index d57553486383..6981d6a1ab93 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1366,10 +1366,6 @@ struct pmic_wrapper {
 	struct regmap *regmap;
 	const struct pmic_wrapper_type *master;
 	const struct pwrap_slv_type *slave;
-	struct clk *clk_spi;
-	struct clk *clk_wrap;
-	struct clk *clk_sys;
-	struct clk *clk_tmr;
 	struct reset_control *rstc;
 
 	struct reset_control *rstc_bridge;
@@ -2471,6 +2467,7 @@ static int pwrap_probe(struct platform_device *pdev)
 {
 	int ret, irq;
 	u32 mask_done;
+	struct clk *clk;
 	struct pmic_wrapper *wrp;
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_slave_id = NULL;
@@ -2521,50 +2518,34 @@ static int pwrap_probe(struct platform_device *pdev)
 		}
 	}
 
-	wrp->clk_spi = devm_clk_get(wrp->dev, "spi");
-	if (IS_ERR(wrp->clk_spi)) {
+	clk = devm_clk_get_enabled(wrp->dev, "spi");
+	if (IS_ERR(clk)) {
 		dev_dbg(wrp->dev, "failed to get clock: %ld\n",
-			PTR_ERR(wrp->clk_spi));
-		return PTR_ERR(wrp->clk_spi);
+			PTR_ERR(clk));
+		return PTR_ERR(clk);
 	}
 
-	wrp->clk_wrap = devm_clk_get(wrp->dev, "wrap");
-	if (IS_ERR(wrp->clk_wrap)) {
+	clk = devm_clk_get_enabled(wrp->dev, "wrap");
+	if (IS_ERR(clk)) {
 		dev_dbg(wrp->dev, "failed to get clock: %ld\n",
-			PTR_ERR(wrp->clk_wrap));
-		return PTR_ERR(wrp->clk_wrap);
+			PTR_ERR(clk));
+		return PTR_ERR(clk);
 	}
 
-	wrp->clk_sys = devm_clk_get_optional(wrp->dev, "sys");
-	if (IS_ERR(wrp->clk_sys)) {
-		return dev_err_probe(wrp->dev, PTR_ERR(wrp->clk_sys),
+	clk = devm_clk_get_optional_enabled(wrp->dev, "sys");
+	if (IS_ERR(clk)) {
+		return dev_err_probe(wrp->dev, PTR_ERR(clk),
 				     "failed to get clock: %pe\n",
-				     wrp->clk_sys);
+				     clk);
 	}
 
-	wrp->clk_tmr = devm_clk_get_optional(wrp->dev, "tmr");
-	if (IS_ERR(wrp->clk_tmr)) {
-		return dev_err_probe(wrp->dev, PTR_ERR(wrp->clk_tmr),
+	clk = devm_clk_get_optional_enabled(wrp->dev, "tmr");
+	if (IS_ERR(clk)) {
+		return dev_err_probe(wrp->dev, PTR_ERR(clk),
 				     "failed to get clock: %pe\n",
-				     wrp->clk_tmr);
+				     clk);
 	}
 
-	ret = clk_prepare_enable(wrp->clk_spi);
-	if (ret)
-		return ret;
-
-	ret = clk_prepare_enable(wrp->clk_wrap);
-	if (ret)
-		goto err_out1;
-
-	ret = clk_prepare_enable(wrp->clk_sys);
-	if (ret)
-		goto err_out2;
-
-	ret = clk_prepare_enable(wrp->clk_tmr);
-	if (ret)
-		goto err_out3;
-
 	/* Enable internal dynamic clock */
 	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
 		pwrap_writel(wrp, 1, PWRAP_DCM_EN);
@@ -2579,7 +2560,7 @@ static int pwrap_probe(struct platform_device *pdev)
 		ret = pwrap_init(wrp);
 		if (ret) {
 			dev_dbg(wrp->dev, "init failed with %d\n", ret);
-			goto err_out4;
+			return ret;
 		}
 	}
 
@@ -2592,8 +2573,7 @@ static int pwrap_probe(struct platform_device *pdev)
 
 	if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) {
 		dev_dbg(wrp->dev, "initialization isn't finished\n");
-		ret = -ENODEV;
-		goto err_out4;
+		return -ENODEV;
 	}
 
 	/* Initialize watchdog, may not be done by the bootloader */
@@ -2622,42 +2602,27 @@ static int pwrap_probe(struct platform_device *pdev)
 		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		ret = irq;
-		goto err_out2;
-	}
+	if (irq < 0)
+		return irq;
 
 	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
 			       IRQF_TRIGGER_HIGH,
 			       "mt-pmic-pwrap", wrp);
 	if (ret)
-		goto err_out4;
+		return ret;
 
 	wrp->regmap = devm_regmap_init(wrp->dev, NULL, wrp, wrp->slave->regops->regmap);
-	if (IS_ERR(wrp->regmap)) {
-		ret = PTR_ERR(wrp->regmap);
-		goto err_out2;
-	}
+	if (IS_ERR(wrp->regmap))
+		return PTR_ERR(wrp->regmap);
 
 	ret = of_platform_populate(np, NULL, NULL, wrp->dev);
 	if (ret) {
 		dev_dbg(wrp->dev, "failed to create child devices at %pOF\n",
 				np);
-		goto err_out4;
+		return ret;
 	}
 
 	return 0;
-
-err_out4:
-	clk_disable_unprepare(wrp->clk_tmr);
-err_out3:
-	clk_disable_unprepare(wrp->clk_sys);
-err_out2:
-	clk_disable_unprepare(wrp->clk_wrap);
-err_out1:
-	clk_disable_unprepare(wrp->clk_spi);
-
-	return ret;
 }
 
 static struct platform_driver pwrap_drv = {
-- 
2.45.2

[PATCH 4/4] soc: mediatek: pwrap: Simplify some error messages

[Christophe JAILLET]

dev_err_probe() already display the error code in a human readable form,
there is no need to add it explicitly to the message.

While at it, remove some useless {}.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 6981d6a1ab93..c55f4061b8ef 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -2533,18 +2533,14 @@ static int pwrap_probe(struct platform_device *pdev)
 	}
 
 	clk = devm_clk_get_optional_enabled(wrp->dev, "sys");
-	if (IS_ERR(clk)) {
+	if (IS_ERR(clk))
 		return dev_err_probe(wrp->dev, PTR_ERR(clk),
-				     "failed to get clock: %pe\n",
-				     clk);
-	}
+				     "failed to get sys clock\n");
 
 	clk = devm_clk_get_optional_enabled(wrp->dev, "tmr");
-	if (IS_ERR(clk)) {
+	if (IS_ERR(clk))
 		return dev_err_probe(wrp->dev, PTR_ERR(clk),
-				     "failed to get clock: %pe\n",
-				     clk);
-	}
+				     "failed to get tmr clock\n");
 
 	/* Enable internal dynamic clock */
 	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
-- 
2.45.2

Re: [PATCH 2/4] soc: mediatek: pwrap: Constify some struct int[]

[AngeloGioacchino Del Regno]

Il 29/06/24 11:19, Christophe JAILLET ha scritto:
> These arrays are not modified in this driver.
> 
> Constifying this structure moves some data to a read-only section, so
> increase overall security.
> 
> On a x86_64, with allmodconfig, as an example:
> Before:
> ======
>     text	   data	    bss	    dec	    hex	filename
>    45528	   8532	     16	  54076	   d33c	drivers/soc/mediatek/mtk-pmic-wrap.o
> 
> After:
> =====
>     text	   data	    bss	    dec	    hex	filename
>    52664	   1384	     16	  54064	   d330	drivers/soc/mediatek/mtk-pmic-wrap.o
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 1/4] soc: mediatek: pwrap: Constify struct pmic_wrapper_type

[AngeloGioacchino Del Regno]

Il 29/06/24 11:19, Christophe JAILLET ha scritto:
> 'struct pmic_wrapper_type' is not modified in this driver.
> 
> Constifying this structure moves some data to a read-only section, so
> increase overall security.
> 
> On a x86_64, with allmodconfig, as an example:
> Before:
> ======
>     text	   data	    bss	    dec	    hex	filename
>    45336	   8724	     16	  54076	   d33c	drivers/soc/mediatek/mtk-pmic-wrap.o
> 
> After:
> =====
>     text	   data	    bss	    dec	    hex	filename
>    45528	   8532	     16	  54076	   d33c	drivers/soc/mediatek/mtk-pmic-wrap.o
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
> Compile tested-only
> ---
>   drivers/soc/mediatek/mtk-pmic-wrap.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index efd9cae212dc..0da0cdec5050 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -2397,7 +2397,7 @@ static const struct pmic_wrapper_type pwrap_mt8183 = {
>   	.init_soc_specific = pwrap_mt8183_init_soc_specific,
>   };
>   
> -static struct pmic_wrapper_type pwrap_mt8195 = {
> +static const struct pmic_wrapper_type pwrap_mt8195 = {
>   	.regs = mt8195_regs,
>   	.type = PWRAP_MT8195,
>   	.arb_en_all = 0x777f, /* NEED CONFIRM */
> @@ -2423,7 +2423,7 @@ static const struct pmic_wrapper_type pwrap_mt8365 = {
>   	.init_soc_specific = NULL,
>   };
>   
> -static struct pmic_wrapper_type pwrap_mt8516 = {
> +static const struct pmic_wrapper_type pwrap_mt8516 = {
>   	.regs = mt8516_regs,
>   	.type = PWRAP_MT8516,
>   	.arb_en_all = 0xff,
> @@ -2435,7 +2435,7 @@ static struct pmic_wrapper_type pwrap_mt8516 = {
>   	.init_soc_specific = NULL,
>   };
>   
> -static struct pmic_wrapper_type pwrap_mt8186 = {
> +static const struct pmic_wrapper_type pwrap_mt8186 = {
>   	.regs = mt8186_regs,
>   	.type = PWRAP_MT8186,
>   	.arb_en_all = 0xfb27f,

Re: [PATCH 3/4] soc: mediatek: pwrap: Use devm_clk_get_[optional_]enabled()

[AngeloGioacchino Del Regno]

Il 29/06/24 11:19, Christophe JAILLET ha scritto:
> Use devm_clk_get_enabled() and devm_clk_get_optional_enabled() to simplify
> the code and to make sure that clk_disable_unprepare() is called if the
> driver is unloaded.
> 
> Fixes: 55924157da8c ("soc: mediatek: pwrap: add support for sys & tmr clocks")
> Fixes: 1f022d84bd19 ("soc: mediatek: Add PMIC wrapper for MT8135 and MT8173 SoCs")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested-only
> ---
>   drivers/soc/mediatek/mtk-pmic-wrap.c | 85 ++++++++--------------------
>   1 file changed, 25 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index d57553486383..6981d6a1ab93 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -1366,10 +1366,6 @@ struct pmic_wrapper {
>   	struct regmap *regmap;
>   	const struct pmic_wrapper_type *master;
>   	const struct pwrap_slv_type *slave;
> -	struct clk *clk_spi;
> -	struct clk *clk_wrap;
> -	struct clk *clk_sys;
> -	struct clk *clk_tmr;
>   	struct reset_control *rstc;
>   
>   	struct reset_control *rstc_bridge;
> @@ -2471,6 +2467,7 @@ static int pwrap_probe(struct platform_device *pdev)
>   {
>   	int ret, irq;
>   	u32 mask_done;
> +	struct clk *clk;
>   	struct pmic_wrapper *wrp;
>   	struct device_node *np = pdev->dev.of_node;
>   	const struct of_device_id *of_slave_id = NULL;
> @@ -2521,50 +2518,34 @@ static int pwrap_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> -	wrp->clk_spi = devm_clk_get(wrp->dev, "spi");
> -	if (IS_ERR(wrp->clk_spi)) {
> +	clk = devm_clk_get_enabled(wrp->dev, "spi");

Uhm... in this case, it might be worth using devm_clk_bulk_get_all_enable()
instead... as anyway we're never turning off those clocks during operation.

I checked the devicetrees and the clocks are ordered the right way, but then
we can also consider the fact that the bindings are enforcing the clock order
so that's.. golden.

Practically, by using devm_clk_bulk_get_all_enable(), you're removing even
more lines from this driver, as those four clocks (and four times error checking)
will be reduced to just one call.....!

Thanks for cleaning up this driver, btw!

Cheers,
Angelo


> +	if (IS_ERR(clk)) {
>   		dev_dbg(wrp->dev, "failed to get clock: %ld\n",
> -			PTR_ERR(wrp->clk_spi));
> -		return PTR_ERR(wrp->clk_spi);
> +			PTR_ERR(clk));
> +		return PTR_ERR(clk);
>   	}
>   
> -	wrp->clk_wrap = devm_clk_get(wrp->dev, "wrap");
> -	if (IS_ERR(wrp->clk_wrap)) {
> +	clk = devm_clk_get_enabled(wrp->dev, "wrap");
> +	if (IS_ERR(clk)) {
>   		dev_dbg(wrp->dev, "failed to get clock: %ld\n",
> -			PTR_ERR(wrp->clk_wrap));
> -		return PTR_ERR(wrp->clk_wrap);
> +			PTR_ERR(clk));
> +		return PTR_ERR(clk);
>   	}
>   
> -	wrp->clk_sys = devm_clk_get_optional(wrp->dev, "sys");
> -	if (IS_ERR(wrp->clk_sys)) {
> -		return dev_err_probe(wrp->dev, PTR_ERR(wrp->clk_sys),
> +	clk = devm_clk_get_optional_enabled(wrp->dev, "sys");
> +	if (IS_ERR(clk)) {
> +		return dev_err_probe(wrp->dev, PTR_ERR(clk),
>   				     "failed to get clock: %pe\n",
> -				     wrp->clk_sys);
> +				     clk);
>   	}
>   
> -	wrp->clk_tmr = devm_clk_get_optional(wrp->dev, "tmr");
> -	if (IS_ERR(wrp->clk_tmr)) {
> -		return dev_err_probe(wrp->dev, PTR_ERR(wrp->clk_tmr),
> +	clk = devm_clk_get_optional_enabled(wrp->dev, "tmr");
> +	if (IS_ERR(clk)) {
> +		return dev_err_probe(wrp->dev, PTR_ERR(clk),
>   				     "failed to get clock: %pe\n",
> -				     wrp->clk_tmr);
> +				     clk);
>   	}
>   
> -	ret = clk_prepare_enable(wrp->clk_spi);
> -	if (ret)
> -		return ret;
> -
> -	ret = clk_prepare_enable(wrp->clk_wrap);
> -	if (ret)
> -		goto err_out1;
> -
> -	ret = clk_prepare_enable(wrp->clk_sys);
> -	if (ret)
> -		goto err_out2;
> -
> -	ret = clk_prepare_enable(wrp->clk_tmr);
> -	if (ret)
> -		goto err_out3;
> -
>   	/* Enable internal dynamic clock */
>   	if (HAS_CAP(wrp->master->caps, PWRAP_CAP_DCM)) {
>   		pwrap_writel(wrp, 1, PWRAP_DCM_EN);
> @@ -2579,7 +2560,7 @@ static int pwrap_probe(struct platform_device *pdev)
>   		ret = pwrap_init(wrp);
>   		if (ret) {
>   			dev_dbg(wrp->dev, "init failed with %d\n", ret);
> -			goto err_out4;
> +			return ret;
>   		}
>   	}
>   
> @@ -2592,8 +2573,7 @@ static int pwrap_probe(struct platform_device *pdev)
>   
>   	if (!(pwrap_readl(wrp, PWRAP_WACS2_RDATA) & mask_done)) {
>   		dev_dbg(wrp->dev, "initialization isn't finished\n");
> -		ret = -ENODEV;
> -		goto err_out4;
> +		return -ENODEV;
>   	}
>   
>   	/* Initialize watchdog, may not be done by the bootloader */
> @@ -2622,42 +2602,27 @@ static int pwrap_probe(struct platform_device *pdev)
>   		pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN);
>   
>   	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		ret = irq;
> -		goto err_out2;
> -	}
> +	if (irq < 0)
> +		return irq;
>   
>   	ret = devm_request_irq(wrp->dev, irq, pwrap_interrupt,
>   			       IRQF_TRIGGER_HIGH,
>   			       "mt-pmic-pwrap", wrp);
>   	if (ret)
> -		goto err_out4;
> +		return ret;
>   
>   	wrp->regmap = devm_regmap_init(wrp->dev, NULL, wrp, wrp->slave->regops->regmap);
> -	if (IS_ERR(wrp->regmap)) {
> -		ret = PTR_ERR(wrp->regmap);
> -		goto err_out2;
> -	}
> +	if (IS_ERR(wrp->regmap))
> +		return PTR_ERR(wrp->regmap);
>   
>   	ret = of_platform_populate(np, NULL, NULL, wrp->dev);
>   	if (ret) {
>   		dev_dbg(wrp->dev, "failed to create child devices at %pOF\n",
>   				np);
> -		goto err_out4;
> +		return ret;
>   	}
>   
>   	return 0;
> -
> -err_out4:
> -	clk_disable_unprepare(wrp->clk_tmr);
> -err_out3:
> -	clk_disable_unprepare(wrp->clk_sys);
> -err_out2:
> -	clk_disable_unprepare(wrp->clk_wrap);
> -err_out1:
> -	clk_disable_unprepare(wrp->clk_spi);
> -
> -	return ret;
>   }
>   
>   static struct platform_driver pwrap_drv = {