Re: [PATCH 1/2] backlight: bd6107: Make use of the helper function dev_err_probe()

Hi
Thanks for your feedback.
On 17 9月 21 10:17:29, Daniel Thompson wrote:

On Fri, Sep 17, 2021 at 11:13:06AM +0800, Cai Huoqing wrote:

quoted

When possible use dev_err_probe help to properly deal with the
PROBE_DEFER error, the benefit is that DEFER issue will be logged
in the devices_deferred debugfs file.
Using dev_err_probe() can reduce code size, and the error value
gets printed.

Signed-off-by: Cai Huoqing <redacted>

Change seems OK but does this really need to be done one file at a time?
I'd prefer to see all of backlight handled in one go (given the scope of
this change if applied across the kernel it needs automatic tooling
anyway).

Only two related patches for backlight.
I try to keep one patch for a subdriver, sometimes different
subdriver owner need to mark reviwed independently.

Thanks,
Cai

Daniel.

quoted

---
 drivers/video/backlight/bd6107.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/video/backlight/bd6107.c b/drivers/video/backlight/bd6107.c
index 515184fbe33a..e21b793302a2 100644
--- a/drivers/video/backlight/bd6107.c
+++ b/drivers/video/backlight/bd6107.c

@@ -120,7 +120,6 @@ static int bd6107_probe(struct i2c_client *client,
 	struct backlight_device *backlight;
 	struct backlight_properties props;
 	struct bd6107 *bd;
-	int ret;
 
 	if (pdata == NULL) {
 		dev_err(&client->dev, "No platform data\n");

@@ -148,11 +147,9 @@ static int bd6107_probe(struct i2c_client *client,
 	 * the reset.
 	 */
 	bd->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(bd->reset)) {
-		dev_err(&client->dev, "unable to request reset GPIO\n");
-		ret = PTR_ERR(bd->reset);
-		return ret;
-	}
+	if (IS_ERR(bd->reset))
+		return dev_err_probe(&client->dev, PTR_ERR(bd->reset),
+				     "unable to request reset GPIO\n");
 
 	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_RAW;

@@ -164,10 +161,9 @@ static int bd6107_probe(struct i2c_client *client,
 					      dev_name(&client->dev),
 					      &bd->client->dev, bd,
 					      &bd6107_backlight_ops, &props);
-	if (IS_ERR(backlight)) {
-		dev_err(&client->dev, "failed to register backlight\n");
-		return PTR_ERR(backlight);
-	}
+	if (IS_ERR(backlight))
+		return dev_err_probe(&client->dev, PTR_ERR(backlight),
+				     "failed to register backlight\n");
 
 	backlight_update_status(backlight);
 	i2c_set_clientdata(client, backlight);

-- 
2.25.1