diff mbox series

[leds,v2,08/11] leds: turris-omnia: Use dev_err_probe() where appropriate

Message ID 20240903101930.16251-9-kabel@kernel.org
State Superseded
Headers show
Series Turris Omnia LED driver changes | expand

Commit Message

Marek Behún Sept. 3, 2024, 10:19 a.m. UTC 
Use dev_err_probe() instead of dev_err() + separate return where
appropriate.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/leds/leds-turris-omnia.c | 50 ++++++++++----------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

Comments

Marek Behún Sept. 4, 2024, 7:37 a.m. UTC | #1 
On Tue, Sep 03, 2024 at 05:56:29PM +0200, Andrew Lunn wrote:
> On Tue, Sep 03, 2024 at 01:45:49PM +0300, Andy Shevchenko wrote:
> > On Tue, Sep 3, 2024 at 1:20 PM Marek Behún <kabel@kernel.org> wrote:
> > >
> > > Use dev_err_probe() instead of dev_err() + separate return where
> > > appropriate.
> > 
> > ...
> > 
> > > +       if (ret)
> > > +               return dev_err_probe(dev, ret, "Cannot set LED %pOF to software mode\n", np);
> > 
> > Side note: Not sure how np is being used besides the messaging. If
> > it's only for the messaging, I would rather see %pfw and fwnode based
> > approach.
> 
> This board has a number of Ethernet switches described in DT.  Nobody
> takes ACPI seriously for any sort of complex networking. So using
> fwnode is probably pointless, this board will probably never be used
> with anything other than DT.

Although this is true, my opinion is that converting drivers to fwnode
API would still have some theoretical merit. For example using the
device_property_*() functions, where possible, seems nicer semantically.

I think I tried it once, or at least asked Pavel if I should, and he
turned me down, so I abandoned the effort.

But the patch would be a very simple one.

Marek
diff mbox series

Patch

diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
index 8d3bddc90fe0..857dba811d5e 100644
--- a/drivers/leds/leds-turris-omnia.c
+++ b/drivers/leds/leds-turris-omnia.c
@@ -238,33 +238,23 @@  static int omnia_led_register(struct i2c_client *client, struct omnia_led *led,
 	/* put the LED into software mode */
 	ret = omnia_cmd_write_u8(client, OMNIA_CMD_LED_MODE, OMNIA_CMD_LED_MODE_LED(led->reg) |
 							     OMNIA_CMD_LED_MODE_USER);
-	if (ret) {
-		dev_err(dev, "Cannot set LED %pOF to software mode: %i\n", np,
-			ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot set LED %pOF to software mode\n", np);
 
 	/* disable the LED */
 	ret = omnia_cmd_write_u8(client, OMNIA_CMD_LED_STATE, OMNIA_CMD_LED_STATE_LED(led->reg));
-	if (ret) {
-		dev_err(dev, "Cannot set LED %pOF brightness: %i\n", np, ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot set LED %pOF brightness\n", np);
 
 	/* Set initial color and cache it */
 	ret = omnia_led_send_color_cmd(client, led);
-	if (ret < 0) {
-		dev_err(dev, "Cannot set LED %pOF initial color: %i\n", np,
-			ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Cannot set LED %pOF initial color\n", np);
 
 	ret = devm_led_classdev_multicolor_register_ext(dev, &led->mc_cdev,
 							&init_data);
-	if (ret < 0) {
-		dev_err(dev, "Cannot register LED %pOF: %i\n", np, ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Cannot register LED %pOF\n", np);
 
 	return 1;
 }
@@ -406,13 +396,10 @@  static int omnia_leds_probe(struct i2c_client *client)
 	int ret, count;
 
 	count = of_get_available_child_count(np);
-	if (!count) {
-		dev_err(dev, "LEDs are not defined in device tree!\n");
-		return -ENODEV;
-	} else if (count > OMNIA_BOARD_LEDS) {
-		dev_err(dev, "Too many LEDs defined in device tree!\n");
-		return -EINVAL;
-	}
+	if (!count)
+		return dev_err_probe(dev, -ENODEV, "LEDs are not defined in device tree!\n");
+	else if (count > OMNIA_BOARD_LEDS)
+		return dev_err_probe(dev, -EINVAL, "Too many LEDs defined in device tree!\n");
 
 	leds = devm_kzalloc(dev, struct_size(leds, leds, count), GFP_KERNEL);
 	if (!leds)
@@ -422,11 +409,8 @@  static int omnia_leds_probe(struct i2c_client *client)
 	i2c_set_clientdata(client, leds);
 
 	ret = omnia_mcu_get_features(client);
-	if (ret < 0) {
-		dev_err(dev, "Cannot determine MCU supported features: %d\n",
-			ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Cannot determine MCU supported features\n");
 
 	leds->has_gamma_correction = ret & OMNIA_FEAT_LED_GAMMA_CORRECTION;
 
@@ -441,10 +425,8 @@  static int omnia_leds_probe(struct i2c_client *client)
 	mutex_init(&leds->lock);
 
 	ret = devm_led_trigger_register(dev, &omnia_hw_trigger);
-	if (ret < 0) {
-		dev_err(dev, "Cannot register private LED trigger: %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Cannot register private LED trigger\n");
 
 	led = &leds->leds[0];
 	for_each_available_child_of_node_scoped(np, child) {