Re: [PATCH] leds: lp3952: Store leds[LP3952_LED_ALL] in struct lp3952_led_array

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Axel,

Thanks for catching this. I missed also redundant devm_kfree here.
Would it be OK for you if I merged your cleanup with the original
patch and added your Reviewed-by to the commit message instead?
This driver hasn't been pushed to the mainline yet, so it would be
nice not to introduce its original version with some shortcomings
that need to be immediately fixed.

Best regards,
Jacek Anaszewski

On 07/17/2016 04:18 AM, Axel Lin wrote:
The LP3952_LED_ALL is known at compile time, so we can just store
leds[LP3952_LED_ALL] in struct lp3952_led_array rather than calling
devm_kcalloc() to allocate the memory.

Signed-off-by: Axel Lin <axel.lin@xxxxxxxxxx>
---
  drivers/leds/leds-lp3952.c  | 9 ---------
  include/linux/leds-lp3952.h | 2 +-
  2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/leds/leds-lp3952.c b/drivers/leds/leds-lp3952.c
index f6157c0..a73c8ff 100644
--- a/drivers/leds/leds-lp3952.c
+++ b/drivers/leds/leds-lp3952.c
@@ -215,21 +215,12 @@ static int lp3952_probe(struct i2c_client *client,
  			const struct i2c_device_id *id)
  {
  	int status;
-	struct lp3952_ctrl_hdl *leds;
  	struct lp3952_led_array *priv;

  	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
  	if (!priv)
  		return -ENOMEM;

-	leds = devm_kcalloc(&client->dev, LP3952_LED_ALL, sizeof(*leds),
-			    GFP_KERNEL);
-	if (!leds) {
-		devm_kfree(&client->dev, priv);
-		return -ENOMEM;
-	}
-
-	priv->leds = leds;
  	priv->client = client;

  	priv->enable_gpio = devm_gpiod_get(&client->dev, "nrst",
diff --git a/include/linux/leds-lp3952.h b/include/linux/leds-lp3952.h
index edd5ed6..49b37ed 100644
--- a/include/linux/leds-lp3952.h
+++ b/include/linux/leds-lp3952.h
@@ -119,7 +119,7 @@ struct lp3952_led_array {
  	struct regmap *regmap;
  	struct i2c_client *client;
  	struct gpio_desc *enable_gpio;
-	struct lp3952_ctrl_hdl *leds;
+	struct lp3952_ctrl_hdl leds[LP3952_LED_ALL];
  };

  #endif /* LEDS_LP3952_H_ */



--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux