2022-11-23 12:23:38

by Matti Vaittinen

[permalink] [raw]
Subject: [PATCH 0/3] regulator: ROHM BDxxxxx minor print improvements

Minor (printing) improvements for ROHM regulator drivers.

This series:

- Drops an unnecessary info print from bd718x7.
(Added a fixes tag for this but not really sure if worth
adding to stable)
- Convert the ROHM BDxxxxx PMIC regulator drivers to use dev_err_probe().
- Change the probe logic for bd718x7 to favor the more usual devm-style
where errors are returned immediately.

Patch 1:
- Drop the unnecessary print. (a fix)
Patch 2:
- Do simple conversion to dev_err() => dev_err_probe()
Patch 3:
- Convert bd718x7 to use dev_err_probe() and drop the goto-based
single point of exit.

---

Matti Vaittinen (3):
regulator: bd718x7: Drop unnecessary info print
regulator: bd71815: bd71828: bd9576: Use dev_err_probe()
regulator: bd718x7: Use dev_err_probe()

drivers/regulator/bd71815-regulator.c | 10 ++---
drivers/regulator/bd71828-regulator.c | 21 +++++------
drivers/regulator/bd718x7-regulator.c | 53 ++++++++++----------------
drivers/regulator/bd9576-regulator.c | 54 +++++++++++++--------------
4 files changed, 57 insertions(+), 81 deletions(-)


base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
--
2.38.1


--
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 =]


Attachments:
(No filename) (1.50 kB)
signature.asc (499.00 B)
Download all attachments

2022-11-23 12:42:07

by Matti Vaittinen

[permalink] [raw]
Subject: [PATCH 2/3] regulator: bd71815: bd71828: bd9576: Use dev_err_probe()

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 BD71828, ROHM BD71815 and ROHM BD9576 regulator drivers
to use the dev_err_probe() when returned error is not hard-coded constant.

Signed-off-by: Matti Vaittinen <[email protected]>
---
drivers/regulator/bd71815-regulator.c | 10 ++---
drivers/regulator/bd71828-regulator.c | 21 +++++------
drivers/regulator/bd9576-regulator.c | 54 +++++++++++++--------------
3 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/drivers/regulator/bd71815-regulator.c b/drivers/regulator/bd71815-regulator.c
index c2b8b8be7824..8b55046eded8 100644
--- a/drivers/regulator/bd71815-regulator.c
+++ b/drivers/regulator/bd71815-regulator.c
@@ -602,12 +602,10 @@ static int bd7181x_probe(struct platform_device *pdev)
config.ena_gpiod = NULL;

rdev = devm_regulator_register(&pdev->dev, desc, &config);
- if (IS_ERR(rdev)) {
- dev_err(&pdev->dev,
- "failed to register %s regulator\n",
- desc->name);
- return PTR_ERR(rdev);
- }
+ if (IS_ERR(rdev))
+ return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+ "failed to register %s regulator\n",
+ desc->name);
}
return 0;
}
diff --git a/drivers/regulator/bd71828-regulator.c b/drivers/regulator/bd71828-regulator.c
index a4f09a5a30ca..ad728f4f2241 100644
--- a/drivers/regulator/bd71828-regulator.c
+++ b/drivers/regulator/bd71828-regulator.c
@@ -750,23 +750,20 @@ static int bd71828_probe(struct platform_device *pdev)
rd = &bd71828_rdata[i];
rdev = devm_regulator_register(&pdev->dev,
&rd->desc, &config);
- if (IS_ERR(rdev)) {
- dev_err(&pdev->dev,
- "failed to register %s regulator\n",
- rd->desc.name);
- return PTR_ERR(rdev);
- }
+ if (IS_ERR(rdev))
+ return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+ "failed to register %s regulator\n",
+ rd->desc.name);
+
for (j = 0; j < rd->reg_init_amnt; j++) {
ret = regmap_update_bits(config.regmap,
rd->reg_inits[j].reg,
rd->reg_inits[j].mask,
rd->reg_inits[j].val);
- if (ret) {
- dev_err(&pdev->dev,
- "regulator %s init failed\n",
- rd->desc.name);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "regulator %s init failed\n",
+ rd->desc.name);
}
}
return 0;
diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c
index 393c8693b327..02c70768652b 100644
--- a/drivers/regulator/bd9576-regulator.c
+++ b/drivers/regulator/bd9576-regulator.c
@@ -953,30 +953,28 @@ static int bd957x_probe(struct platform_device *pdev)
dev_fwnode(pdev->dev.parent),
"rohm,vout1-en", GPIOD_OUT_LOW,
"vout1-en");
- if (!IS_ERR(en)) {
- /* VOUT1_OPS gpio ctrl */
- /*
- * Regulator core prioritizes the ena_gpio over
- * enable/disable/is_enabled callbacks so no need to
- * clear them. We can still use same ops
- */
+
+ /* VOUT1_OPS gpio ctrl */
+ /*
+ * Regulator core prioritizes the ena_gpio over
+ * enable/disable/is_enabled callbacks so no need to clear them
+ * even if GPIO is used. So, we can still use same ops.
+ *
+ * In theory it is possible someone wants to set vout1-en LOW
+ * during OTP loading and set VOUT1 to be controlled by GPIO -
+ * but control the GPIO from some where else than this driver.
+ * For that to work we should unset the is_enabled callback
+ * here.
+ *
+ * I believe such case where rohm,vout1-en-low is set and
+ * vout1-en-gpios is not is likely to be a misconfiguration.
+ * So let's just err out for now.
+ */
+ if (!IS_ERR(en))
config.ena_gpiod = en;
- } else {
- /*
- * In theory it is possible someone wants to set
- * vout1-en LOW during OTP loading and set VOUT1 to be
- * controlled by GPIO - but control the GPIO from some
- * where else than this driver. For that to work we
- * should unset the is_enabled callback here.
- *
- * I believe such case where rohm,vout1-en-low is set
- * and vout1-en-gpios is not is likely to be a
- * misconfiguration. So let's just err out for now.
- */
- dev_err(&pdev->dev,
- "Failed to get VOUT1 control GPIO\n");
- return PTR_ERR(en);
- }
+ else
+ return dev_err_probe(&pdev->dev, PTR_ERR(en),
+ "Failed to get VOUT1 control GPIO\n");
}

/*
@@ -1037,12 +1035,10 @@ static int bd957x_probe(struct platform_device *pdev)

r->rdev = devm_regulator_register(&pdev->dev, desc,
&config);
- if (IS_ERR(r->rdev)) {
- dev_err(&pdev->dev,
- "failed to register %s regulator\n",
- desc->name);
- return PTR_ERR(r->rdev);
- }
+ if (IS_ERR(r->rdev))
+ return dev_err_probe(&pdev->dev, PTR_ERR(r->rdev),
+ "failed to register %s regulator\n",
+ desc->name);
/*
* Clear the VOUT1 GPIO setting - rest of the regulators do not
* support GPIO control
--
2.38.1


--
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 =]


Attachments:
(No filename) (5.70 kB)
signature.asc (499.00 B)
Download all attachments

2022-11-23 17:55:52

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/3] regulator: ROHM BDxxxxx minor print improvements

On Wed, 23 Nov 2022 13:59:48 +0200, Matti Vaittinen wrote:
> Minor (printing) improvements for ROHM regulator drivers.
>
> This series:
>
> - Drops an unnecessary info print from bd718x7.
> (Added a fixes tag for this but not really sure if worth
> adding to stable)
> - Convert the ROHM BDxxxxx PMIC regulator drivers to use dev_err_probe().
> - Change the probe logic for bd718x7 to favor the more usual devm-style
> where errors are returned immediately.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/3] regulator: bd718x7: Drop unnecessary info print
commit: 44501eba9bb28946382b7a53099ce8098d1610f0
[2/3] regulator: bd71815: bd71828: bd9576: Use dev_err_probe()
commit: d4e93e8da012880882671a46ac6ae3aefcae8076
[3/3] regulator: bd718x7: Use dev_err_probe()
commit: 662a9bf883f42d09145cf083d93dadf0307f15ea

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark