[PATCH v1 1/5] leds: is31fl319x: Make use of device properties

[PATCH v1 1/5] leds: is31fl319x: Make use of device properties

[Andy Shevchenko]

Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.

Add mod_devicetable.h include.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/Kconfig           |  2 +-
 drivers/leds/leds-is31fl319x.c | 44 ++++++++++++++++------------------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a49979f41eee..9a896be26dc7 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -697,7 +697,7 @@ config LEDS_MENF21BMC
 
 config LEDS_IS31FL319X
 	tristate "LED Support for ISSI IS31FL319x I2C LED controller family"
-	depends on LEDS_CLASS && I2C && OF
+	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
 	  This option enables support for LEDs connected to ISSI IS31FL319x
diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 056cf3bcffdc..b1a0ca340e24 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -11,9 +11,9 @@
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/leds.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
@@ -362,25 +362,23 @@ static const struct of_device_id of_is31fl319x_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
 
-static int is31fl319x_parse_child_dt(const struct device *dev,
-				     const struct device_node *child,
+static int is31fl319x_parse_child_fw(const struct device *dev,
+				     const struct fwnode_handle *child,
 				     struct is31fl319x_led *led,
 				     struct is31fl319x_chip *is31)
 {
 	struct led_classdev *cdev = &led->cdev;
 	int ret;
 
-	if (of_property_read_string(child, "label", &cdev->name))
-		cdev->name = child->name;
+	if (fwnode_property_read_string(child, "label", &cdev->name))
+		cdev->name = fwnode_get_name(child);
 
-	ret = of_property_read_string(child, "linux,default-trigger",
-				      &cdev->default_trigger);
+	ret = fwnode_property_read_string(child, "linux,default-trigger", &cdev->default_trigger);
 	if (ret < 0 && ret != -EINVAL) /* is optional */
 		return ret;
 
 	led->max_microamp = is31->cdef->current_default;
-	ret = of_property_read_u32(child, "led-max-microamp",
-				   &led->max_microamp);
+	ret = fwnode_property_read_u32(child, "led-max-microamp", &led->max_microamp);
 	if (!ret) {
 		if (led->max_microamp < is31->cdef->current_min)
 			return -EINVAL;	/* not supported */
@@ -391,16 +389,12 @@ static int is31fl319x_parse_child_dt(const struct device *dev,
 	return 0;
 }
 
-static int is31fl319x_parse_dt(struct device *dev,
-			       struct is31fl319x_chip *is31)
+static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 {
-	struct device_node *np = dev_of_node(dev), *child;
+	struct fwnode_handle *fwnode = dev_fwnode(dev), *child;
 	int count;
 	int ret;
 
-	if (!np)
-		return -ENODEV;
-
 	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
 	if (IS_ERR(is31->shutdown_gpio)) {
 		ret = PTR_ERR(is31->shutdown_gpio);
@@ -410,7 +404,9 @@ static int is31fl319x_parse_dt(struct device *dev,
 
 	is31->cdef = device_get_match_data(dev);
 
-	count = of_get_available_child_count(np);
+	count = 0;
+	fwnode_for_each_available_child_node(fwnode, child)
+		count++;
 
 	dev_dbg(dev, "probing with %d leds defined in DT\n", count);
 
@@ -420,11 +416,11 @@ static int is31fl319x_parse_dt(struct device *dev,
 		return -ENODEV;
 	}
 
-	for_each_available_child_of_node(np, child) {
+	fwnode_for_each_available_child_node(fwnode, child) {
 		struct is31fl319x_led *led;
 		u32 reg;
 
-		ret = of_property_read_u32(child, "reg", &reg);
+		ret = fwnode_property_read_u32(child, "reg", &reg);
 		if (ret) {
 			dev_err(dev, "Failed to read led 'reg' property\n");
 			goto put_child_node;
@@ -444,7 +440,7 @@ static int is31fl319x_parse_dt(struct device *dev,
 			goto put_child_node;
 		}
 
-		ret = is31fl319x_parse_child_dt(dev, child, led, is31);
+		ret = is31fl319x_parse_child_fw(dev, child, led, is31);
 		if (ret) {
 			dev_err(dev, "led %u DT parsing failed\n", reg);
 			goto put_child_node;
@@ -455,7 +451,7 @@ static int is31fl319x_parse_dt(struct device *dev,
 
 	is31->audio_gain_db = 0;
 	if (is31->cdef->is_3196or3199) {
-		ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db);
+		ret = fwnode_property_read_u32(fwnode, "audio-gain-db", &is31->audio_gain_db);
 		if (!ret)
 			is31->audio_gain_db = min(is31->audio_gain_db,
 						  IS31FL3196_AUDIO_GAIN_DB_MAX);
@@ -464,7 +460,7 @@ static int is31fl319x_parse_dt(struct device *dev,
 	return 0;
 
 put_child_node:
-	of_node_put(child);
+	fwnode_handle_put(child);
 	return ret;
 }
 
@@ -521,7 +517,7 @@ static int is31fl319x_probe(struct i2c_client *client,
 
 	mutex_init(&is31->lock);
 
-	err = is31fl319x_parse_dt(&client->dev, is31);
+	err = is31fl319x_parse_fw(&client->dev, is31);
 	if (err)
 		goto free_mutex;
 
@@ -619,7 +615,7 @@ MODULE_DEVICE_TABLE(i2c, is31fl319x_id);
 static struct i2c_driver is31fl319x_driver = {
 	.driver   = {
 		.name           = "leds-is31fl319x",
-		.of_match_table = of_match_ptr(of_is31fl319x_match),
+		.of_match_table = of_is31fl319x_match,
 	},
 	.probe    = is31fl319x_probe,
 	.remove   = is31fl319x_remove,
-- 
2.35.1

[PATCH v1 2/5] leds: is31fl319x: Make use of dev_err_probe()

[Andy Shevchenko]

Simplify the error handling in probe function by switching from
dev_err() to dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/leds-is31fl319x.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index b1a0ca340e24..d9cf70505c43 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -396,11 +396,9 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 	int ret;
 
 	is31->shutdown_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
-	if (IS_ERR(is31->shutdown_gpio)) {
-		ret = PTR_ERR(is31->shutdown_gpio);
-		dev_err(dev, "Failed to get shutdown gpio: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(is31->shutdown_gpio))
+		return dev_err_probe(dev, PTR_ERR(is31->shutdown_gpio),
+				     "Failed to get shutdown gpio\n");
 
 	is31->cdef = device_get_match_data(dev);
 
@@ -410,11 +408,10 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 
 	dev_dbg(dev, "probing with %d leds defined in DT\n", count);
 
-	if (!count || count > is31->cdef->num_leds) {
-		dev_err(dev, "Number of leds defined must be between 1 and %u\n",
-			is31->cdef->num_leds);
-		return -ENODEV;
-	}
+	if (!count || count > is31->cdef->num_leds)
+		return dev_err_probe(dev, -ENODEV,
+				     "Number of leds defined must be between 1 and %u\n",
+				     is31->cdef->num_leds);
 
 	fwnode_for_each_available_child_node(fwnode, child) {
 		struct is31fl319x_led *led;
@@ -422,27 +419,25 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 
 		ret = fwnode_property_read_u32(child, "reg", &reg);
 		if (ret) {
-			dev_err(dev, "Failed to read led 'reg' property\n");
+			ret = dev_err_probe(dev, ret, "Failed to read led 'reg' property\n");
 			goto put_child_node;
 		}
 
 		if (reg < 1 || reg > is31->cdef->num_leds) {
-			dev_err(dev, "invalid led reg %u\n", reg);
-			ret = -EINVAL;
+			ret = dev_err_probe(dev, -EINVAL, "invalid led reg %u\n", reg);
 			goto put_child_node;
 		}
 
 		led = &is31->leds[reg - 1];
 
 		if (led->configured) {
-			dev_err(dev, "led %u is already configured\n", reg);
-			ret = -EINVAL;
+			ret = dev_err_probe(dev, -EINVAL, "led %u is already configured\n", reg);
 			goto put_child_node;
 		}
 
 		ret = is31fl319x_parse_child_fw(dev, child, led, is31);
 		if (ret) {
-			dev_err(dev, "led %u DT parsing failed\n", reg);
+			ret = dev_err_probe(dev, ret, "led %u DT parsing failed\n", reg);
 			goto put_child_node;
 		}
 
@@ -530,8 +525,7 @@ static int is31fl319x_probe(struct i2c_client *client,
 	is31->client = client;
 	is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config);
 	if (IS_ERR(is31->regmap)) {
-		dev_err(&client->dev, "failed to allocate register map\n");
-		err = PTR_ERR(is31->regmap);
+		err = dev_err_probe(dev, PTR_ERR(is31->regmap), "failed to allocate register map\n");
 		goto free_mutex;
 	}
 
@@ -540,8 +534,7 @@ static int is31fl319x_probe(struct i2c_client *client,
 	/* check for write-reply from chip (we can't read any registers) */
 	err = regmap_write(is31->regmap, is31->cdef->reset_reg, 0x00);
 	if (err < 0) {
-		dev_err(&client->dev, "no response from chip write: err = %d\n", err);
-		err = -EIO; /* does not answer */
+		err = dev_err_probe(dev, err, "no response from chip write\n");
 		goto free_mutex;
 	}
 
-- 
2.35.1

[PATCH v1 3/5] leds: is31fl319x: Fix devm vs. non-devm ordering

[Andy Shevchenko]

When non-devm resources are allocated they mustn't be followed by
devm allocations, otherwise it will break the tear down ordering
and might lead to crashes or other bugs during ->remove() stage.
Fix this by wrapping mutex_destroy() call with
devm_add_action_or_reset().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/leds-is31fl319x.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index d9cf70505c43..233ed67830a3 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -511,10 +511,13 @@ static int is31fl319x_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	mutex_init(&is31->lock);
+	err = devm_add_action(dev, (void (*)(void *))mutex_destroy, &is31->lock);
+	if (err)
+		return err;
 
 	err = is31fl319x_parse_fw(&client->dev, is31);
 	if (err)
-		goto free_mutex;
+		return err;
 
 	if (is31->shutdown_gpio) {
 		gpiod_direction_output(is31->shutdown_gpio, 0);
@@ -524,19 +527,15 @@ static int is31fl319x_probe(struct i2c_client *client,
 
 	is31->client = client;
 	is31->regmap = devm_regmap_init_i2c(client, is31->cdef->is31fl319x_regmap_config);
-	if (IS_ERR(is31->regmap)) {
-		err = dev_err_probe(dev, PTR_ERR(is31->regmap), "failed to allocate register map\n");
-		goto free_mutex;
-	}
+	if (IS_ERR(is31->regmap))
+		return dev_err_probe(dev, PTR_ERR(is31->regmap), "failed to allocate register map\n");
 
 	i2c_set_clientdata(client, is31);
 
 	/* check for write-reply from chip (we can't read any registers) */
 	err = regmap_write(is31->regmap, is31->cdef->reset_reg, 0x00);
-	if (err < 0) {
-		err = dev_err_probe(dev, err, "no response from chip write\n");
-		goto free_mutex;
-	}
+	if (err < 0)
+		return dev_err_probe(dev, err, "no response from chip write\n");
 
 	/*
 	 * Kernel conventions require per-LED led-max-microamp property.
@@ -568,22 +567,10 @@ static int is31fl319x_probe(struct i2c_client *client,
 
 		err = devm_led_classdev_register(&client->dev, &led->cdev);
 		if (err < 0)
-			goto free_mutex;
+			return err;
 	}
 
 	return 0;
-
-free_mutex:
-	mutex_destroy(&is31->lock);
-	return err;
-}
-
-static int is31fl319x_remove(struct i2c_client *client)
-{
-	struct is31fl319x_chip *is31 = i2c_get_clientdata(client);
-
-	mutex_destroy(&is31->lock);
-	return 0;
 }
 
 /*
@@ -611,7 +598,6 @@ static struct i2c_driver is31fl319x_driver = {
 		.of_match_table = of_is31fl319x_match,
 	},
 	.probe    = is31fl319x_probe,
-	.remove   = is31fl319x_remove,
 	.id_table = is31fl319x_id,
 };
 
-- 
2.35.1

[PATCH v1 4/5] leds: is31fl319x: use simple i2c probe function

[Andy Shevchenko]

The i2c probe functions here don't use the id information provided in
their second argument, so the single-parameter i2c probe function
("probe_new") can be used instead.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/leds-is31fl319x.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 233ed67830a3..52b59b62f437 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -494,8 +494,7 @@ static inline int is31fl3196_db_to_gain(u32 dezibel)
 	return dezibel / IS31FL3196_AUDIO_GAIN_DB_STEP;
 }
 
-static int is31fl319x_probe(struct i2c_client *client,
-			    const struct i2c_device_id *id)
+static int is31fl319x_probe(struct i2c_client *client)
 {
 	struct is31fl319x_chip *is31;
 	struct device *dev = &client->dev;
@@ -597,7 +596,7 @@ static struct i2c_driver is31fl319x_driver = {
 		.name           = "leds-is31fl319x",
 		.of_match_table = of_is31fl319x_match,
 	},
-	.probe    = is31fl319x_probe,
+	.probe_new = is31fl319x_probe,
 	.id_table = is31fl319x_id,
 };
 
-- 
2.35.1

[PATCH v1 5/5] leds: is31fl319x: sort header inclusion alphabetically

[Andy Shevchenko]

Sort header inclusion alphabetically.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/leds-is31fl319x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 52b59b62f437..f2d05ae91753 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -8,7 +8,9 @@
  * effect LEDs.
  */
 
+#include <linux/delay.h>
 #include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/leds.h>
 #include <linux/mod_devicetable.h>
@@ -16,8 +18,6 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
-#include <linux/delay.h>
-#include <linux/gpio/consumer.h>
 
 /* register numbers */
 #define IS31FL319X_SHUTDOWN		0x00
-- 
2.35.1