v5:
- added and tested Andy' series which converts driver to use
device properties, dev_err_probe, devm_add_action_or_reset
and simple i2c probe_new:
https://lore.kernel.org/linux-leds/[email protected]/
v4:
- took Andy's comments into account
- add patch 7 for cleanup and formatting fixes
v3:
- pick up Rob's R-b for patches 1 and 2
- reinstate bindings docs license and maintainer
changes with Nikolaus agreement
- took Andy's comments on patch 4 into account
v2:
- keep original bindings license and maintainer/owner (Rob)
- squash bindings patches 2 & 4 (Krzysztof)
v1-resend:
- no change, resending after configuring git to accomodate
for smtp provider limit of 5 emails per batch
- just change cover-letter to mention si-en chip for idol347
The is31fl3190, is31fl3191 and is31fl3193 chips (1 or 3 PWM channels)
cannot be handled the same as is31fl3196 and is31fl3199,
if only because the register map is different.
Also:
- the software shutdown bit is reversed
- and additional field needs to be set to enable all channels
- the led-max-microamp current values and setting are not the same
Datasheets:
https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf
This series:
- converts dt-bindings to dtschema, adding all si-en compatibles
for convenience and consistency, and adding constraints on
supported values for eg. reg address and led-max-microamp
- changes vars, structs and defines to not use 319X suffix
but 3190 for 319{0,1,3} and 3196 for 319{6,9}
- adds fields in chipdef struct for chip-specific values
- only in patch 6, adds is31fl319{0,1,3} specific values
so those chips can work.
- patch 7 cleanups parts non touched in previous patches
- patches 8 to 12 are Andy's to modernize the driver
Tested on msm8916-alcatel-idol347, which probably has an
si-en,sn3190 or si-en,sn3191 (only one white led indicator).
Andy Shevchenko (5):
leds: is31fl319x: Make use of device properties
leds: is31fl319x: Make use of dev_err_probe()
leds: is31fl319x: Fix devm vs. non-devm ordering
leds: is31fl319x: use simple i2c probe function
leds: is31fl319x: sort header inclusion alphabetically
Vincent Knecht (7):
dt-bindings: leds: Convert is31fl319x to dtschema
dt-bindings: leds: is31fl319x: Document variants specificities
leds: is31fl319x: Add missing si-en compatibles
leds: is31fl319x: Use non-wildcard names for vars, structs and defines
leds: is31fl319x: Move chipset-specific values in chipdef struct
leds: is31fl319x: Add support for is31fl319{0,1,3} chips
leds: is31fl319x: Cleanup formatting and dev_dbg calls
.../bindings/leds/issi,is31fl319x.yaml | 193 +++++++
.../bindings/leds/leds-is31fl319x.txt | 61 --
drivers/leds/Kconfig | 2 +-
drivers/leds/leds-is31fl319x.c | 533 +++++++++++-------
4 files changed, 537 insertions(+), 252 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
delete mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl319x.txt
--
2.35.3
From: Andy Shevchenko <[email protected]>
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 <[email protected]>
Signed-off-by: Vincent Knecht <[email protected]>
---
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", ®);
+ ret = fwnode_property_read_u32(child, "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.3
Allow setting chips' specifics in chipdef struct by adding fields for:
- the reset register address
- a pointer to a regmap_config struct
- a pointer to a brightness_set function
- current default, min and max values
- a boolean to distinguish 319{0,1,3} and 319{6,9} chips
and use those fields in places where distinction has to be made.
The fields for 319{0,1,3} still point to 319{6,9} values.
No functional change.
Signed-off-by: Vincent Knecht <[email protected]>
---
v4:
- Fix non standard place for comments
---
drivers/leds/leds-is31fl319x.c | 208 ++++++++++++++++++++-------------
1 file changed, 125 insertions(+), 83 deletions(-)
diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 672b0e9e1d85..fe0cb68b583b 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -78,38 +78,58 @@ struct is31fl319x_chip {
struct is31fl319x_chipdef {
int num_leds;
+ u8 reset_reg;
+ const struct regmap_config *is31fl319x_regmap_config;
+ int (*brightness_set)(struct led_classdev *cdev, enum led_brightness brightness);
+ u32 current_default;
+ u32 current_min;
+ u32 current_max;
+ bool is_3196or3199;
};
-static const struct is31fl319x_chipdef is31fl3190_cdef = {
- .num_leds = 1,
-};
-
-static const struct is31fl319x_chipdef is31fl3193_cdef = {
- .num_leds = 3,
-};
+static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
+{
+ /* we have no readable registers */
+ return false;
+}
-static const struct is31fl319x_chipdef is31fl3196_cdef = {
- .num_leds = 6,
-};
+static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg)
+{
+ /* volatile registers are not cached */
+ switch (reg) {
+ case IS31FL3196_DATA_UPDATE:
+ case IS31FL3196_TIME_UPDATE:
+ case IS31FL3196_RESET:
+ return true; /* always write-through */
+ default:
+ return false;
+ }
+}
-static const struct is31fl319x_chipdef is31fl3199_cdef = {
- .num_leds = 9,
+static const struct reg_default is31fl3196_reg_defaults[] = {
+ { IS31FL3196_CONFIG1, 0x00 },
+ { IS31FL3196_CONFIG2, 0x00 },
+ { IS31FL3196_PWM(0), 0x00 },
+ { IS31FL3196_PWM(1), 0x00 },
+ { IS31FL3196_PWM(2), 0x00 },
+ { IS31FL3196_PWM(3), 0x00 },
+ { IS31FL3196_PWM(4), 0x00 },
+ { IS31FL3196_PWM(5), 0x00 },
+ { IS31FL3196_PWM(6), 0x00 },
+ { IS31FL3196_PWM(7), 0x00 },
+ { IS31FL3196_PWM(8), 0x00 },
};
-static const struct of_device_id of_is31fl319x_match[] = {
- { .compatible = "issi,is31fl3190", .data = &is31fl3190_cdef, },
- { .compatible = "issi,is31fl3191", .data = &is31fl3190_cdef, },
- { .compatible = "issi,is31fl3193", .data = &is31fl3193_cdef, },
- { .compatible = "issi,is31fl3196", .data = &is31fl3196_cdef, },
- { .compatible = "issi,is31fl3199", .data = &is31fl3199_cdef, },
- { .compatible = "si-en,sn3190", .data = &is31fl3190_cdef, },
- { .compatible = "si-en,sn3191", .data = &is31fl3190_cdef, },
- { .compatible = "si-en,sn3193", .data = &is31fl3193_cdef, },
- { .compatible = "si-en,sn3196", .data = &is31fl3196_cdef, },
- { .compatible = "si-en,sn3199", .data = &is31fl3199_cdef, },
- { }
+static struct regmap_config is31fl3196_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = IS31FL3196_REG_CNT,
+ .cache_type = REGCACHE_FLAT,
+ .readable_reg = is31fl319x_readable_reg,
+ .volatile_reg = is31fl3196_volatile_reg,
+ .reg_defaults = is31fl3196_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults),
};
-MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
static int is31fl3196_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
@@ -175,9 +195,69 @@ static int is31fl3196_brightness_set(struct led_classdev *cdev,
return ret;
}
+static const struct is31fl319x_chipdef is31fl3190_cdef = {
+ .num_leds = 1,
+ .reset_reg = IS31FL3196_RESET,
+ .is31fl319x_regmap_config = &is31fl3196_regmap_config,
+ .brightness_set = is31fl3196_brightness_set,
+ .current_default = IS31FL3196_CURRENT_uA_DEFAULT,
+ .current_min = IS31FL3196_CURRENT_uA_MIN,
+ .current_max = IS31FL3196_CURRENT_uA_MAX,
+ .is_3196or3199 = true,
+};
+
+static const struct is31fl319x_chipdef is31fl3193_cdef = {
+ .num_leds = 3,
+ .reset_reg = IS31FL3196_RESET,
+ .is31fl319x_regmap_config = &is31fl3196_regmap_config,
+ .brightness_set = is31fl3196_brightness_set,
+ .current_default = IS31FL3196_CURRENT_uA_DEFAULT,
+ .current_min = IS31FL3196_CURRENT_uA_MIN,
+ .current_max = IS31FL3196_CURRENT_uA_MAX,
+ .is_3196or3199 = true,
+};
+
+static const struct is31fl319x_chipdef is31fl3196_cdef = {
+ .num_leds = 6,
+ .reset_reg = IS31FL3196_RESET,
+ .is31fl319x_regmap_config = &is31fl3196_regmap_config,
+ .brightness_set = is31fl3196_brightness_set,
+ .current_default = IS31FL3196_CURRENT_uA_DEFAULT,
+ .current_min = IS31FL3196_CURRENT_uA_MIN,
+ .current_max = IS31FL3196_CURRENT_uA_MAX,
+ .is_3196or3199 = true,
+};
+
+static const struct is31fl319x_chipdef is31fl3199_cdef = {
+ .num_leds = 9,
+ .reset_reg = IS31FL3196_RESET,
+ .is31fl319x_regmap_config = &is31fl3196_regmap_config,
+ .brightness_set = is31fl3196_brightness_set,
+ .current_default = IS31FL3196_CURRENT_uA_DEFAULT,
+ .current_min = IS31FL3196_CURRENT_uA_MIN,
+ .current_max = IS31FL3196_CURRENT_uA_MAX,
+ .is_3196or3199 = true,
+};
+
+static const struct of_device_id of_is31fl319x_match[] = {
+ { .compatible = "issi,is31fl3190", .data = &is31fl3190_cdef, },
+ { .compatible = "issi,is31fl3191", .data = &is31fl3190_cdef, },
+ { .compatible = "issi,is31fl3193", .data = &is31fl3193_cdef, },
+ { .compatible = "issi,is31fl3196", .data = &is31fl3196_cdef, },
+ { .compatible = "issi,is31fl3199", .data = &is31fl3199_cdef, },
+ { .compatible = "si-en,sn3190", .data = &is31fl3190_cdef, },
+ { .compatible = "si-en,sn3191", .data = &is31fl3190_cdef, },
+ { .compatible = "si-en,sn3193", .data = &is31fl3193_cdef, },
+ { .compatible = "si-en,sn3196", .data = &is31fl3196_cdef, },
+ { .compatible = "si-en,sn3199", .data = &is31fl3199_cdef, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
+
static int is31fl319x_parse_child_dt(const struct device *dev,
const struct device_node *child,
- struct is31fl319x_led *led)
+ struct is31fl319x_led *led,
+ struct is31fl319x_chip *is31)
{
struct led_classdev *cdev = &led->cdev;
int ret;
@@ -190,14 +270,14 @@ static int is31fl319x_parse_child_dt(const struct device *dev,
if (ret < 0 && ret != -EINVAL) /* is optional */
return ret;
- led->max_microamp = IS31FL3196_CURRENT_uA_DEFAULT;
+ led->max_microamp = is31->cdef->current_default;
ret = of_property_read_u32(child, "led-max-microamp",
&led->max_microamp);
if (!ret) {
- if (led->max_microamp < IS31FL3196_CURRENT_uA_MIN)
+ if (led->max_microamp < is31->cdef->current_min)
return -EINVAL; /* not supported */
led->max_microamp = min(led->max_microamp,
- IS31FL3196_CURRENT_uA_MAX);
+ is31->cdef->current_max);
}
return 0;
@@ -258,7 +338,7 @@ static int is31fl319x_parse_dt(struct device *dev,
goto put_child_node;
}
- ret = is31fl319x_parse_child_dt(dev, child, led);
+ ret = is31fl319x_parse_child_dt(dev, child, led, is31);
if (ret) {
dev_err(dev, "led %u DT parsing failed\n", reg);
goto put_child_node;
@@ -268,10 +348,12 @@ static int is31fl319x_parse_dt(struct device *dev,
}
is31->audio_gain_db = 0;
- ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db);
- if (!ret)
- is31->audio_gain_db = min(is31->audio_gain_db,
- IS31FL3196_AUDIO_GAIN_DB_MAX);
+ if (is31->cdef->is_3196or3199) {
+ ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db);
+ if (!ret)
+ is31->audio_gain_db = min(is31->audio_gain_db,
+ IS31FL3196_AUDIO_GAIN_DB_MAX);
+ }
return 0;
@@ -280,48 +362,6 @@ static int is31fl319x_parse_dt(struct device *dev,
return ret;
}
-static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
-{ /* we have no readable registers */
- return false;
-}
-
-static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg)
-{ /* volatile registers are not cached */
- switch (reg) {
- case IS31FL3196_DATA_UPDATE:
- case IS31FL3196_TIME_UPDATE:
- case IS31FL3196_RESET:
- return true; /* always write-through */
- default:
- return false;
- }
-}
-
-static const struct reg_default is31fl3196_reg_defaults[] = {
- { IS31FL3196_CONFIG1, 0x00 },
- { IS31FL3196_CONFIG2, 0x00 },
- { IS31FL3196_PWM(0), 0x00 },
- { IS31FL3196_PWM(1), 0x00 },
- { IS31FL3196_PWM(2), 0x00 },
- { IS31FL3196_PWM(3), 0x00 },
- { IS31FL3196_PWM(4), 0x00 },
- { IS31FL3196_PWM(5), 0x00 },
- { IS31FL3196_PWM(6), 0x00 },
- { IS31FL3196_PWM(7), 0x00 },
- { IS31FL3196_PWM(8), 0x00 },
-};
-
-static struct regmap_config is31fl3196_regmap_config = {
- .reg_bits = 8,
- .val_bits = 8,
- .max_register = IS31FL3196_REG_CNT,
- .cache_type = REGCACHE_FLAT,
- .readable_reg = is31fl319x_readable_reg,
- .volatile_reg = is31fl3196_volatile_reg,
- .reg_defaults = is31fl3196_reg_defaults,
- .num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults),
-};
-
static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp)
{ /* round down to nearest supported value (range check done by caller) */
u32 step = microamp / IS31FL3196_CURRENT_uA_STEP;
@@ -343,7 +383,7 @@ static int is31fl319x_probe(struct i2c_client *client,
struct device *dev = &client->dev;
int err;
int i = 0;
- u32 aggregated_led_microamp = IS31FL3196_CURRENT_uA_MAX;
+ u32 aggregated_led_microamp;
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -EIO;
@@ -365,7 +405,7 @@ static int is31fl319x_probe(struct i2c_client *client,
}
is31->client = client;
- is31->regmap = devm_regmap_init_i2c(client, &is31fl3196_regmap_config);
+ 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);
@@ -375,7 +415,7 @@ static int is31fl319x_probe(struct i2c_client *client,
i2c_set_clientdata(client, is31);
/* check for write-reply from chip (we can't read any registers) */
- err = regmap_write(is31->regmap, IS31FL3196_RESET, 0x00);
+ 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);
@@ -388,14 +428,16 @@ static int is31fl319x_probe(struct i2c_client *client,
* But the chip does not allow to limit individual LEDs.
* So we take minimum from all subnodes for safety of hardware.
*/
+ aggregated_led_microamp = is31->cdef->current_max;
for (i = 0; i < is31->cdef->num_leds; i++)
if (is31->leds[i].configured &&
is31->leds[i].max_microamp < aggregated_led_microamp)
aggregated_led_microamp = is31->leds[i].max_microamp;
- regmap_write(is31->regmap, IS31FL3196_CONFIG2,
- is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) |
- is31fl3196_db_to_gain(is31->audio_gain_db));
+ if (is31->cdef->is_3196or3199)
+ regmap_write(is31->regmap, IS31FL3196_CONFIG2,
+ is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) |
+ is31fl3196_db_to_gain(is31->audio_gain_db));
for (i = 0; i < is31->cdef->num_leds; i++) {
struct is31fl319x_led *led = &is31->leds[i];
@@ -404,7 +446,7 @@ static int is31fl319x_probe(struct i2c_client *client,
continue;
led->chip = is31;
- led->cdev.brightness_set_blocking = is31fl3196_brightness_set;
+ led->cdev.brightness_set_blocking = is31->cdef->brightness_set;
err = devm_led_classdev_register(&client->dev, &led->cdev);
if (err < 0)
--
2.35.3
Set specific chipset structs values for is31fl319{0,1,3}
so that those chips can actually work.
Datasheets:
https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf
Signed-off-by: Vincent Knecht <[email protected]>
---
v4:
- keep container_of() call on one line
- remove dev_dbg() call after regmap_read()
- change is31fl3190_microamp_to_cs() to return directly
from the default case
- fix non-standard place for comment
- add missing space before }
---
drivers/leds/leds-is31fl319x.c | 161 ++++++++++++++++++++++++++++++---
1 file changed, 147 insertions(+), 14 deletions(-)
diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index fe0cb68b583b..2314922bc009 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -21,6 +21,31 @@
/* register numbers */
#define IS31FL319X_SHUTDOWN 0x00
+
+/* registers for 3190, 3191 and 3193 */
+#define IS31FL3190_BREATHING 0x01
+#define IS31FL3190_LEDMODE 0x02
+#define IS31FL3190_CURRENT 0x03
+#define IS31FL3190_PWM(channel) (0x04 + channel)
+#define IS31FL3190_DATA_UPDATE 0x07
+#define IS31FL3190_T0(channel) (0x0a + channel)
+#define IS31FL3190_T1T2(channel) (0x10 + channel)
+#define IS31FL3190_T3T4(channel) (0x16 + channel)
+#define IS31FL3190_TIME_UPDATE 0x1c
+#define IS31FL3190_LEDCONTROL 0x1d
+#define IS31FL3190_RESET 0x2f
+
+#define IS31FL3190_CURRENT_uA_MIN 5000
+#define IS31FL3190_CURRENT_uA_DEFAULT 42000
+#define IS31FL3190_CURRENT_uA_MAX 42000
+#define IS31FL3190_CURRENT_MASK GENMASK(4, 2)
+#define IS31FL3190_CURRENT_5_mA 0x02
+#define IS31FL3190_CURRENT_10_mA 0x01
+#define IS31FL3190_CURRENT_17dot5_mA 0x04
+#define IS31FL3190_CURRENT_30_mA 0x03
+#define IS31FL3190_CURRENT_42_mA 0x00
+
+/* registers for 3196 and 3199 */
#define IS31FL3196_CTRL1 0x01
#define IS31FL3196_CTRL2 0x02
#define IS31FL3196_CONFIG1 0x03
@@ -93,6 +118,38 @@ static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
return false;
}
+static bool is31fl3190_volatile_reg(struct device *dev, unsigned int reg)
+{
+ /* volatile registers are not cached */
+ switch (reg) {
+ case IS31FL3190_DATA_UPDATE:
+ case IS31FL3190_TIME_UPDATE:
+ case IS31FL3190_RESET:
+ return true; /* always write-through */
+ default:
+ return false;
+ }
+}
+
+static const struct reg_default is31fl3190_reg_defaults[] = {
+ { IS31FL3190_LEDMODE, 0x00 },
+ { IS31FL3190_CURRENT, 0x00 },
+ { IS31FL3190_PWM(0), 0x00 },
+ { IS31FL3190_PWM(1), 0x00 },
+ { IS31FL3190_PWM(2), 0x00 },
+};
+
+static struct regmap_config is31fl3190_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = IS31FL3190_RESET,
+ .cache_type = REGCACHE_FLAT,
+ .readable_reg = is31fl319x_readable_reg,
+ .volatile_reg = is31fl3190_volatile_reg,
+ .reg_defaults = is31fl3190_reg_defaults,
+ .num_reg_defaults = ARRAY_SIZE(is31fl3190_reg_defaults),
+};
+
static bool is31fl3196_volatile_reg(struct device *dev, unsigned int reg)
{
/* volatile registers are not cached */
@@ -131,6 +188,60 @@ static struct regmap_config is31fl3196_regmap_config = {
.num_reg_defaults = ARRAY_SIZE(is31fl3196_reg_defaults),
};
+static int is31fl3190_brightness_set(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct is31fl319x_led *led = container_of(cdev, struct is31fl319x_led, cdev);
+ struct is31fl319x_chip *is31 = led->chip;
+ int chan = led - is31->leds;
+ int ret;
+ int i;
+ u8 ctrl = 0;
+
+ dev_dbg(&is31->client->dev, "%s %d: %d\n", __func__, chan, brightness);
+
+ mutex_lock(&is31->lock);
+
+ /* update PWM register */
+ ret = regmap_write(is31->regmap, IS31FL3190_PWM(chan), brightness);
+ if (ret < 0)
+ goto out;
+
+ /* read current brightness of all PWM channels */
+ for (i = 0; i < is31->cdef->num_leds; i++) {
+ unsigned int pwm_value;
+ bool on;
+
+ /*
+ * since neither cdev nor the chip can provide
+ * the current setting, we read from the regmap cache
+ */
+
+ ret = regmap_read(is31->regmap, IS31FL3190_PWM(i), &pwm_value);
+ on = ret >= 0 && pwm_value > LED_OFF;
+
+ ctrl |= on << i;
+ }
+
+ if (ctrl > 0) {
+ dev_dbg(&is31->client->dev, "power up %02x\n", ctrl);
+ regmap_write(is31->regmap, IS31FL3190_LEDCONTROL, ctrl);
+ /* update PWMs */
+ regmap_write(is31->regmap, IS31FL3190_DATA_UPDATE, 0x00);
+ /* enable chip from shut down and enable all channels */
+ ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x20);
+ } else {
+ dev_dbg(&is31->client->dev, "power down\n");
+ /* shut down (no need to clear LEDCONTROL) */
+ ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x01);
+ }
+
+out:
+ mutex_unlock(&is31->lock);
+
+ return ret;
+}
+
static int is31fl3196_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
@@ -197,24 +308,24 @@ static int is31fl3196_brightness_set(struct led_classdev *cdev,
static const struct is31fl319x_chipdef is31fl3190_cdef = {
.num_leds = 1,
- .reset_reg = IS31FL3196_RESET,
- .is31fl319x_regmap_config = &is31fl3196_regmap_config,
- .brightness_set = is31fl3196_brightness_set,
- .current_default = IS31FL3196_CURRENT_uA_DEFAULT,
- .current_min = IS31FL3196_CURRENT_uA_MIN,
- .current_max = IS31FL3196_CURRENT_uA_MAX,
- .is_3196or3199 = true,
+ .reset_reg = IS31FL3190_RESET,
+ .is31fl319x_regmap_config = &is31fl3190_regmap_config,
+ .brightness_set = is31fl3190_brightness_set,
+ .current_default = IS31FL3190_CURRENT_uA_DEFAULT,
+ .current_min = IS31FL3190_CURRENT_uA_MIN,
+ .current_max = IS31FL3190_CURRENT_uA_MAX,
+ .is_3196or3199 = false,
};
static const struct is31fl319x_chipdef is31fl3193_cdef = {
.num_leds = 3,
- .reset_reg = IS31FL3196_RESET,
- .is31fl319x_regmap_config = &is31fl3196_regmap_config,
- .brightness_set = is31fl3196_brightness_set,
- .current_default = IS31FL3196_CURRENT_uA_DEFAULT,
- .current_min = IS31FL3196_CURRENT_uA_MIN,
- .current_max = IS31FL3196_CURRENT_uA_MAX,
- .is_3196or3199 = true,
+ .reset_reg = IS31FL3190_RESET,
+ .is31fl319x_regmap_config = &is31fl3190_regmap_config,
+ .brightness_set = is31fl3190_brightness_set,
+ .current_default = IS31FL3190_CURRENT_uA_DEFAULT,
+ .current_min = IS31FL3190_CURRENT_uA_MIN,
+ .current_max = IS31FL3190_CURRENT_uA_MAX,
+ .is_3196or3199 = false,
};
static const struct is31fl319x_chipdef is31fl3196_cdef = {
@@ -362,6 +473,25 @@ static int is31fl319x_parse_dt(struct device *dev,
return ret;
}
+static inline int is31fl3190_microamp_to_cs(struct device *dev, u32 microamp)
+{
+ switch (microamp) {
+ case 5000:
+ return IS31FL3190_CURRENT_5_mA;
+ case 10000:
+ return IS31FL3190_CURRENT_10_mA;
+ case 17500:
+ return IS31FL3190_CURRENT_17dot5_mA;
+ case 30000:
+ return IS31FL3190_CURRENT_30_mA;
+ case 42000:
+ return IS31FL3190_CURRENT_42_mA;
+ default:
+ dev_warn(dev, "Unsupported current value: %d, using 5000 µA!\n", microamp);
+ return IS31FL3190_CURRENT_5_mA;
+ }
+}
+
static inline int is31fl3196_microamp_to_cs(struct device *dev, u32 microamp)
{ /* round down to nearest supported value (range check done by caller) */
u32 step = microamp / IS31FL3196_CURRENT_uA_STEP;
@@ -438,6 +568,9 @@ static int is31fl319x_probe(struct i2c_client *client,
regmap_write(is31->regmap, IS31FL3196_CONFIG2,
is31fl3196_microamp_to_cs(dev, aggregated_led_microamp) |
is31fl3196_db_to_gain(is31->audio_gain_db));
+ else
+ regmap_update_bits(is31->regmap, IS31FL3190_CURRENT, IS31FL3190_CURRENT_MASK,
+ is31fl3190_microamp_to_cs(dev, aggregated_led_microamp));
for (i = 0; i < is31->cdef->num_leds; i++) {
struct is31fl319x_led *led = &is31->leds[i];
--
2.35.3
From: Andy Shevchenko <[email protected]>
Simplify the error handling in probe function by switching from
dev_err() to dev_err_probe().
Signed-off-by: Andy Shevchenko <[email protected]>
Signed-off-by: Vincent Knecht <[email protected]>
---
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", ®);
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.3
Add si-en compatibles for all chip variants and add conditionals
depending on compatibles to document variants specs:
- possible reg addresses
- whether audio-gain-db is supported or not
- maximum number of leds
- led-max-microamp values
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Vincent Knecht <[email protected]>
---
.../bindings/leds/issi,is31fl319x.yaml | 84 ++++++++++++++++++-
1 file changed, 82 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml b/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
index 0d684aeeb8cd..940333f2d69c 100644
--- a/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
+++ b/Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
@@ -28,6 +28,10 @@ properties:
- issi,is31fl3193
- issi,is31fl3196
- issi,is31fl3199
+ - si-en,sn3190
+ - si-en,sn3191
+ - si-en,sn3193
+ - si-en,sn3196
- si-en,sn3199
reg:
@@ -61,8 +65,6 @@ patternProperties:
maximum: 9
led-max-microamp:
- default: 20000
- enum: [5000, 10000, 15000, 20000, 25000, 30000, 35000, 40000]
description:
Note that a driver will take the lowest of all LED limits
since the chip has a single global setting. The lowest value
@@ -71,6 +73,84 @@ patternProperties:
and not the current, which will always have its peak value
equal to led-max-microamp.
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3190
+ - issi,is31fl3191
+ - issi,is31fl3193
+ - si-en,sn3190
+ - si-en,sn3191
+ - si-en,sn3193
+ then:
+ properties:
+ reg:
+ enum: [0x68, 0x69, 0x6a, 0x6b]
+
+ audio-gain-db: false
+
+ patternProperties:
+ "^led@[1-9]$":
+ properties:
+ led-max-microamp:
+ default: 42000
+ enum: [5000, 10000, 17500, 30000, 42000]
+ else:
+ properties:
+ reg:
+ enum: [0x64, 0x65, 0x66, 0x67]
+
+ patternProperties:
+ "^led@[1-9]$":
+ properties:
+ led-max-microamp:
+ default: 20000
+ enum: [5000, 10000, 15000, 20000, 25000, 30000, 35000, 40000]
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3190
+ - issi,is31fl3191
+ - si-en,sn3190
+ - si-en,sn3191
+ then:
+ patternProperties:
+ "^led@[1-9]$":
+ properties:
+ reg:
+ maximum: 1
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3193
+ - si-en,sn3193
+ then:
+ patternProperties:
+ "^led@[1-9]$":
+ properties:
+ reg:
+ maximum: 3
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - issi,is31fl3196
+ - si-en,sn3196
+ then:
+ patternProperties:
+ "^led@[1-9]$":
+ properties:
+ reg:
+ maximum: 6
+
required:
- compatible
- reg
--
2.35.3
From: Andy Shevchenko <[email protected]>
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 <[email protected]>
Signed-off-by: Vincent Knecht <[email protected]>
---
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.3
On Tue, 12 Jul 2022 12:08:28 +0200, Vincent Knecht wrote:
> Add si-en compatibles for all chip variants and add conditionals
> depending on compatibles to document variants specs:
> - possible reg addresses
> - whether audio-gain-db is supported or not
> - maximum number of leds
> - led-max-microamp values
>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Vincent Knecht <[email protected]>
> ---
> .../bindings/leds/issi,is31fl319x.yaml | 84 ++++++++++++++++++-
> 1 file changed, 82 insertions(+), 2 deletions(-)
>
Applied, thanks!