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 caihuoqing@baidu.com --- 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);
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 caihuoqing@baidu.com --- drivers/video/backlight/l4f00242t03.c | 34 ++++++++++----------------- 1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c index 46f97d1c3d21..8d81d4dec3c6 100644 --- a/drivers/video/backlight/l4f00242t03.c +++ b/drivers/video/backlight/l4f00242t03.c @@ -179,37 +179,29 @@ static int l4f00242t03_probe(struct spi_device *spi) priv->spi = spi;
priv->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(priv->reset)) { - dev_err(&spi->dev, - "Unable to get the lcd l4f00242t03 reset gpio.\n"); - return PTR_ERR(priv->reset); - } + if (IS_ERR(priv->reset)) + return dev_err_probe(&spi->dev, PTR_ERR(priv->reset), + "Unable to get the lcd l4f00242t03 reset gpio.\n"); gpiod_set_consumer_name(priv->reset, "lcd l4f00242t03 reset");
priv->enable = devm_gpiod_get(&spi->dev, "enable", GPIOD_OUT_LOW); - if (IS_ERR(priv->enable)) { - dev_err(&spi->dev, - "Unable to get the lcd l4f00242t03 data en gpio.\n"); - return PTR_ERR(priv->enable); - } + if (IS_ERR(priv->enable)) + return dev_err_probe(&spi->dev, PTR_ERR(priv->enable), + "Unable to get the lcd l4f00242t03 data en gpio.\n"); gpiod_set_consumer_name(priv->enable, "lcd l4f00242t03 data enable");
priv->io_reg = devm_regulator_get(&spi->dev, "vdd"); - if (IS_ERR(priv->io_reg)) { - dev_err(&spi->dev, "%s: Unable to get the IO regulator\n", - __func__); - return PTR_ERR(priv->io_reg); - } + if (IS_ERR(priv->io_reg)) + return dev_err_probe(&spi->dev, PTR_ERR(priv->io_reg), + "%s: Unable to get the IO regulator\n", __func__);
priv->core_reg = devm_regulator_get(&spi->dev, "vcore"); - if (IS_ERR(priv->core_reg)) { - dev_err(&spi->dev, "%s: Unable to get the core regulator\n", - __func__); - return PTR_ERR(priv->core_reg); - } + if (IS_ERR(priv->core_reg)) + return dev_err_probe(&spi->dev, PTR_ERR(priv->core_reg), + "%s: Unable to get the core regulator\n", __func__);
priv->ld = devm_lcd_device_register(&spi->dev, "l4f00242t03", &spi->dev, - priv, &l4f_ops); + priv, &l4f_ops); if (IS_ERR(priv->ld)) return PTR_ERR(priv->ld);
On Fri, Sep 17, 2021 at 11:13:06AM +0800, Cai Huoqing wrote:
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 caihuoqing@baidu.com
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).
Daniel.
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
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:
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 caihuoqing@baidu.com
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.
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
On Fri, Sep 17, 2021 at 07:05:28PM +0800, Cai Huoqing wrote:
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:
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 caihuoqing@baidu.com
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.
Can you explain how you reach this conclusion?
I only checked two drivers (the first two when you list the drivers an alphabetic order) but both contain code that appears to match the pattern you are targeting.
I try to keep one patch for a subdriver, sometimes different subdriver owner need to mark reviwed independently.
For mechanical patches of this nature I don't think there is any need for acks from individual backlight driver authors. Reviews are 100% welcome but we would not hold a single patch back until all authors have reviewed it.
Daniel.
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
dri-devel@lists.freedesktop.org
-
Cai Huoqing -
Daniel Thompson