[PATCH 0/4] mfd: exynos-lpass: Fix some error handling paths

[PATCH 0/4] mfd: exynos-lpass: Fix some error handling paths

[Christophe JAILLET]

This serie fixes several issues related to error handling paths in
drivers/mfd/exynos-lpass.c.

I've split it in 4 patches to ease review, but could be merge in only
1 patch. Or only (3 and 4) could be merged, as they are related.

Patch 1: Fix a leak in the error handling path of the probe. It Should
be straighforward.

Patch 2: Slighly unsure of the order of the code. In the probe, we
enable pm, then lpass, so should we disable lpass, then pm?

Patch 3: Is just a preparation for patch 4. It makes patch 4 more
readable.

Patch 4: Fix a leak in the error handling path of the probe. It Should
be straighforward.


All these patches are compile tested only.

Christophe JAILLET (4):
  mfd: exynos-lpass: Fix an error handling path in exynos_lpass_probe()
  mfd: exynos-lpass: Avoid calling exynos_lpass_disable() twice in
    exynos_lpass_remove()
  mfd: exynos-lpass: Move exynos_lpass_remove()
  mfd: exynos-lpass: Fix another error handling path in
    exynos_lpass_probe()

 drivers/mfd/exynos-lpass.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

-- 
2.49.0

[PATCH 1/4] mfd: exynos-lpass: Fix an error handling path in exynos_lpass_probe()

[Christophe JAILLET]

If an error occurs after a successful regmap_init_mmio(), regmap_exit()
should be called as already done in the .remove() function.

Switch to devm_regmap_init_mmio() to avoid the leak and simplify the
.remove() function.

Fixes: c695abab2429 ("mfd: Add Samsung Exynos Low Power Audio Subsystem driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
The .remove function was added in commit c414df12bdf7 ("mfd: exynos-lpass:
Add missing remove() function")

Compile tested only.
---
 drivers/mfd/exynos-lpass.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/exynos-lpass.c b/drivers/mfd/exynos-lpass.c
index 6a585173230b..6b95927e99be 100644
--- a/drivers/mfd/exynos-lpass.c
+++ b/drivers/mfd/exynos-lpass.c
@@ -122,8 +122,8 @@ static int exynos_lpass_probe(struct platform_device *pdev)
 	if (IS_ERR(lpass->sfr0_clk))
 		return PTR_ERR(lpass->sfr0_clk);
 
-	lpass->top = regmap_init_mmio(dev, base_top,
-					&exynos_lpass_reg_conf);
+	lpass->top = devm_regmap_init_mmio(dev, base_top,
+					   &exynos_lpass_reg_conf);
 	if (IS_ERR(lpass->top)) {
 		dev_err(dev, "LPASS top regmap initialization failed\n");
 		return PTR_ERR(lpass->top);
@@ -145,7 +145,6 @@ static void exynos_lpass_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	if (!pm_runtime_status_suspended(&pdev->dev))
 		exynos_lpass_disable(lpass);
-	regmap_exit(lpass->top);
 }
 
 static int __maybe_unused exynos_lpass_suspend(struct device *dev)
-- 
2.49.0

[PATCH 2/4] mfd: exynos-lpass: Avoid calling exynos_lpass_disable() twice in exynos_lpass_remove()

[Christophe JAILLET]

exynos_lpass_disable() is called twice in the remove function. Remove
one of these calls.

Fixes: 90f447170c6f ("mfd: exynos-lpass: Add runtime PM support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
---
 drivers/mfd/exynos-lpass.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/exynos-lpass.c b/drivers/mfd/exynos-lpass.c
index 6b95927e99be..a2785ceea8bf 100644
--- a/drivers/mfd/exynos-lpass.c
+++ b/drivers/mfd/exynos-lpass.c
@@ -141,7 +141,6 @@ static void exynos_lpass_remove(struct platform_device *pdev)
 {
 	struct exynos_lpass *lpass = platform_get_drvdata(pdev);
 
-	exynos_lpass_disable(lpass);
 	pm_runtime_disable(&pdev->dev);
 	if (!pm_runtime_status_suspended(&pdev->dev))
 		exynos_lpass_disable(lpass);
-- 
2.49.0

[PATCH 3/4] mfd: exynos-lpass: Move exynos_lpass_remove()

[Christophe JAILLET]

In order be able to call exynos_lpass_remove() from the error handling
path of the probe, it first needs to be moved before the probe.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/mfd/exynos-lpass.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/exynos-lpass.c b/drivers/mfd/exynos-lpass.c
index a2785ceea8bf..7fd8585ba35a 100644
--- a/drivers/mfd/exynos-lpass.c
+++ b/drivers/mfd/exynos-lpass.c
@@ -104,6 +104,15 @@ static const struct regmap_config exynos_lpass_reg_conf = {
 	.fast_io	= true,
 };
 
+static void exynos_lpass_remove(struct platform_device *pdev)
+{
+	struct exynos_lpass *lpass = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		exynos_lpass_disable(lpass);
+}
+
 static int exynos_lpass_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -137,15 +146,6 @@ static int exynos_lpass_probe(struct platform_device *pdev)
 	return devm_of_platform_populate(dev);
 }
 
-static void exynos_lpass_remove(struct platform_device *pdev)
-{
-	struct exynos_lpass *lpass = platform_get_drvdata(pdev);
-
-	pm_runtime_disable(&pdev->dev);
-	if (!pm_runtime_status_suspended(&pdev->dev))
-		exynos_lpass_disable(lpass);
-}
-
 static int __maybe_unused exynos_lpass_suspend(struct device *dev)
 {
 	struct exynos_lpass *lpass = dev_get_drvdata(dev);
-- 
2.49.0

[PATCH 4/4] mfd: exynos-lpass: Fix another error handling path in exynos_lpass_probe()

[Christophe JAILLET]

If devm_of_platform_populate() fails, some clean-up needs to be done, as
already done in the remove function.

Add the needed error handling path.

Fixes: c695abab2429 ("mfd: Add Samsung Exynos Low Power Audio Subsystem driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
---
 drivers/mfd/exynos-lpass.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/exynos-lpass.c b/drivers/mfd/exynos-lpass.c
index 7fd8585ba35a..9f2601e4e583 100644
--- a/drivers/mfd/exynos-lpass.c
+++ b/drivers/mfd/exynos-lpass.c
@@ -118,6 +118,7 @@ static int exynos_lpass_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct exynos_lpass *lpass;
 	void __iomem *base_top;
+	int ret;
 
 	lpass = devm_kzalloc(dev, sizeof(*lpass), GFP_KERNEL);
 	if (!lpass)
@@ -143,7 +144,15 @@ static int exynos_lpass_probe(struct platform_device *pdev)
 	pm_runtime_enable(dev);
 	exynos_lpass_enable(lpass);
 
-	return devm_of_platform_populate(dev);
+	ret = devm_of_platform_populate(dev);
+	if (ret)
+		goto err_remove_exynos_lpass;
+
+	return 0;
+
+err_remove_exynos_lpass:
+	exynos_lpass_remove(pdev);
+	return ret;
 }
 
 static int __maybe_unused exynos_lpass_suspend(struct device *dev)
-- 
2.49.0

Re: [PATCH 1/4] mfd: exynos-lpass: Fix an error handling path in exynos_lpass_probe()

[Krzysztof Kozlowski]

On 29/03/2025 08:45, Christophe JAILLET wrote:
> If an error occurs after a successful regmap_init_mmio(), regmap_exit()
> should be called as already done in the .remove() function.
> 
> Switch to devm_regmap_init_mmio() to avoid the leak and simplify the
> .remove() function.
> 
> Fixes: c695abab2429 ("mfd: Add Samsung Exynos Low Power Audio Subsystem driver")

c414df12bdf7 should be fixed instead, I think.

> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> The .remove function was added in commit c414df12bdf7 ("mfd: exynos-lpass:
> Add missing remove() function")

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 2/4] mfd: exynos-lpass: Avoid calling exynos_lpass_disable() twice in exynos_lpass_remove()

[Krzysztof Kozlowski]

On 29/03/2025 08:45, Christophe JAILLET wrote:
> exynos_lpass_disable() is called twice in the remove function. Remove
> one of these calls.
> 
> Fixes: 90f447170c6f ("mfd: exynos-lpass: Add runtime PM support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only.
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 3/4] mfd: exynos-lpass: Move exynos_lpass_remove()

[Krzysztof Kozlowski]

On 29/03/2025 08:45, Christophe JAILLET wrote:
> In order be able to call exynos_lpass_remove() from the error handling
> path of the probe, it first needs to be moved before the probe.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---

This change makes no sense on its own. There is no point in moving the
code just for moving the code. That's not the goal.

Squash it.

Best regards,
Krzysztof

Re: [PATCH 4/4] mfd: exynos-lpass: Fix another error handling path in exynos_lpass_probe()

[Krzysztof Kozlowski]

On 29/03/2025 08:45, Christophe JAILLET wrote:
>  	lpass = devm_kzalloc(dev, sizeof(*lpass), GFP_KERNEL);
>  	if (!lpass)
> @@ -143,7 +144,15 @@ static int exynos_lpass_probe(struct platform_device *pdev)
>  	pm_runtime_enable(dev);
>  	exynos_lpass_enable(lpass);
>  
> -	return devm_of_platform_populate(dev);
> +	ret = devm_of_platform_populate(dev);
> +	if (ret)
> +		goto err_remove_exynos_lpass;
> +
> +	return 0;
> +
> +err_remove_exynos_lpass:
> +	exynos_lpass_remove(pdev);

You should have here standard unwinding, not calling remove. Or better
just convert everything to devm, because mixing devm and non-devm is
error prone.

Best regards,
Krzysztof