Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 23 Nov 2022 14:00:38 +0200
From:   Matti Vaittinen <mazziesaccount@gmail.com>
To:     Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
        Matti Vaittinen <mazziesaccount@gmail.com>
Cc:     Matti Vaittinen <mazziesaccount@gmail.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>, linux-kernel@vger.kernel.org
Subject: [PATCH 3/3] regulator: bd718x7: Use dev_err_probe()
Message-ID: <fee54c2d04bb41b51381e31523f9ed31575206d2.1669203610.git.mazziesaccount@gmail.com>
References: <cover.1669203610.git.mazziesaccount@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="+iqplJKpcdzleyKU"
Content-Disposition: inline
In-Reply-To: <cover.1669203610.git.mazziesaccount@gmail.com>
Precedence: bulk


--+iqplJKpcdzleyKU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

The dev_err_probe() has (at least) following benefits over dev_err()
when printing an error print for a failed function call at a device
driver probe:
        - Omit error level print if error is 'EPRBE_DEFER'
        - Standardized print format for returned error
        - return the error value allowing shortening calls like:

        if (ret) {
                dev_err(...);
                return ret;
        }

        to

        if (ret)
                return dev_err_probe(...);

Convert the ROHM BD718x7 regulator driver to use the dev_err_probe() when
returned error is not hard-coded constant.

NOTE:
This commit also changes the error handling path to return immediately
=66rom a spot where the error is spotted instead of using a single point
of exit.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
For many years I have preferred a single point of exit in a function
when managing it does not require any big tricks. In my experience a
single point of exit makes resource-leaking and lock releasing much less
error prone. Eg, consider mistakes like:

=2E..
mutex_lock(&mtx)
=2E..
if (err)
	return err;
=2E..
mutex_unlock(&mtx);

return 0;

Vs.

=2E..
mutex_lock(&mtx)
=2E..
if (err)
	goto err_out;
=2E..

err_out:
mutex_unlock(&mtx);

return err;

I still think a single point of exit is often a good idea. However, I am
slowly adapting to thought that the single point of exit does not really
play a big role in bd718x7 regulator probe and using dev_err_probe()
allows us to avoid the extra {} after condition... So, maybe it indeed
is a time for me to ditch the goto here. Please, let me know if you
think othervice :)
---
 drivers/regulator/bd718x7-regulator.c | 51 ++++++++++-----------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd71=
8x7-regulator.c
index d161b0026f33..894fab0d53d0 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -1706,20 +1706,17 @@ static int bd718xx_probe(struct platform_device *pd=
ev)
 		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported chip type\n");
-		err =3D -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
=20
 	/* Register LOCK release */
 	err =3D regmap_update_bits(regmap, BD718XX_REG_REGLOCK,
 				 (REGLOCK_PWRSEQ | REGLOCK_VREG), 0);
-	if (err) {
-		dev_err(&pdev->dev, "Failed to unlock PMIC (%d)\n", err);
-		goto err;
-	} else {
-		dev_dbg(&pdev->dev, "Unlocked lock register 0x%x\n",
-			BD718XX_REG_REGLOCK);
-	}
+	if (err)
+		return dev_err_probe(&pdev->dev, err, "Failed to unlock PMIC\n");
+
+	dev_dbg(&pdev->dev, "Unlocked lock register 0x%x\n",
+		BD718XX_REG_REGLOCK);
=20
 	use_snvs =3D of_property_read_bool(pdev->dev.parent->of_node,
 					 "rohm,reset-snvs-powered");
@@ -1736,13 +1733,11 @@ static int bd718xx_probe(struct platform_device *pd=
ev)
 					 BD718XX_WDOG_POWEROFF_MASK |
 					 BD718XX_KEY_L_POWEROFF_MASK,
 					 BD718XX_POWOFF_TO_RDY);
-		if (err) {
-			dev_err(&pdev->dev, "Failed to change reset target\n");
-			goto err;
-		} else {
-			dev_dbg(&pdev->dev,
-				"Changed all resets from SVNS to READY\n");
-		}
+		if (err)
+			return dev_err_probe(&pdev->dev, err,
+					     "Failed to change reset target\n");
+
+		dev_dbg(&pdev->dev, "Changed all resets from SVNS to READY\n");
 	}
=20
 	config.dev =3D pdev->dev.parent;
@@ -1778,13 +1773,10 @@ static int bd718xx_probe(struct platform_device *pd=
ev)
 			desc->ops =3D swops[i];
=20
 		rdev =3D devm_regulator_register(&pdev->dev, desc, &config);
-		if (IS_ERR(rdev)) {
-			dev_err(&pdev->dev,
-				"failed to register %s regulator\n",
-				desc->name);
-			err =3D PTR_ERR(rdev);
-			goto err;
-		}
+		if (IS_ERR(rdev))
+			return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+					     "failed to register %s regulator\n",
+					     desc->name);
=20
 		/*
 		 * Regulator register gets the regulator constraints and
@@ -1807,28 +1799,23 @@ static int bd718xx_probe(struct platform_device *pd=
ev)
 		    !rdev->constraints->boot_on)) {
 			err =3D regmap_update_bits(regmap, r->init.reg,
 						 r->init.mask, r->init.val);
-			if (err) {
-				dev_err(&pdev->dev,
+			if (err)
+				return dev_err_probe(&pdev->dev, err,
 					"Failed to take control for (%s)\n",
 					desc->name);
-				goto err;
-			}
 		}
 		for (j =3D 0; j < r->additional_init_amnt; j++) {
 			err =3D regmap_update_bits(regmap,
 						 r->additional_inits[j].reg,
 						 r->additional_inits[j].mask,
 						 r->additional_inits[j].val);
-			if (err) {
-				dev_err(&pdev->dev,
+			if (err)
+				return dev_err_probe(&pdev->dev, err,
 					"Buck (%s) initialization failed\n",
 					desc->name);
-				goto err;
-			}
 		}
 	}
=20
-err:
 	return err;
 }
=20
--=20
2.38.1


--=20
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =3D]=20

--+iqplJKpcdzleyKU
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEIx+f8wZb28fLKEhTeFA3/03aocUFAmN+C2YACgkQeFA3/03a
ocUWRgf/fEgNu7YYaMh5NmKk50Btq9kWI28dfnEKIa8PQAhJ6IePYvwfwAP7p4Ke
mr+XktaBV0CQi2e4yENTKuZMk1XKGY4mHuO7rdMNYrbR6EqWmia0ijZwai4FAi9Q
2Iga1IYdAHtuU5RX4yuFr8E8r7ZzppxBU4RjslIMt7MB8/kl/xqSdJWaSVLs0oLb
CQ4bftM8X5LHsRoQ3swbJIRQVBt3FsZEokDARIgaUSogykSd8SoOCY9/usThzH6Q
004f5ugNtR5XwwYyXvkDOJo4DRgcA8i3yrTZ/YpVLJMZwEBVkagnlZ18NnZqXpn6
tjyl8BWn58Gthij524kFUh71QlhkaA==
=47Ei
-----END PGP SIGNATURE-----

--+iqplJKpcdzleyKU--