[PATCH v6 0/2] driver: leds: is31fl319x dimmable LED driver

[PATCH v6 0/2] driver: leds: is31fl319x dimmable LED driver

[H. Nikolaus Schaller]

Changes V6 suggested by Jacek Anaszewski:
* indentation/tab of macro values
* use inline for some helper functions
* define constant 12 (CS reference point) as semantic macro

2016-07-19 11:40:07: Changes V5 suggested by Jacek Anaszewski:
* update macros/function for audio gain
* mention PWM in bindings

2016-07-19 10:03:40: Changes V4 (mostly fixed by Andrey Utkin):
* Fix struct is31fl319x_driver alignment
* Rework chip matching as suggested by Jacek Anaszewski
* Enhance macro-constants names
* Traverse cdef.num_leds, not MAX_LEDS
* Fix some checkpatch --strict notices
* Fixup for MAX_LEDS -> cdef->num_leds
* Fix lots of review notices
    - macrodef for bits shift
     - drop dev_err() on parse_dt() fail (thus inside parse_dt() every
       failure must give a proper message)
     - of_node_put() where needed
     - rework is31fl319x_microamp_to_cs()
     - destroy mutex properly
* remove note that it is based on tca6507 driver (has deviated completely) (fixed by hns)
* reject if any led current < current_min (fixed by hns)
* simplify helper function is31fl319x_microamp_to_cs and round down (fixed by hns)
* Drop extra empty line in Documentation/devicetree/bindings/leds/is31fl319x.txt
* remove led-max-microamps global property in bindings documentation (fixed by hns)

2016-07-08 21:49:42: Changes V3 (mostly done by Andrey Utkin):
* general coding style improvements
* added a mutex to properly serialize multiple calls to set_backlight
* use regmap defaults (suggested by Jacek Anaszewski)
* improve DT parsing (suggested by Jacek Anaszewski)
* define dependency on REGMAP_I2C
* minor code improvements

2016-07-06 12:02:47: Changes V2:
* suggested by David Rivshin
	* add more "compatible" strings for other chip variants/brands
	* renumber output <reg> values to expect a range of 1..9
	* fixes for typos and DT example, Kconfig message
	* fix location in Makefile and Kconfig
	* remove some dead/not implemented code
	* use of_property_read_string for better error handling
	* coding style improvements
	* use devm_led_classdev_register and simplify error path
* suggested by Jacek Anaszewski
	* fix more typos & writing style
	* separate bindings document into a second patch
	* max current property renamed and define uA instead of mA
	* debugging message improvements
	* remove platform data and header file completely and therefore require DT
	* use regmap to handle caching and locking + i2c serialization
* suggested by Rob Herring
	* bindings documentation style improvements

V1: 2016-04-18 20:43:18:
This patch adds a driver for the is31fl3196/99 dimmable dual/triple rgb
controller chips from ISSI.

H. Nikolaus Schaller (2):
  led: is31fl319x: 1/3/6/9-channel light effect led driver
  Bindings documentation for ISSI is31fl319x driver

 .../devicetree/bindings/leds/is31fl319x.txt        |  59 +++
 drivers/leds/Kconfig                               |  12 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-is31fl319x.c                     | 450 +++++++++++++++++++++
 4 files changed, 522 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/is31fl319x.txt
 create mode 100644 drivers/leds/leds-is31fl319x.c

-- 
2.7.3

[PATCH v6 1/2] led: is31fl319x: 1/3/6/9-channel light effect led driver

[H. Nikolaus Schaller]

This is a driver for the Integrated Silicon Solution Inc. LED driver
chips series IS31FL319x. They can drive 1, 3, 6  or up to 9
LEDs.

Each LED is individually controllable in brightness (through pwm)
in 256 steps so that RGB LEDs can show any of ca. 16 Mio colors.

The maximum current of the LEDs can be programmed and limited to
5 .. 40mA through a device tree property.

The chip is connected through I2C and can have one of 4 addresses
in the range 0x64 .. 0x67 depending on how the AD pin is connected. The
address is defined by the reg property as usual.

The chip also has a shutdown input which could be connected to a GPIO,
but this driver uses software shutdown if all LEDs are inactivated.

The chip also has breathing and audio features which are not fully
supported by this driver.

Tested-on: OMAP5 based Pyra handheld prototype.
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Andrey Utkin <andrey_utkin@fastmail.com>
---
 drivers/leds/Kconfig           |  12 ++
 drivers/leds/Makefile          |   1 +
 drivers/leds/leds-is31fl319x.c | 450 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 463 insertions(+)
 create mode 100644 drivers/leds/leds-is31fl319x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 5ae2834..6439a7d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -570,6 +570,18 @@ config LEDS_SEAD3
 	  This driver can also be built as a module. If so the module
 	  will be called leds-sead3.
 
+config LEDS_IS31FL319X
+	tristate "LED Support for ISSI IS31FL319x I2C LED controller family"
+	depends on LEDS_CLASS && I2C && OF
+	select REGMAP_I2C
+	help
+	  This option enables support for LEDs connected to ISSI IS31FL319x
+	  fancy LED driver chips accessed via the I2C bus.
+	  Driver supports individual PWM brightness control for each channel.
+
+	  This driver can also be built as a module. If so the module will be
+	  called leds-is31fl319x.
+
 config LEDS_IS31FL32XX
 	tristate "LED support for ISSI IS31FL32XX I2C LED controller family"
 	depends on LEDS_CLASS && I2C && OF
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cb2013d..b6ce9f9 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MENF21BMC)		+= leds-menf21bmc.o
 obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
 obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
 obj-$(CONFIG_LEDS_SEAD3)		+= leds-sead3.o
+obj-$(CONFIG_LEDS_IS31FL319X)		+= leds-is31fl319x.o
 obj-$(CONFIG_LEDS_IS31FL32XX)		+= leds-is31fl32xx.o
 
 # LED SPI Drivers
diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
new file mode 100644
index 0000000..dbdcfdd
--- /dev/null
+++ b/drivers/leds/leds-is31fl319x.c
@@ -0,0 +1,450 @@
+/*
+ * Copyright 2015-16 Golden Delicious Computers
+ *
+ * Author: Nikolaus Schaller <hns@goldelico.com>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License.  See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * LED driver for the IS31FL319{0,1,3,6,9} to drive 1, 3, 6 or 9 light
+ * effect LEDs.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* register numbers */
+#define IS31FL319X_SHUTDOWN     0x00
+#define IS31FL319X_CTRL1        0x01
+#define IS31FL319X_CTRL2        0x02
+#define IS31FL319X_CONFIG1      0x03
+#define IS31FL319X_CONFIG2      0x04
+#define IS31FL319X_RAMP_MODE    0x05
+#define IS31FL319X_BREATH_MASK  0x06
+#define IS31FL319X_PWM(channel) (0x07 + channel)
+#define IS31FL319X_DATA_UPDATE  0x10
+#define IS31FL319X_T0(channel)  (0x11 + channel)
+#define IS31FL319X_T123_1       0x1a
+#define IS31FL319X_T123_2       0x1b
+#define IS31FL319X_T123_3       0x1c
+#define IS31FL319X_T4(channel)  (0x1d + channel)
+#define IS31FL319X_TIME_UPDATE  0x26
+#define IS31FL319X_RESET        0xff
+
+#define IS31FL319X_REG_CNT      (IS31FL319X_RESET + 1)
+
+#define IS31FL319X_MAX_LEDS 9
+
+/* CS (Current Setting) in CONFIG2 register */
+#define IS31FL319X_CONFIG2_CS_SHIFT    4
+#define IS31FL319X_CONFIG2_CS_MASK     0x7
+#define IS31FL319X_CONFIG2_CS_STEP_REF 12
+
+#define IS31FL319X_CURRENT_MIN     ((u32)5000)
+#define IS31FL319X_CURRENT_MAX     ((u32)40000)
+#define IS31FL319X_CURRENT_STEP    ((u32)5000)
+#define IS31FL319X_CURRENT_DEFAULT ((u32)20000)
+
+/* Audio gain in CONFIG2 register */
+#define IS31FL319X_AUDIO_GAIN_DB_MAX  ((u32)21)
+#define IS31FL319X_AUDIO_GAIN_DB_STEP ((u32)3)
+
+/*
+ * regmap is used as a cache of chip's register space,
+ * to avoid reading back brightness values from chip,
+ * which is known to hang.
+ */
+struct is31fl319x_chip {
+	const struct is31fl319x_chipdef *cdef;
+	struct i2c_client               *client;
+	struct regmap                   *regmap;
+	struct mutex                    lock;
+	u32                             audio_gain_db;
+
+	struct is31fl319x_led {
+		struct is31fl319x_chip  *chip;
+		struct led_classdev     cdev;
+		u32                     max_microamp;
+		bool                    configured;
+	} leds[IS31FL319X_MAX_LEDS];
+};
+
+struct is31fl319x_chipdef {
+	int num_leds;
+};
+
+static const struct is31fl319x_chipdef is31fl3190_cdef = {
+	.num_leds = 1,
+};
+
+static const struct is31fl319x_chipdef is31fl3193_cdef = {
+	.num_leds = 3,
+};
+
+static const struct is31fl319x_chipdef is31fl3196_cdef = {
+	.num_leds = 6,
+};
+
+static const struct is31fl319x_chipdef is31fl3199_cdef = {
+	.num_leds = 9,
+};
+
+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,sn3199",    .data = &is31fl3199_cdef, },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
+
+static int is31fl319x_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 ctrl1 = 0, ctrl2 = 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, IS31FL319X_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, IS31FL319X_PWM(i), &pwm_value);
+		dev_dbg(&is31->client->dev, "%s read %d: ret=%d: %d\n",
+			__func__, i, ret, pwm_value);
+		on = ret >= 0 && pwm_value > LED_OFF;
+
+		if (i < 3)
+			ctrl1 |= on << i;       /* 0..2 => bit 0..2 */
+		else if (i < 6)
+			ctrl1 |= on << (i + 1); /* 3..5 => bit 4..6 */
+		else
+			ctrl2 |= on << (i - 6); /* 6..8 => bit 0..2 */
+	}
+
+	if (ctrl1 > 0 || ctrl2 > 0) {
+		dev_dbg(&is31->client->dev, "power up %02x %02x\n",
+			ctrl1, ctrl2);
+		regmap_write(is31->regmap, IS31FL319X_CTRL1, ctrl1);
+		regmap_write(is31->regmap, IS31FL319X_CTRL2, ctrl2);
+		/* update PWMs */
+		regmap_write(is31->regmap, IS31FL319X_DATA_UPDATE, 0x00);
+		/* enable chip from shut down */
+		ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x01);
+	} else {
+		dev_dbg(&is31->client->dev, "power down\n");
+		/* shut down (no need to clear CTRL1/2) */
+		ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x00);
+	}
+
+out:
+	mutex_unlock(&is31->lock);
+
+	return ret;
+}
+
+static int is31fl319x_parse_child_dt(const struct device *dev,
+				     const struct device_node *child,
+				     struct is31fl319x_led *led)
+{
+	struct led_classdev *cdev = &led->cdev;
+	int ret;
+
+	if (of_property_read_string(child, "label", &cdev->name))
+		cdev->name = child->name;
+
+	ret = of_property_read_string(child, "linux,default-trigger",
+				      &cdev->default_trigger);
+	if (ret < 0 && ret != -EINVAL) /* is optional */
+		return ret;
+
+	led->max_microamp = IS31FL319X_CURRENT_DEFAULT;
+	ret = of_property_read_u32(child, "led-max-microamp",
+				   &led->max_microamp);
+	if (!ret) {
+		if (led->max_microamp < IS31FL319X_CURRENT_MIN)
+			return -EINVAL;	/* not supported */
+		led->max_microamp = min(led->max_microamp,
+					  IS31FL319X_CURRENT_MAX);
+	}
+
+	return 0;
+}
+
+static int is31fl319x_parse_dt(struct device *dev,
+			       struct is31fl319x_chip *is31)
+{
+	struct device_node *np = dev->of_node, *child;
+	const struct of_device_id *of_dev_id;
+	int count;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	of_dev_id = of_match_device(of_is31fl319x_match, dev);
+	if (!of_dev_id) {
+		dev_err(dev, "Failed to match device with supported chips\n");
+		return -EINVAL;
+	}
+
+	is31->cdef = of_dev_id->data;
+
+	count = of_get_child_count(np);
+
+	dev_dbg(dev, "probe %s with %d leds defined in DT\n",
+		of_dev_id->compatible, 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;
+	}
+
+	for_each_child_of_node(np, child) {
+		struct is31fl319x_led *led;
+		u32 reg;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret) {
+			dev_err(dev, "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;
+			goto put_child_node;
+		}
+
+		led = &is31->leds[reg - 1];
+
+		if (led->configured) {
+			dev_err(dev, "led %u is already configured\n", reg);
+			ret = -EINVAL;
+			goto put_child_node;
+		}
+
+		ret = is31fl319x_parse_child_dt(dev, child, led);
+		if (ret) {
+			dev_err(dev, "led %u DT parsing failed\n", reg);
+			goto put_child_node;
+		}
+
+		led->configured = true;
+	}
+
+	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,
+					  IS31FL319X_AUDIO_GAIN_DB_MAX);
+
+	return 0;
+
+put_child_node:
+	of_node_put(child);
+	return ret;
+}
+
+static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
+{ /* we have no readable registers */
+	return false;
+}
+
+static bool is31fl319x_volatile_reg(struct device *dev, unsigned int reg)
+{ /* volatile registers are not cached */
+	switch (reg) {
+	case IS31FL319X_DATA_UPDATE:
+	case IS31FL319X_TIME_UPDATE:
+	case IS31FL319X_RESET:
+		return true; /* always write-through */
+	default:
+		return false;
+	}
+}
+
+static const struct reg_default is31fl319x_reg_defaults[] = {
+	{ IS31FL319X_CONFIG1, 0x00},
+	{ IS31FL319X_CONFIG2, 0x00},
+	{ IS31FL319X_PWM(0), 0x00},
+	{ IS31FL319X_PWM(1), 0x00},
+	{ IS31FL319X_PWM(2), 0x00},
+	{ IS31FL319X_PWM(3), 0x00},
+	{ IS31FL319X_PWM(4), 0x00},
+	{ IS31FL319X_PWM(5), 0x00},
+	{ IS31FL319X_PWM(6), 0x00},
+	{ IS31FL319X_PWM(7), 0x00},
+	{ IS31FL319X_PWM(8), 0x00},
+};
+
+static struct regmap_config regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = IS31FL319X_REG_CNT,
+	.cache_type = REGCACHE_FLAT,
+	.readable_reg = is31fl319x_readable_reg,
+	.volatile_reg = is31fl319x_volatile_reg,
+	.reg_defaults = is31fl319x_reg_defaults,
+	.num_reg_defaults = ARRAY_SIZE(is31fl319x_reg_defaults),
+};
+
+static inline int is31fl319x_microamp_to_cs(struct device *dev, u32 microamp)
+{ /* round down to nearest supported value (range check done by caller) */
+	u32 step = microamp / IS31FL319X_CURRENT_STEP;
+
+	return ((IS31FL319X_CONFIG2_CS_STEP_REF - step) &
+		IS31FL319X_CONFIG2_CS_MASK) <<
+		IS31FL319X_CONFIG2_CS_SHIFT; /* CS encoding */
+}
+
+static inline int is31fl319x_db_to_gain(u32 dezibel)
+{ /* round down to nearest supported value (range check done by caller) */
+	return dezibel / IS31FL319X_AUDIO_GAIN_DB_STEP;
+}
+
+static int is31fl319x_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	struct is31fl319x_chip *is31;
+	struct device *dev = &client->dev;
+	struct i2c_adapter *adapter = to_i2c_adapter(dev->parent);
+	int err;
+	int i = 0;
+	u32 aggregated_led_microamp = IS31FL319X_CURRENT_MAX;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
+		return -EIO;
+
+	is31 = devm_kzalloc(&client->dev, sizeof(*is31), GFP_KERNEL);
+	if (!is31)
+		return -ENOMEM;
+
+	mutex_init(&is31->lock);
+
+	err = is31fl319x_parse_dt(&client->dev, is31);
+	if (err)
+		goto free_mutex;
+
+	is31->client = client;
+	is31->regmap = devm_regmap_init_i2c(client, &regmap_config);
+	if (IS_ERR(is31->regmap)) {
+		dev_err(&client->dev, "failed to allocate register map\n");
+		err = PTR_ERR(is31->regmap);
+		goto free_mutex;
+	}
+
+	i2c_set_clientdata(client, is31);
+
+	/* check for write-reply from chip (we can't read any registers) */
+	err = regmap_write(is31->regmap, IS31FL319X_RESET, 0x00);
+	if (err < 0) {
+		dev_err(&client->dev, "no response from chip write: err = %d\n",
+			err);
+		err = -EIO; /* does not answer */
+		goto free_mutex;
+	}
+
+	/*
+	 * Kernel conventions require per-LED led-max-microamp property.
+	 * But the chip does not allow to limit individual LEDs.
+	 * So we take minimum from all subnodes for safety of hardware.
+	 */
+	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, IS31FL319X_CONFIG2,
+		     is31fl319x_microamp_to_cs(dev, aggregated_led_microamp) |
+		     is31fl319x_db_to_gain(is31->audio_gain_db));
+
+	for (i = 0; i < is31->cdef->num_leds; i++) {
+		struct is31fl319x_led *led = &is31->leds[i];
+
+		if (!led->configured)
+			continue;
+
+		led->chip = is31;
+		led->cdev.brightness_set_blocking = is31fl319x_brightness_set;
+
+		err = devm_led_classdev_register(&client->dev, &led->cdev);
+		if (err < 0)
+			goto free_mutex;
+	}
+
+	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;
+}
+
+/*
+ * i2c-core (and modalias) requires that id_table be properly filled,
+ * even though it is not used for DeviceTree based instantiation.
+ */
+static const struct i2c_device_id is31fl319x_id[] = {
+	{ "is31fl3190" },
+	{ "is31fl3191" },
+	{ "is31fl3193" },
+	{ "is31fl3196" },
+	{ "is31fl3199" },
+	{ "sn3199" },
+	{},
+};
+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),
+	},
+	.probe    = is31fl319x_probe,
+	.remove   = is31fl319x_remove,
+	.id_table = is31fl319x_id,
+};
+
+module_i2c_driver(is31fl319x_driver);
+
+MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
+MODULE_AUTHOR("Andrey Utkin <andrey_utkin@fastmail.com>");
+MODULE_DESCRIPTION("IS31FL319X LED driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.3

[PATCH v6 2/2] Bindings documentation for ISSI is31fl319x driver

[H. Nikolaus Schaller]

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../devicetree/bindings/leds/is31fl319x.txt        | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/is31fl319x.txt

diff --git a/Documentation/devicetree/bindings/leds/is31fl319x.txt b/Documentation/devicetree/bindings/leds/is31fl319x.txt
new file mode 100644
index 0000000..03287c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/is31fl319x.txt
@@ -0,0 +1,59 @@
+LEDs connected to is31fl319x LED controller chip
+
+Required properties:
+- compatible : Should be any of
+	"issi,is31fl3190"
+	"issi,is31fl3191"
+	"issi,is31fl3193"
+	"issi,is31fl3196"
+	"issi,is31fl3199"
+	"si-en,sn3199".
+- #address-cells: Must be 1.
+- #size-cells: Must be 0.
+- reg: 0x64, 0x65, 0x66, 0x67.
+
+Optional properties:
+- audio-gain-db : audio gain selection for external analog modulation input.
+	Valid values: 0 - 21, step by 3 (rounded down)
+	Default: 0
+
+Each led is represented as a sub-node of the issi,is31fl319x device.
+There can be less leds subnodes than the chip can support but not more.
+
+Required led sub-node properties:
+- reg : number of LED line
+	Valid values: 1 - number of leds supported by the chip variant.
+
+Optional led sub-node properties:
+- label : see Documentation/devicetree/bindings/leds/common.txt.
+- linux,default-trigger :
+	see Documentation/devicetree/bindings/leds/common.txt.
+- led-max-microamp : (optional)
+	Valid values: 5000 - 40000, step by 5000 (rounded down)
+	Default: 20000 (20 mA)
+	Note: a driver will take the lowest of all led limits since the
+	chip has a single global setting. The lowest value will be chosen
+	due to the PWM specificity, where lower brightness is achieved
+	by reducing the dury-cycle of pulses and not the current, which
+	will always have its peak value equal to led-max-microamp.
+
+Examples:
+
+fancy_leds: leds@65 {
+	compatible = "issi,is31fl3196";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x65>;
+
+	red_aux: led@1 {
+		label = "red:aux";
+		reg = <1>;
+		led-max-microamp = <10000>;
+	};
+
+	green_power: led@5 {
+		label = "green:power";
+		reg = <5>;
+		linux,default-trigger = "default-on";
+	};
+};
-- 
2.7.3

Re: [PATCH v6 1/2] led: is31fl319x: 1/3/6/9-channel light effect led driver

[Jacek Anaszewski]

Tweaked the tabulation of macro values a bit more and applied
the patches to the for-4.9 branch of linux-leds.git.

Thanks,
Jacek Anaszewski

On 07/19/2016 01:47 PM, H. Nikolaus Schaller wrote:
> This is a driver for the Integrated Silicon Solution Inc. LED driver
> chips series IS31FL319x. They can drive 1, 3, 6  or up to 9
> LEDs.
>
> Each LED is individually controllable in brightness (through pwm)
> in 256 steps so that RGB LEDs can show any of ca. 16 Mio colors.
>
> The maximum current of the LEDs can be programmed and limited to
> 5 .. 40mA through a device tree property.
>
> The chip is connected through I2C and can have one of 4 addresses
> in the range 0x64 .. 0x67 depending on how the AD pin is connected. The
> address is defined by the reg property as usual.
>
> The chip also has a shutdown input which could be connected to a GPIO,
> but this driver uses software shutdown if all LEDs are inactivated.
>
> The chip also has breathing and audio features which are not fully
> supported by this driver.
>
> Tested-on: OMAP5 based Pyra handheld prototype.
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Signed-off-by: Andrey Utkin <andrey_utkin@fastmail.com>
> ---
>   drivers/leds/Kconfig           |  12 ++
>   drivers/leds/Makefile          |   1 +
>   drivers/leds/leds-is31fl319x.c | 450 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 463 insertions(+)
>   create mode 100644 drivers/leds/leds-is31fl319x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 5ae2834..6439a7d 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -570,6 +570,18 @@ config LEDS_SEAD3
>   	  This driver can also be built as a module. If so the module
>   	  will be called leds-sead3.
>
> +config LEDS_IS31FL319X
> +	tristate "LED Support for ISSI IS31FL319x I2C LED controller family"
> +	depends on LEDS_CLASS && I2C && OF
> +	select REGMAP_I2C
> +	help
> +	  This option enables support for LEDs connected to ISSI IS31FL319x
> +	  fancy LED driver chips accessed via the I2C bus.
> +	  Driver supports individual PWM brightness control for each channel.
> +
> +	  This driver can also be built as a module. If so the module will be
> +	  called leds-is31fl319x.
> +
>   config LEDS_IS31FL32XX
>   	tristate "LED support for ISSI IS31FL32XX I2C LED controller family"
>   	depends on LEDS_CLASS && I2C && OF
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index cb2013d..b6ce9f9 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MENF21BMC)		+= leds-menf21bmc.o
>   obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
>   obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
>   obj-$(CONFIG_LEDS_SEAD3)		+= leds-sead3.o
> +obj-$(CONFIG_LEDS_IS31FL319X)		+= leds-is31fl319x.o
>   obj-$(CONFIG_LEDS_IS31FL32XX)		+= leds-is31fl32xx.o
>
>   # LED SPI Drivers
> diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
> new file mode 100644
> index 0000000..dbdcfdd
> --- /dev/null
> +++ b/drivers/leds/leds-is31fl319x.c
> @@ -0,0 +1,450 @@
> +/*
> + * Copyright 2015-16 Golden Delicious Computers
> + *
> + * Author: Nikolaus Schaller <hns@goldelico.com>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * LED driver for the IS31FL319{0,1,3,6,9} to drive 1, 3, 6 or 9 light
> + * effect LEDs.
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +/* register numbers */
> +#define IS31FL319X_SHUTDOWN     0x00
> +#define IS31FL319X_CTRL1        0x01
> +#define IS31FL319X_CTRL2        0x02
> +#define IS31FL319X_CONFIG1      0x03
> +#define IS31FL319X_CONFIG2      0x04
> +#define IS31FL319X_RAMP_MODE    0x05
> +#define IS31FL319X_BREATH_MASK  0x06
> +#define IS31FL319X_PWM(channel) (0x07 + channel)
> +#define IS31FL319X_DATA_UPDATE  0x10
> +#define IS31FL319X_T0(channel)  (0x11 + channel)
> +#define IS31FL319X_T123_1       0x1a
> +#define IS31FL319X_T123_2       0x1b
> +#define IS31FL319X_T123_3       0x1c
> +#define IS31FL319X_T4(channel)  (0x1d + channel)
> +#define IS31FL319X_TIME_UPDATE  0x26
> +#define IS31FL319X_RESET        0xff
> +
> +#define IS31FL319X_REG_CNT      (IS31FL319X_RESET + 1)
> +
> +#define IS31FL319X_MAX_LEDS 9
> +
> +/* CS (Current Setting) in CONFIG2 register */
> +#define IS31FL319X_CONFIG2_CS_SHIFT    4
> +#define IS31FL319X_CONFIG2_CS_MASK     0x7
> +#define IS31FL319X_CONFIG2_CS_STEP_REF 12
> +
> +#define IS31FL319X_CURRENT_MIN     ((u32)5000)
> +#define IS31FL319X_CURRENT_MAX     ((u32)40000)
> +#define IS31FL319X_CURRENT_STEP    ((u32)5000)
> +#define IS31FL319X_CURRENT_DEFAULT ((u32)20000)
> +
> +/* Audio gain in CONFIG2 register */
> +#define IS31FL319X_AUDIO_GAIN_DB_MAX  ((u32)21)
> +#define IS31FL319X_AUDIO_GAIN_DB_STEP ((u32)3)
> +
> +/*
> + * regmap is used as a cache of chip's register space,
> + * to avoid reading back brightness values from chip,
> + * which is known to hang.
> + */
> +struct is31fl319x_chip {
> +	const struct is31fl319x_chipdef *cdef;
> +	struct i2c_client               *client;
> +	struct regmap                   *regmap;
> +	struct mutex                    lock;
> +	u32                             audio_gain_db;
> +
> +	struct is31fl319x_led {
> +		struct is31fl319x_chip  *chip;
> +		struct led_classdev     cdev;
> +		u32                     max_microamp;
> +		bool                    configured;
> +	} leds[IS31FL319X_MAX_LEDS];
> +};
> +
> +struct is31fl319x_chipdef {
> +	int num_leds;
> +};
> +
> +static const struct is31fl319x_chipdef is31fl3190_cdef = {
> +	.num_leds = 1,
> +};
> +
> +static const struct is31fl319x_chipdef is31fl3193_cdef = {
> +	.num_leds = 3,
> +};
> +
> +static const struct is31fl319x_chipdef is31fl3196_cdef = {
> +	.num_leds = 6,
> +};
> +
> +static const struct is31fl319x_chipdef is31fl3199_cdef = {
> +	.num_leds = 9,
> +};
> +
> +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,sn3199",    .data = &is31fl3199_cdef, },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
> +
> +static int is31fl319x_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 ctrl1 = 0, ctrl2 = 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, IS31FL319X_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, IS31FL319X_PWM(i), &pwm_value);
> +		dev_dbg(&is31->client->dev, "%s read %d: ret=%d: %d\n",
> +			__func__, i, ret, pwm_value);
> +		on = ret >= 0 && pwm_value > LED_OFF;
> +
> +		if (i < 3)
> +			ctrl1 |= on << i;       /* 0..2 => bit 0..2 */
> +		else if (i < 6)
> +			ctrl1 |= on << (i + 1); /* 3..5 => bit 4..6 */
> +		else
> +			ctrl2 |= on << (i - 6); /* 6..8 => bit 0..2 */
> +	}
> +
> +	if (ctrl1 > 0 || ctrl2 > 0) {
> +		dev_dbg(&is31->client->dev, "power up %02x %02x\n",
> +			ctrl1, ctrl2);
> +		regmap_write(is31->regmap, IS31FL319X_CTRL1, ctrl1);
> +		regmap_write(is31->regmap, IS31FL319X_CTRL2, ctrl2);
> +		/* update PWMs */
> +		regmap_write(is31->regmap, IS31FL319X_DATA_UPDATE, 0x00);
> +		/* enable chip from shut down */
> +		ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x01);
> +	} else {
> +		dev_dbg(&is31->client->dev, "power down\n");
> +		/* shut down (no need to clear CTRL1/2) */
> +		ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x00);
> +	}
> +
> +out:
> +	mutex_unlock(&is31->lock);
> +
> +	return ret;
> +}
> +
> +static int is31fl319x_parse_child_dt(const struct device *dev,
> +				     const struct device_node *child,
> +				     struct is31fl319x_led *led)
> +{
> +	struct led_classdev *cdev = &led->cdev;
> +	int ret;
> +
> +	if (of_property_read_string(child, "label", &cdev->name))
> +		cdev->name = child->name;
> +
> +	ret = of_property_read_string(child, "linux,default-trigger",
> +				      &cdev->default_trigger);
> +	if (ret < 0 && ret != -EINVAL) /* is optional */
> +		return ret;
> +
> +	led->max_microamp = IS31FL319X_CURRENT_DEFAULT;
> +	ret = of_property_read_u32(child, "led-max-microamp",
> +				   &led->max_microamp);
> +	if (!ret) {
> +		if (led->max_microamp < IS31FL319X_CURRENT_MIN)
> +			return -EINVAL;	/* not supported */
> +		led->max_microamp = min(led->max_microamp,
> +					  IS31FL319X_CURRENT_MAX);
> +	}
> +
> +	return 0;
> +}
> +
> +static int is31fl319x_parse_dt(struct device *dev,
> +			       struct is31fl319x_chip *is31)
> +{
> +	struct device_node *np = dev->of_node, *child;
> +	const struct of_device_id *of_dev_id;
> +	int count;
> +	int ret;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	of_dev_id = of_match_device(of_is31fl319x_match, dev);
> +	if (!of_dev_id) {
> +		dev_err(dev, "Failed to match device with supported chips\n");
> +		return -EINVAL;
> +	}
> +
> +	is31->cdef = of_dev_id->data;
> +
> +	count = of_get_child_count(np);
> +
> +	dev_dbg(dev, "probe %s with %d leds defined in DT\n",
> +		of_dev_id->compatible, 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;
> +	}
> +
> +	for_each_child_of_node(np, child) {
> +		struct is31fl319x_led *led;
> +		u32 reg;
> +
> +		ret = of_property_read_u32(child, "reg", &reg);
> +		if (ret) {
> +			dev_err(dev, "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;
> +			goto put_child_node;
> +		}
> +
> +		led = &is31->leds[reg - 1];
> +
> +		if (led->configured) {
> +			dev_err(dev, "led %u is already configured\n", reg);
> +			ret = -EINVAL;
> +			goto put_child_node;
> +		}
> +
> +		ret = is31fl319x_parse_child_dt(dev, child, led);
> +		if (ret) {
> +			dev_err(dev, "led %u DT parsing failed\n", reg);
> +			goto put_child_node;
> +		}
> +
> +		led->configured = true;
> +	}
> +
> +	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,
> +					  IS31FL319X_AUDIO_GAIN_DB_MAX);
> +
> +	return 0;
> +
> +put_child_node:
> +	of_node_put(child);
> +	return ret;
> +}
> +
> +static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
> +{ /* we have no readable registers */
> +	return false;
> +}
> +
> +static bool is31fl319x_volatile_reg(struct device *dev, unsigned int reg)
> +{ /* volatile registers are not cached */
> +	switch (reg) {
> +	case IS31FL319X_DATA_UPDATE:
> +	case IS31FL319X_TIME_UPDATE:
> +	case IS31FL319X_RESET:
> +		return true; /* always write-through */
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct reg_default is31fl319x_reg_defaults[] = {
> +	{ IS31FL319X_CONFIG1, 0x00},
> +	{ IS31FL319X_CONFIG2, 0x00},
> +	{ IS31FL319X_PWM(0), 0x00},
> +	{ IS31FL319X_PWM(1), 0x00},
> +	{ IS31FL319X_PWM(2), 0x00},
> +	{ IS31FL319X_PWM(3), 0x00},
> +	{ IS31FL319X_PWM(4), 0x00},
> +	{ IS31FL319X_PWM(5), 0x00},
> +	{ IS31FL319X_PWM(6), 0x00},
> +	{ IS31FL319X_PWM(7), 0x00},
> +	{ IS31FL319X_PWM(8), 0x00},
> +};
> +
> +static struct regmap_config regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = IS31FL319X_REG_CNT,
> +	.cache_type = REGCACHE_FLAT,
> +	.readable_reg = is31fl319x_readable_reg,
> +	.volatile_reg = is31fl319x_volatile_reg,
> +	.reg_defaults = is31fl319x_reg_defaults,
> +	.num_reg_defaults = ARRAY_SIZE(is31fl319x_reg_defaults),
> +};
> +
> +static inline int is31fl319x_microamp_to_cs(struct device *dev, u32 microamp)
> +{ /* round down to nearest supported value (range check done by caller) */
> +	u32 step = microamp / IS31FL319X_CURRENT_STEP;
> +
> +	return ((IS31FL319X_CONFIG2_CS_STEP_REF - step) &
> +		IS31FL319X_CONFIG2_CS_MASK) <<
> +		IS31FL319X_CONFIG2_CS_SHIFT; /* CS encoding */
> +}
> +
> +static inline int is31fl319x_db_to_gain(u32 dezibel)
> +{ /* round down to nearest supported value (range check done by caller) */
> +	return dezibel / IS31FL319X_AUDIO_GAIN_DB_STEP;
> +}
> +
> +static int is31fl319x_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	struct is31fl319x_chip *is31;
> +	struct device *dev = &client->dev;
> +	struct i2c_adapter *adapter = to_i2c_adapter(dev->parent);
> +	int err;
> +	int i = 0;
> +	u32 aggregated_led_microamp = IS31FL319X_CURRENT_MAX;
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
> +		return -EIO;
> +
> +	is31 = devm_kzalloc(&client->dev, sizeof(*is31), GFP_KERNEL);
> +	if (!is31)
> +		return -ENOMEM;
> +
> +	mutex_init(&is31->lock);
> +
> +	err = is31fl319x_parse_dt(&client->dev, is31);
> +	if (err)
> +		goto free_mutex;
> +
> +	is31->client = client;
> +	is31->regmap = devm_regmap_init_i2c(client, &regmap_config);
> +	if (IS_ERR(is31->regmap)) {
> +		dev_err(&client->dev, "failed to allocate register map\n");
> +		err = PTR_ERR(is31->regmap);
> +		goto free_mutex;
> +	}
> +
> +	i2c_set_clientdata(client, is31);
> +
> +	/* check for write-reply from chip (we can't read any registers) */
> +	err = regmap_write(is31->regmap, IS31FL319X_RESET, 0x00);
> +	if (err < 0) {
> +		dev_err(&client->dev, "no response from chip write: err = %d\n",
> +			err);
> +		err = -EIO; /* does not answer */
> +		goto free_mutex;
> +	}
> +
> +	/*
> +	 * Kernel conventions require per-LED led-max-microamp property.
> +	 * But the chip does not allow to limit individual LEDs.
> +	 * So we take minimum from all subnodes for safety of hardware.
> +	 */
> +	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, IS31FL319X_CONFIG2,
> +		     is31fl319x_microamp_to_cs(dev, aggregated_led_microamp) |
> +		     is31fl319x_db_to_gain(is31->audio_gain_db));
> +
> +	for (i = 0; i < is31->cdef->num_leds; i++) {
> +		struct is31fl319x_led *led = &is31->leds[i];
> +
> +		if (!led->configured)
> +			continue;
> +
> +		led->chip = is31;
> +		led->cdev.brightness_set_blocking = is31fl319x_brightness_set;
> +
> +		err = devm_led_classdev_register(&client->dev, &led->cdev);
> +		if (err < 0)
> +			goto free_mutex;
> +	}
> +
> +	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;
> +}
> +
> +/*
> + * i2c-core (and modalias) requires that id_table be properly filled,
> + * even though it is not used for DeviceTree based instantiation.
> + */
> +static const struct i2c_device_id is31fl319x_id[] = {
> +	{ "is31fl3190" },
> +	{ "is31fl3191" },
> +	{ "is31fl3193" },
> +	{ "is31fl3196" },
> +	{ "is31fl3199" },
> +	{ "sn3199" },
> +	{},
> +};
> +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),
> +	},
> +	.probe    = is31fl319x_probe,
> +	.remove   = is31fl319x_remove,
> +	.id_table = is31fl319x_id,
> +};
> +
> +module_i2c_driver(is31fl319x_driver);
> +
> +MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
> +MODULE_AUTHOR("Andrey Utkin <andrey_utkin@fastmail.com>");
> +MODULE_DESCRIPTION("IS31FL319X LED driver");
> +MODULE_LICENSE("GPL v2");
>

Re: [PATCH v6 1/2] led: is31fl319x: 1/3/6/9-channel light effect led driver

[H. Nikolaus Schaller]

Hi,

> Am 19.07.2016 um 14:29 schrieb Jacek Anaszewski <j.anaszewski@samsung.com>:
> 
> Tweaked the tabulation of macro values a bit more and applied

thanks!
I think you better know how you want to have them exactly (checkpatch didn't complain).

> the patches to the for-4.9 branch of linux-leds.git.

Fine!

> 
> Thanks,
> Jacek Anaszewski

BR and thanks as well,
Nikolaus

> 
> On 07/19/2016 01:47 PM, H. Nikolaus Schaller wrote:
>> This is a driver for the Integrated Silicon Solution Inc. LED driver
>> chips series IS31FL319x. They can drive 1, 3, 6  or up to 9
>> LEDs.
>> 
>> Each LED is individually controllable in brightness (through pwm)
>> in 256 steps so that RGB LEDs can show any of ca. 16 Mio colors.
>> 
>> The maximum current of the LEDs can be programmed and limited to
>> 5 .. 40mA through a device tree property.
>> 
>> The chip is connected through I2C and can have one of 4 addresses
>> in the range 0x64 .. 0x67 depending on how the AD pin is connected. The
>> address is defined by the reg property as usual.
>> 
>> The chip also has a shutdown input which could be connected to a GPIO,
>> but this driver uses software shutdown if all LEDs are inactivated.
>> 
>> The chip also has breathing and audio features which are not fully
>> supported by this driver.
>> 
>> Tested-on: OMAP5 based Pyra handheld prototype.
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Signed-off-by: Andrey Utkin <andrey_utkin@fastmail.com>
>> ---
>>  drivers/leds/Kconfig           |  12 ++
>>  drivers/leds/Makefile          |   1 +
>>  drivers/leds/leds-is31fl319x.c | 450 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 463 insertions(+)
>>  create mode 100644 drivers/leds/leds-is31fl319x.c
>> 
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 5ae2834..6439a7d 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -570,6 +570,18 @@ config LEDS_SEAD3
>>  	  This driver can also be built as a module. If so the module
>>  	  will be called leds-sead3.
>> 
>> +config LEDS_IS31FL319X
>> +	tristate "LED Support for ISSI IS31FL319x I2C LED controller family"
>> +	depends on LEDS_CLASS && I2C && OF
>> +	select REGMAP_I2C
>> +	help
>> +	  This option enables support for LEDs connected to ISSI IS31FL319x
>> +	  fancy LED driver chips accessed via the I2C bus.
>> +	  Driver supports individual PWM brightness control for each channel.
>> +
>> +	  This driver can also be built as a module. If so the module will be
>> +	  called leds-is31fl319x.
>> +
>>  config LEDS_IS31FL32XX
>>  	tristate "LED support for ISSI IS31FL32XX I2C LED controller family"
>>  	depends on LEDS_CLASS && I2C && OF
>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>> index cb2013d..b6ce9f9 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -66,6 +66,7 @@ obj-$(CONFIG_LEDS_MENF21BMC)		+= leds-menf21bmc.o
>>  obj-$(CONFIG_LEDS_KTD2692)		+= leds-ktd2692.o
>>  obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
>>  obj-$(CONFIG_LEDS_SEAD3)		+= leds-sead3.o
>> +obj-$(CONFIG_LEDS_IS31FL319X)		+= leds-is31fl319x.o
>>  obj-$(CONFIG_LEDS_IS31FL32XX)		+= leds-is31fl32xx.o
>> 
>>  # LED SPI Drivers
>> diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
>> new file mode 100644
>> index 0000000..dbdcfdd
>> --- /dev/null
>> +++ b/drivers/leds/leds-is31fl319x.c
>> @@ -0,0 +1,450 @@
>> +/*
>> + * Copyright 2015-16 Golden Delicious Computers
>> + *
>> + * Author: Nikolaus Schaller <hns@goldelico.com>
>> + *
>> + * This file is subject to the terms and conditions of version 2 of
>> + * the GNU General Public License.  See the file COPYING in the main
>> + * directory of this archive for more details.
>> + *
>> + * LED driver for the IS31FL319{0,1,3,6,9} to drive 1, 3, 6 or 9 light
>> + * effect LEDs.
>> + *
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/leds.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +/* register numbers */
>> +#define IS31FL319X_SHUTDOWN     0x00
>> +#define IS31FL319X_CTRL1        0x01
>> +#define IS31FL319X_CTRL2        0x02
>> +#define IS31FL319X_CONFIG1      0x03
>> +#define IS31FL319X_CONFIG2      0x04
>> +#define IS31FL319X_RAMP_MODE    0x05
>> +#define IS31FL319X_BREATH_MASK  0x06
>> +#define IS31FL319X_PWM(channel) (0x07 + channel)
>> +#define IS31FL319X_DATA_UPDATE  0x10
>> +#define IS31FL319X_T0(channel)  (0x11 + channel)
>> +#define IS31FL319X_T123_1       0x1a
>> +#define IS31FL319X_T123_2       0x1b
>> +#define IS31FL319X_T123_3       0x1c
>> +#define IS31FL319X_T4(channel)  (0x1d + channel)
>> +#define IS31FL319X_TIME_UPDATE  0x26
>> +#define IS31FL319X_RESET        0xff
>> +
>> +#define IS31FL319X_REG_CNT      (IS31FL319X_RESET + 1)
>> +
>> +#define IS31FL319X_MAX_LEDS 9
>> +
>> +/* CS (Current Setting) in CONFIG2 register */
>> +#define IS31FL319X_CONFIG2_CS_SHIFT    4
>> +#define IS31FL319X_CONFIG2_CS_MASK     0x7
>> +#define IS31FL319X_CONFIG2_CS_STEP_REF 12
>> +
>> +#define IS31FL319X_CURRENT_MIN     ((u32)5000)
>> +#define IS31FL319X_CURRENT_MAX     ((u32)40000)
>> +#define IS31FL319X_CURRENT_STEP    ((u32)5000)
>> +#define IS31FL319X_CURRENT_DEFAULT ((u32)20000)
>> +
>> +/* Audio gain in CONFIG2 register */
>> +#define IS31FL319X_AUDIO_GAIN_DB_MAX  ((u32)21)
>> +#define IS31FL319X_AUDIO_GAIN_DB_STEP ((u32)3)
>> +
>> +/*
>> + * regmap is used as a cache of chip's register space,
>> + * to avoid reading back brightness values from chip,
>> + * which is known to hang.
>> + */
>> +struct is31fl319x_chip {
>> +	const struct is31fl319x_chipdef *cdef;
>> +	struct i2c_client               *client;
>> +	struct regmap                   *regmap;
>> +	struct mutex                    lock;
>> +	u32                             audio_gain_db;
>> +
>> +	struct is31fl319x_led {
>> +		struct is31fl319x_chip  *chip;
>> +		struct led_classdev     cdev;
>> +		u32                     max_microamp;
>> +		bool                    configured;
>> +	} leds[IS31FL319X_MAX_LEDS];
>> +};
>> +
>> +struct is31fl319x_chipdef {
>> +	int num_leds;
>> +};
>> +
>> +static const struct is31fl319x_chipdef is31fl3190_cdef = {
>> +	.num_leds = 1,
>> +};
>> +
>> +static const struct is31fl319x_chipdef is31fl3193_cdef = {
>> +	.num_leds = 3,
>> +};
>> +
>> +static const struct is31fl319x_chipdef is31fl3196_cdef = {
>> +	.num_leds = 6,
>> +};
>> +
>> +static const struct is31fl319x_chipdef is31fl3199_cdef = {
>> +	.num_leds = 9,
>> +};
>> +
>> +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,sn3199",    .data = &is31fl3199_cdef, },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
>> +
>> +static int is31fl319x_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 ctrl1 = 0, ctrl2 = 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, IS31FL319X_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, IS31FL319X_PWM(i), &pwm_value);
>> +		dev_dbg(&is31->client->dev, "%s read %d: ret=%d: %d\n",
>> +			__func__, i, ret, pwm_value);
>> +		on = ret >= 0 && pwm_value > LED_OFF;
>> +
>> +		if (i < 3)
>> +			ctrl1 |= on << i;       /* 0..2 => bit 0..2 */
>> +		else if (i < 6)
>> +			ctrl1 |= on << (i + 1); /* 3..5 => bit 4..6 */
>> +		else
>> +			ctrl2 |= on << (i - 6); /* 6..8 => bit 0..2 */
>> +	}
>> +
>> +	if (ctrl1 > 0 || ctrl2 > 0) {
>> +		dev_dbg(&is31->client->dev, "power up %02x %02x\n",
>> +			ctrl1, ctrl2);
>> +		regmap_write(is31->regmap, IS31FL319X_CTRL1, ctrl1);
>> +		regmap_write(is31->regmap, IS31FL319X_CTRL2, ctrl2);
>> +		/* update PWMs */
>> +		regmap_write(is31->regmap, IS31FL319X_DATA_UPDATE, 0x00);
>> +		/* enable chip from shut down */
>> +		ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x01);
>> +	} else {
>> +		dev_dbg(&is31->client->dev, "power down\n");
>> +		/* shut down (no need to clear CTRL1/2) */
>> +		ret = regmap_write(is31->regmap, IS31FL319X_SHUTDOWN, 0x00);
>> +	}
>> +
>> +out:
>> +	mutex_unlock(&is31->lock);
>> +
>> +	return ret;
>> +}
>> +
>> +static int is31fl319x_parse_child_dt(const struct device *dev,
>> +				     const struct device_node *child,
>> +				     struct is31fl319x_led *led)
>> +{
>> +	struct led_classdev *cdev = &led->cdev;
>> +	int ret;
>> +
>> +	if (of_property_read_string(child, "label", &cdev->name))
>> +		cdev->name = child->name;
>> +
>> +	ret = of_property_read_string(child, "linux,default-trigger",
>> +				      &cdev->default_trigger);
>> +	if (ret < 0 && ret != -EINVAL) /* is optional */
>> +		return ret;
>> +
>> +	led->max_microamp = IS31FL319X_CURRENT_DEFAULT;
>> +	ret = of_property_read_u32(child, "led-max-microamp",
>> +				   &led->max_microamp);
>> +	if (!ret) {
>> +		if (led->max_microamp < IS31FL319X_CURRENT_MIN)
>> +			return -EINVAL;	/* not supported */
>> +		led->max_microamp = min(led->max_microamp,
>> +					  IS31FL319X_CURRENT_MAX);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int is31fl319x_parse_dt(struct device *dev,
>> +			       struct is31fl319x_chip *is31)
>> +{
>> +	struct device_node *np = dev->of_node, *child;
>> +	const struct of_device_id *of_dev_id;
>> +	int count;
>> +	int ret;
>> +
>> +	if (!np)
>> +		return -ENODEV;
>> +
>> +	of_dev_id = of_match_device(of_is31fl319x_match, dev);
>> +	if (!of_dev_id) {
>> +		dev_err(dev, "Failed to match device with supported chips\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	is31->cdef = of_dev_id->data;
>> +
>> +	count = of_get_child_count(np);
>> +
>> +	dev_dbg(dev, "probe %s with %d leds defined in DT\n",
>> +		of_dev_id->compatible, 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;
>> +	}
>> +
>> +	for_each_child_of_node(np, child) {
>> +		struct is31fl319x_led *led;
>> +		u32 reg;
>> +
>> +		ret = of_property_read_u32(child, "reg", &reg);
>> +		if (ret) {
>> +			dev_err(dev, "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;
>> +			goto put_child_node;
>> +		}
>> +
>> +		led = &is31->leds[reg - 1];
>> +
>> +		if (led->configured) {
>> +			dev_err(dev, "led %u is already configured\n", reg);
>> +			ret = -EINVAL;
>> +			goto put_child_node;
>> +		}
>> +
>> +		ret = is31fl319x_parse_child_dt(dev, child, led);
>> +		if (ret) {
>> +			dev_err(dev, "led %u DT parsing failed\n", reg);
>> +			goto put_child_node;
>> +		}
>> +
>> +		led->configured = true;
>> +	}
>> +
>> +	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,
>> +					  IS31FL319X_AUDIO_GAIN_DB_MAX);
>> +
>> +	return 0;
>> +
>> +put_child_node:
>> +	of_node_put(child);
>> +	return ret;
>> +}
>> +
>> +static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
>> +{ /* we have no readable registers */
>> +	return false;
>> +}
>> +
>> +static bool is31fl319x_volatile_reg(struct device *dev, unsigned int reg)
>> +{ /* volatile registers are not cached */
>> +	switch (reg) {
>> +	case IS31FL319X_DATA_UPDATE:
>> +	case IS31FL319X_TIME_UPDATE:
>> +	case IS31FL319X_RESET:
>> +		return true; /* always write-through */
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static const struct reg_default is31fl319x_reg_defaults[] = {
>> +	{ IS31FL319X_CONFIG1, 0x00},
>> +	{ IS31FL319X_CONFIG2, 0x00},
>> +	{ IS31FL319X_PWM(0), 0x00},
>> +	{ IS31FL319X_PWM(1), 0x00},
>> +	{ IS31FL319X_PWM(2), 0x00},
>> +	{ IS31FL319X_PWM(3), 0x00},
>> +	{ IS31FL319X_PWM(4), 0x00},
>> +	{ IS31FL319X_PWM(5), 0x00},
>> +	{ IS31FL319X_PWM(6), 0x00},
>> +	{ IS31FL319X_PWM(7), 0x00},
>> +	{ IS31FL319X_PWM(8), 0x00},
>> +};
>> +
>> +static struct regmap_config regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.max_register = IS31FL319X_REG_CNT,
>> +	.cache_type = REGCACHE_FLAT,
>> +	.readable_reg = is31fl319x_readable_reg,
>> +	.volatile_reg = is31fl319x_volatile_reg,
>> +	.reg_defaults = is31fl319x_reg_defaults,
>> +	.num_reg_defaults = ARRAY_SIZE(is31fl319x_reg_defaults),
>> +};
>> +
>> +static inline int is31fl319x_microamp_to_cs(struct device *dev, u32 microamp)
>> +{ /* round down to nearest supported value (range check done by caller) */
>> +	u32 step = microamp / IS31FL319X_CURRENT_STEP;
>> +
>> +	return ((IS31FL319X_CONFIG2_CS_STEP_REF - step) &
>> +		IS31FL319X_CONFIG2_CS_MASK) <<
>> +		IS31FL319X_CONFIG2_CS_SHIFT; /* CS encoding */
>> +}
>> +
>> +static inline int is31fl319x_db_to_gain(u32 dezibel)
>> +{ /* round down to nearest supported value (range check done by caller) */
>> +	return dezibel / IS31FL319X_AUDIO_GAIN_DB_STEP;
>> +}
>> +
>> +static int is31fl319x_probe(struct i2c_client *client,
>> +			    const struct i2c_device_id *id)
>> +{
>> +	struct is31fl319x_chip *is31;
>> +	struct device *dev = &client->dev;
>> +	struct i2c_adapter *adapter = to_i2c_adapter(dev->parent);
>> +	int err;
>> +	int i = 0;
>> +	u32 aggregated_led_microamp = IS31FL319X_CURRENT_MAX;
>> +
>> +	if (!i2c_check_functionality(adapter, I2C_FUNC_I2C))
>> +		return -EIO;
>> +
>> +	is31 = devm_kzalloc(&client->dev, sizeof(*is31), GFP_KERNEL);
>> +	if (!is31)
>> +		return -ENOMEM;
>> +
>> +	mutex_init(&is31->lock);
>> +
>> +	err = is31fl319x_parse_dt(&client->dev, is31);
>> +	if (err)
>> +		goto free_mutex;
>> +
>> +	is31->client = client;
>> +	is31->regmap = devm_regmap_init_i2c(client, &regmap_config);
>> +	if (IS_ERR(is31->regmap)) {
>> +		dev_err(&client->dev, "failed to allocate register map\n");
>> +		err = PTR_ERR(is31->regmap);
>> +		goto free_mutex;
>> +	}
>> +
>> +	i2c_set_clientdata(client, is31);
>> +
>> +	/* check for write-reply from chip (we can't read any registers) */
>> +	err = regmap_write(is31->regmap, IS31FL319X_RESET, 0x00);
>> +	if (err < 0) {
>> +		dev_err(&client->dev, "no response from chip write: err = %d\n",
>> +			err);
>> +		err = -EIO; /* does not answer */
>> +		goto free_mutex;
>> +	}
>> +
>> +	/*
>> +	 * Kernel conventions require per-LED led-max-microamp property.
>> +	 * But the chip does not allow to limit individual LEDs.
>> +	 * So we take minimum from all subnodes for safety of hardware.
>> +	 */
>> +	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, IS31FL319X_CONFIG2,
>> +		     is31fl319x_microamp_to_cs(dev, aggregated_led_microamp) |
>> +		     is31fl319x_db_to_gain(is31->audio_gain_db));
>> +
>> +	for (i = 0; i < is31->cdef->num_leds; i++) {
>> +		struct is31fl319x_led *led = &is31->leds[i];
>> +
>> +		if (!led->configured)
>> +			continue;
>> +
>> +		led->chip = is31;
>> +		led->cdev.brightness_set_blocking = is31fl319x_brightness_set;
>> +
>> +		err = devm_led_classdev_register(&client->dev, &led->cdev);
>> +		if (err < 0)
>> +			goto free_mutex;
>> +	}
>> +
>> +	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;
>> +}
>> +
>> +/*
>> + * i2c-core (and modalias) requires that id_table be properly filled,
>> + * even though it is not used for DeviceTree based instantiation.
>> + */
>> +static const struct i2c_device_id is31fl319x_id[] = {
>> +	{ "is31fl3190" },
>> +	{ "is31fl3191" },
>> +	{ "is31fl3193" },
>> +	{ "is31fl3196" },
>> +	{ "is31fl3199" },
>> +	{ "sn3199" },
>> +	{},
>> +};
>> +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),
>> +	},
>> +	.probe    = is31fl319x_probe,
>> +	.remove   = is31fl319x_remove,
>> +	.id_table = is31fl319x_id,
>> +};
>> +
>> +module_i2c_driver(is31fl319x_driver);
>> +
>> +MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
>> +MODULE_AUTHOR("Andrey Utkin <andrey_utkin@fastmail.com>");
>> +MODULE_DESCRIPTION("IS31FL319X LED driver");
>> +MODULE_LICENSE("GPL v2");
>> 

Re: [PATCH v6 2/2] Bindings documentation for ISSI is31fl319x driver

[Rob Herring]

On Tue, Jul 19, 2016 at 01:47:31PM +0200, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  .../devicetree/bindings/leds/is31fl319x.txt        | 59 ++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/is31fl319x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/is31fl319x.txt b/Documentation/devicetree/bindings/leds/is31fl319x.txt
> new file mode 100644
> index 0000000..03287c0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/is31fl319x.txt
> @@ -0,0 +1,59 @@
> +LEDs connected to is31fl319x LED controller chip
> +
> +Required properties:
> +- compatible : Should be any of
> +	"issi,is31fl3190"
> +	"issi,is31fl3191"
> +	"issi,is31fl3193"
> +	"issi,is31fl3196"
> +	"issi,is31fl3199"
> +	"si-en,sn3199".
> +- #address-cells: Must be 1.
> +- #size-cells: Must be 0.
> +- reg: 0x64, 0x65, 0x66, 0x67.

This is an OR or AND? OR it seems. With that,

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v6 2/2] Bindings documentation for ISSI is31fl319x driver

[Jacek Anaszewski]

On 07/20/2016 03:24 AM, Rob Herring wrote:
> On Tue, Jul 19, 2016 at 01:47:31PM +0200, H. Nikolaus Schaller wrote:
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>>   .../devicetree/bindings/leds/is31fl319x.txt        | 59 ++++++++++++++++++++++
>>   1 file changed, 59 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/is31fl319x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/is31fl319x.txt b/Documentation/devicetree/bindings/leds/is31fl319x.txt
>> new file mode 100644
>> index 0000000..03287c0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/is31fl319x.txt
>> @@ -0,0 +1,59 @@
>> +LEDs connected to is31fl319x LED controller chip
>> +
>> +Required properties:
>> +- compatible : Should be any of
>> +	"issi,is31fl3190"
>> +	"issi,is31fl3191"
>> +	"issi,is31fl3193"
>> +	"issi,is31fl3196"
>> +	"issi,is31fl3199"
>> +	"si-en,sn3199".
>> +- #address-cells: Must be 1.
>> +- #size-cells: Must be 0.
>> +- reg: 0x64, 0x65, 0x66, 0x67.
>
> This is an OR or AND? OR it seems. With that,

Nikolaus, could you please make it explicit?

>
> Acked-by: Rob Herring <robh@kernel.org>
>
>


-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v6 2/2] Bindings documentation for ISSI is31fl319x driver

[H. Nikolaus Schaller]

Hi,

> Am 22.07.2016 um 08:36 schrieb Jacek Anaszewski <j.anaszewski@samsung.com>:
> 
> On 07/20/2016 03:24 AM, Rob Herring wrote:
>> On Tue, Jul 19, 2016 at 01:47:31PM +0200, H. Nikolaus Schaller wrote:
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>> ---
>>>  .../devicetree/bindings/leds/is31fl319x.txt        | 59 ++++++++++++++++++++++
>>>  1 file changed, 59 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/leds/is31fl319x.txt
>>> 
>>> diff --git a/Documentation/devicetree/bindings/leds/is31fl319x.txt b/Documentation/devicetree/bindings/leds/is31fl319x.txt
>>> new file mode 100644
>>> index 0000000..03287c0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/leds/is31fl319x.txt
>>> @@ -0,0 +1,59 @@
>>> +LEDs connected to is31fl319x LED controller chip
>>> +
>>> +Required properties:
>>> +- compatible : Should be any of
>>> +	"issi,is31fl3190"
>>> +	"issi,is31fl3191"
>>> +	"issi,is31fl3193"
>>> +	"issi,is31fl3196"
>>> +	"issi,is31fl3199"
>>> +	"si-en,sn3199".
>>> +- #address-cells: Must be 1.
>>> +- #size-cells: Must be 0.
>>> +- reg: 0x64, 0x65, 0x66, 0x67.
>> 
>> This is an OR or AND? OR it seems. With that,
> 
> Nikolaus, could you please make it explicit?

Yes, Rob's intuition is right. An AND does
obviously not make sense for a reg property.

It is an OR.

So if you perfer, please substitute 

- reg: 0x64, 0x65, 0x66, or 0x67.

BR,
Nikolaus

Re: [PATCH v6 2/2] Bindings documentation for ISSI is31fl319x driver

[Jacek Anaszewski]

On 07/22/2016 08:42 AM, H. Nikolaus Schaller wrote:
> Hi,
>
>> Am 22.07.2016 um 08:36 schrieb Jacek Anaszewski <j.anaszewski@samsung.com>:
>>
>> On 07/20/2016 03:24 AM, Rob Herring wrote:
>>> On Tue, Jul 19, 2016 at 01:47:31PM +0200, H. Nikolaus Schaller wrote:
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>>   .../devicetree/bindings/leds/is31fl319x.txt        | 59 ++++++++++++++++++++++
>>>>   1 file changed, 59 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/leds/is31fl319x.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/is31fl319x.txt b/Documentation/devicetree/bindings/leds/is31fl319x.txt
>>>> new file mode 100644
>>>> index 0000000..03287c0
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/leds/is31fl319x.txt
>>>> @@ -0,0 +1,59 @@
>>>> +LEDs connected to is31fl319x LED controller chip
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be any of
>>>> +	"issi,is31fl3190"
>>>> +	"issi,is31fl3191"
>>>> +	"issi,is31fl3193"
>>>> +	"issi,is31fl3196"
>>>> +	"issi,is31fl3199"
>>>> +	"si-en,sn3199".
>>>> +- #address-cells: Must be 1.
>>>> +- #size-cells: Must be 0.
>>>> +- reg: 0x64, 0x65, 0x66, 0x67.
>>>
>>> This is an OR or AND? OR it seems. With that,
>>
>> Nikolaus, could you please make it explicit?
>
> Yes, Rob's intuition is right. An AND does
> obviously not make sense for a reg property.
>
> It is an OR.
>
> So if you perfer, please substitute
>
> - reg: 0x64, 0x65, 0x66, or 0x67.

Updated the for-4.9 branch accordingly. I also added "led-" prefix
to the file name to match it with the vast majority of the
remaining ones.

-- 
Best regards,
Jacek Anaszewski