[PATCHv4 00/10] backlight: Add TI LMU backlight driver

[PATCHv4 00/10] backlight: Add TI LMU backlight driver

[Sebastian Reichel]

Hi,

This adds backlight support for TI LMU devices. It was tested with
an lm3532 on Motorola Droid 4, which uses one channel for display
backlight and one channel for keyboard backlight.

Changes since PATCHv3:
 * Complete overhaul to satisfy Rob's remarks regarding to the binding
 * Implement LED subsystem support for keyboard backlights
 * Add lots of cleanup patches for the TI LMU MFD driver

Changes since PATCHv2:
 * Drop all binding parts, that were NAK'd by Rob.

Changes since PATCHv1:
 * split dt-binding documentation in its own patch
 * move include/linux/mfd/ti-lmu-backlight.h to drivers/video/backlight/ti-lmu-backlight-data.h
 * replace LMU_BL_REG() with a normal struct
 * add "const" keyword to lots of internal structures
 * improve error handling
 * use atomic pwm api

-- Sebastian

Sebastian Reichel (10):
  mfd: ti-lmu: constify mfd_cell tables
  mfd: ti-lmu: switch to gpiod
  mfd: ti-lmu: use managed resource for everything
  mfd: ti-lmu: drop of_compatible for backlight driver
  mfd: ti-lmu: use of_device_get_match_data() helper
  mfd: ti-lmu: add PWM support
  mfd: ti-lmu: register one backlight device per channel
  backlight: add TI LMU backlight driver
  dt-bindings: mfd: ti-lmu: update for backlight
  ARM: dts: omap4-droid4: update backlight led-controller

 Documentation/devicetree/bindings/mfd/ti-lmu.txt | 119 ++--
 arch/arm/boot/dts/omap4-droid4-xt894.dts         |  20 +-
 drivers/mfd/ti-lmu.c                             | 170 +++---
 drivers/video/backlight/Kconfig                  |   7 +
 drivers/video/backlight/Makefile                 |   3 +
 drivers/video/backlight/ti-lmu-backlight-core.c  | 666 +++++++++++++++++++++++
 drivers/video/backlight/ti-lmu-backlight-data.c  | 304 +++++++++++
 drivers/video/backlight/ti-lmu-backlight-data.h  |  95 ++++
 include/linux/mfd/ti-lmu.h                       |  10 +-
 9 files changed, 1257 insertions(+), 137 deletions(-)
 create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
 create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
 create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h

-- 
2.16.2

[PATCHv4 01/10] mfd: ti-lmu: constify mfd_cell tables

[Sebastian Reichel]

Add const attribute to all mfd_cell structures.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index cfb411cde51c..990437e5ed0a 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -25,7 +25,7 @@
 #include <linux/slab.h>
 
 struct ti_lmu_data {
-	struct mfd_cell *cells;
+	const struct mfd_cell *cells;
 	int num_cells;
 	unsigned int max_register;
 };
@@ -63,7 +63,7 @@ static void ti_lmu_disable_hw(struct ti_lmu *lmu)
 		gpio_set_value(lmu->en_gpio, 0);
 }
 
-static struct mfd_cell lm3532_devices[] = {
+static const struct mfd_cell lm3532_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3532,
@@ -78,7 +78,7 @@ static struct mfd_cell lm3532_devices[] = {
 	.of_compatible = "ti,lm363x-regulator",	\
 }						\
 
-static struct mfd_cell lm3631_devices[] = {
+static const struct mfd_cell lm3631_devices[] = {
 	LM363X_REGULATOR(LM3631_BOOST),
 	LM363X_REGULATOR(LM3631_LDO_CONT),
 	LM363X_REGULATOR(LM3631_LDO_OREF),
@@ -91,7 +91,7 @@ static struct mfd_cell lm3631_devices[] = {
 	},
 };
 
-static struct mfd_cell lm3632_devices[] = {
+static const struct mfd_cell lm3632_devices[] = {
 	LM363X_REGULATOR(LM3632_BOOST),
 	LM363X_REGULATOR(LM3632_LDO_POS),
 	LM363X_REGULATOR(LM3632_LDO_NEG),
@@ -102,7 +102,7 @@ static struct mfd_cell lm3632_devices[] = {
 	},
 };
 
-static struct mfd_cell lm3633_devices[] = {
+static const struct mfd_cell lm3633_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3633,
@@ -120,7 +120,7 @@ static struct mfd_cell lm3633_devices[] = {
 	},
 };
 
-static struct mfd_cell lm3695_devices[] = {
+static const struct mfd_cell lm3695_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3695,
@@ -128,7 +128,7 @@ static struct mfd_cell lm3695_devices[] = {
 	},
 };
 
-static struct mfd_cell lm3697_devices[] = {
+static const struct mfd_cell lm3697_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3697,
-- 
2.16.2

Re: [PATCHv4 01/10] mfd: ti-lmu: constify mfd_cell tables

[Lee Jones]

On Fri, 30 Mar 2018, Sebastian Reichel wrote:

> Add const attribute to all mfd_cell structures.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCHv4 02/10] mfd: ti-lmu: switch to gpiod

[Sebastian Reichel]

Use new descriptor based API instead of the legacy one.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c       | 28 ++++++++++++----------------
 include/linux/mfd/ti-lmu.h |  3 ++-
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index 990437e5ed0a..e14cb9f41b44 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -12,7 +12,7 @@
 
 #include <linux/delay.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/mfd/core.h>
@@ -21,7 +21,6 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 
 struct ti_lmu_data {
@@ -32,17 +31,8 @@ struct ti_lmu_data {
 
 static int ti_lmu_enable_hw(struct ti_lmu *lmu, enum ti_lmu_id id)
 {
-	int ret;
-
-	if (gpio_is_valid(lmu->en_gpio)) {
-		ret = devm_gpio_request_one(lmu->dev, lmu->en_gpio,
-					    GPIOF_OUT_INIT_HIGH, "lmu_hwen");
-		if (ret) {
-			dev_err(lmu->dev, "Can not request enable GPIO: %d\n",
-				ret);
-			return ret;
-		}
-	}
+	if (lmu->en_gpio)
+		gpiod_set_value(lmu->en_gpio, 1);
 
 	/* Delay about 1ms after HW enable pin control */
 	usleep_range(1000, 1500);
@@ -59,8 +49,8 @@ static int ti_lmu_enable_hw(struct ti_lmu *lmu, enum ti_lmu_id id)
 
 static void ti_lmu_disable_hw(struct ti_lmu *lmu)
 {
-	if (gpio_is_valid(lmu->en_gpio))
-		gpio_set_value(lmu->en_gpio, 0);
+	if (lmu->en_gpio)
+		gpiod_set_value(lmu->en_gpio, 0);
 }
 
 static const struct mfd_cell lm3532_devices[] = {
@@ -204,7 +194,13 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 		return PTR_ERR(lmu->regmap);
 
 	/* HW enable pin control and additional power up sequence if required */
-	lmu->en_gpio = of_get_named_gpio(dev->of_node, "enable-gpios", 0);
+	lmu->en_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(lmu->en_gpio)) {
+		ret = PTR_ERR(lmu->en_gpio);
+		dev_err(dev, "Can not request enable GPIO: %d\n", ret);
+		return ret;
+	}
+
 	ret = ti_lmu_enable_hw(lmu, id->driver_data);
 	if (ret)
 		return ret;
diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
index 09d5f30384e5..1ef51ed36be5 100644
--- a/include/linux/mfd/ti-lmu.h
+++ b/include/linux/mfd/ti-lmu.h
@@ -16,6 +16,7 @@
 #include <linux/gpio.h>
 #include <linux/notifier.h>
 #include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
 
 /* Notifier event */
 #define LMU_EVENT_MONITOR_DONE		0x01
@@ -81,7 +82,7 @@ enum lm363x_regulator_id {
 struct ti_lmu {
 	struct device *dev;
 	struct regmap *regmap;
-	int en_gpio;
+	struct gpio_desc *en_gpio;
 	struct blocking_notifier_head notifier;
 };
 #endif
-- 
2.16.2

Re: [PATCHv4 02/10] mfd: ti-lmu: switch to gpiod

[Lee Jones]

On Fri, 30 Mar 2018, Sebastian Reichel wrote:

> Use new descriptor based API instead of the legacy one.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c       | 28 ++++++++++++----------------
>  include/linux/mfd/ti-lmu.h |  3 ++-
>  2 files changed, 14 insertions(+), 17 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCHv4 03/10] mfd: ti-lmu: use managed resource for everything

[Sebastian Reichel]

This replaces all remaining unmanaged resources with device
managed ones, so that the remove function is no longer needed.
This makes the code slightly shorter and fixes two problems:

1. The hardware is disabled after the child devices have
   been removed. Previously there was a potential race
   condition.
2. The hardware is disabled when mfd_add_devices fails
   during probe.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index e14cb9f41b44..2ee09d099832 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -47,8 +47,9 @@ static int ti_lmu_enable_hw(struct ti_lmu *lmu, enum ti_lmu_id id)
 	return 0;
 }
 
-static void ti_lmu_disable_hw(struct ti_lmu *lmu)
+static void ti_lmu_disable_hw(void *data)
 {
+	struct ti_lmu *lmu = data;
 	if (lmu->en_gpio)
 		gpiod_set_value(lmu->en_gpio, 0);
 }
@@ -205,6 +206,10 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, ti_lmu_disable_hw, lmu);
+	if (ret)
+		return ret;
+
 	/*
 	 * Fault circuit(open/short) can be detected by ti-lmu-fault-monitor.
 	 * After fault detection is done, some devices should re-initialize
@@ -214,17 +219,8 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 
 	i2c_set_clientdata(cl, lmu);
 
-	return mfd_add_devices(lmu->dev, 0, data->cells,
-			       data->num_cells, NULL, 0, NULL);
-}
-
-static int ti_lmu_remove(struct i2c_client *cl)
-{
-	struct ti_lmu *lmu = i2c_get_clientdata(cl);
-
-	ti_lmu_disable_hw(lmu);
-	mfd_remove_devices(lmu->dev);
-	return 0;
+	return devm_mfd_add_devices(lmu->dev, 0, data->cells,
+				    data->num_cells, NULL, 0, NULL);
 }
 
 static const struct i2c_device_id ti_lmu_ids[] = {
@@ -240,7 +236,6 @@ MODULE_DEVICE_TABLE(i2c, ti_lmu_ids);
 
 static struct i2c_driver ti_lmu_driver = {
 	.probe = ti_lmu_probe,
-	.remove = ti_lmu_remove,
 	.driver = {
 		.name = "ti-lmu",
 		.of_match_table = ti_lmu_of_match,
-- 
2.16.2

Re: [PATCHv4 03/10] mfd: ti-lmu: use managed resource for everything

[Lee Jones]

On Fri, 30 Mar 2018, Sebastian Reichel wrote:

> This replaces all remaining unmanaged resources with device
> managed ones, so that the remove function is no longer needed.
> This makes the code slightly shorter and fixes two problems:
> 
> 1. The hardware is disabled after the child devices have
>    been removed. Previously there was a potential race
>    condition.
> 2. The hardware is disabled when mfd_add_devices fails
>    during probe.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCHv4 04/10] mfd: ti-lmu: drop of_compatible for backlight driver

[Sebastian Reichel]

This removes the of_compatible for the backlight sub-device. There
will be no extra sub-node for the backlight.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index 2ee09d099832..de76298cf3a8 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -58,7 +58,6 @@ static const struct mfd_cell lm3532_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3532,
-		.of_compatible = "ti,lm3532-backlight",
 	},
 };
 
@@ -78,7 +77,6 @@ static const struct mfd_cell lm3631_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3631,
-		.of_compatible = "ti,lm3631-backlight",
 	},
 };
 
@@ -89,7 +87,6 @@ static const struct mfd_cell lm3632_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3632,
-		.of_compatible = "ti,lm3632-backlight",
 	},
 };
 
@@ -97,7 +94,6 @@ static const struct mfd_cell lm3633_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3633,
-		.of_compatible = "ti,lm3633-backlight",
 	},
 	{
 		.name          = "lm3633-leds",
@@ -115,7 +111,6 @@ static const struct mfd_cell lm3695_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3695,
-		.of_compatible = "ti,lm3695-backlight",
 	},
 };
 
@@ -123,7 +118,6 @@ static const struct mfd_cell lm3697_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
 		.id            = LM3697,
-		.of_compatible = "ti,lm3697-backlight",
 	},
 	/* Monitoring driver for open/short circuit detection */
 	{
-- 
2.16.2

Re: [PATCHv4 04/10] mfd: ti-lmu: drop of_compatible for backlight driver

[Lee Jones]

On Fri, 30 Mar 2018, Sebastian Reichel wrote:

> This removes the of_compatible for the backlight sub-device. There
> will be no extra sub-node for the backlight.

Won't doing this in a separate patch break bisectability?

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
> index 2ee09d099832..de76298cf3a8 100644
> --- a/drivers/mfd/ti-lmu.c
> +++ b/drivers/mfd/ti-lmu.c
> @@ -58,7 +58,6 @@ static const struct mfd_cell lm3532_devices[] = {
>  	{
>  		.name          = "ti-lmu-backlight",
>  		.id            = LM3532,
> -		.of_compatible = "ti,lm3532-backlight",
>  	},
>  };
>  
> @@ -78,7 +77,6 @@ static const struct mfd_cell lm3631_devices[] = {
>  	{
>  		.name          = "ti-lmu-backlight",
>  		.id            = LM3631,
> -		.of_compatible = "ti,lm3631-backlight",
>  	},
>  };
>  
> @@ -89,7 +87,6 @@ static const struct mfd_cell lm3632_devices[] = {
>  	{
>  		.name          = "ti-lmu-backlight",
>  		.id            = LM3632,
> -		.of_compatible = "ti,lm3632-backlight",
>  	},
>  };
>  
> @@ -97,7 +94,6 @@ static const struct mfd_cell lm3633_devices[] = {
>  	{
>  		.name          = "ti-lmu-backlight",
>  		.id            = LM3633,
> -		.of_compatible = "ti,lm3633-backlight",
>  	},
>  	{
>  		.name          = "lm3633-leds",
> @@ -115,7 +111,6 @@ static const struct mfd_cell lm3695_devices[] = {
>  	{
>  		.name          = "ti-lmu-backlight",
>  		.id            = LM3695,
> -		.of_compatible = "ti,lm3695-backlight",
>  	},
>  };
>  
> @@ -123,7 +118,6 @@ static const struct mfd_cell lm3697_devices[] = {
>  	{
>  		.name          = "ti-lmu-backlight",
>  		.id            = LM3697,
> -		.of_compatible = "ti,lm3697-backlight",
>  	},
>  	/* Monitoring driver for open/short circuit detection */
>  	{

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCHv4 05/10] mfd: ti-lmu: use of_device_get_match_data() helper

[Sebastian Reichel]

Replace of_match_device() with of_device_get_match_data(), which
slightly decreases lines of code and allows to move the DT table
next to the I2C table.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index de76298cf3a8..ce16c896879b 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -142,34 +142,21 @@ TI_LMU_DATA(lm3633, LM3633_MAX_REG);
 TI_LMU_DATA(lm3695, LM3695_MAX_REG);
 TI_LMU_DATA(lm3697, LM3697_MAX_REG);
 
-static const struct of_device_id ti_lmu_of_match[] = {
-	{ .compatible = "ti,lm3532", .data = &lm3532_data },
-	{ .compatible = "ti,lm3631", .data = &lm3631_data },
-	{ .compatible = "ti,lm3632", .data = &lm3632_data },
-	{ .compatible = "ti,lm3633", .data = &lm3633_data },
-	{ .compatible = "ti,lm3695", .data = &lm3695_data },
-	{ .compatible = "ti,lm3697", .data = &lm3697_data },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, ti_lmu_of_match);
-
 static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 {
 	struct device *dev = &cl->dev;
-	const struct of_device_id *match;
 	const struct ti_lmu_data *data;
 	struct regmap_config regmap_cfg;
 	struct ti_lmu *lmu;
 	int ret;
 
-	match = of_match_device(ti_lmu_of_match, dev);
-	if (!match)
-		return -ENODEV;
 	/*
 	 * Get device specific data from of_match table.
 	 * This data is defined by using TI_LMU_DATA() macro.
 	 */
-	data = (struct ti_lmu_data *)match->data;
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return -ENODEV;
 
 	lmu = devm_kzalloc(dev, sizeof(*lmu), GFP_KERNEL);
 	if (!lmu)
@@ -217,6 +204,17 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 				    data->num_cells, NULL, 0, NULL);
 }
 
+static const struct of_device_id ti_lmu_of_match[] = {
+	{ .compatible = "ti,lm3532", .data = &lm3532_data },
+	{ .compatible = "ti,lm3631", .data = &lm3631_data },
+	{ .compatible = "ti,lm3632", .data = &lm3632_data },
+	{ .compatible = "ti,lm3633", .data = &lm3633_data },
+	{ .compatible = "ti,lm3695", .data = &lm3695_data },
+	{ .compatible = "ti,lm3697", .data = &lm3697_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_lmu_of_match);
+
 static const struct i2c_device_id ti_lmu_ids[] = {
 	{ "lm3532", LM3532 },
 	{ "lm3631", LM3631 },
-- 
2.16.2

Re: [PATCHv4 05/10] mfd: ti-lmu: use of_device_get_match_data() helper

[Lee Jones]

On Fri, 30 Mar 2018, Sebastian Reichel wrote:

> Replace of_match_device() with of_device_get_match_data(), which
> slightly decreases lines of code and allows to move the DT table
> next to the I2C table.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCHv4 06/10] mfd: ti-lmu: add PWM support

[Sebastian Reichel]

This adds support to acquire the optional PWM channel,
that can be used by some of the LMU variants.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c       | 11 +++++++++++
 include/linux/mfd/ti-lmu.h |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index ce16c896879b..f43b8acc30e1 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -183,6 +183,17 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 		return ret;
 	}
 
+	lmu->pwm = devm_pwm_get(dev, "lmu-backlight");
+	if (IS_ERR(lmu->pwm)) {
+		ret = PTR_ERR(lmu->pwm);
+		if (ret != -EINVAL) {
+			dev_err(dev, "Failed to get PWM: %d\n", ret);
+			return ret;
+		}
+
+		lmu->pwm = NULL;
+	}
+
 	ret = ti_lmu_enable_hw(lmu, id->driver_data);
 	if (ret)
 		return ret;
diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
index 1ef51ed36be5..246ab5145dff 100644
--- a/include/linux/mfd/ti-lmu.h
+++ b/include/linux/mfd/ti-lmu.h
@@ -17,6 +17,7 @@
 #include <linux/notifier.h>
 #include <linux/regmap.h>
 #include <linux/gpio/consumer.h>
+#include <linux/pwm.h>
 
 /* Notifier event */
 #define LMU_EVENT_MONITOR_DONE		0x01
@@ -77,12 +78,14 @@ enum lm363x_regulator_id {
  * @dev:	Parent device pointer
  * @regmap:	Used for i2c communcation on accessing registers
  * @en_gpio:	GPIO for HWEN pin [Optional]
+ * @pwm:	PWM for module [Optional]
  * @notifier:	Notifier for reporting hwmon event
  */
 struct ti_lmu {
 	struct device *dev;
 	struct regmap *regmap;
 	struct gpio_desc *en_gpio;
+	struct pwm_device *pwm;
 	struct blocking_notifier_head notifier;
 };
 #endif
-- 
2.16.2

Re: [PATCHv4 06/10] mfd: ti-lmu: add PWM support

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

On Fri 2018-03-30 19:24:10, Sebastian Reichel wrote:
> This adds support to acquire the optional PWM channel,
> that can be used by some of the LMU variants.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

Patches 1-6:

Acked-by: Pavel Machek <pavel@ucw.cz>

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv4 06/10] mfd: ti-lmu: add PWM support

[Dan Murphy]

Sebastian

-Milo

On 03/30/2018 12:24 PM, Sebastian Reichel wrote:
> This adds support to acquire the optional PWM channel,
> that can be used by some of the LMU variants.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c       | 11 +++++++++++
>  include/linux/mfd/ti-lmu.h |  3 +++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
> index ce16c896879b..f43b8acc30e1 100644
> --- a/drivers/mfd/ti-lmu.c
> +++ b/drivers/mfd/ti-lmu.c
> @@ -183,6 +183,17 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>  		return ret;
>  	}
>  
> +	lmu->pwm = devm_pwm_get(dev, "lmu-backlight");

Patch 9 of this series, or a derivative of it, should technically be before Patch 6 so that the binding is correct.
Otherwise the binding will not match the code

Dan



> +	if (IS_ERR(lmu->pwm)) {
> +		ret = PTR_ERR(lmu->pwm);
> +		if (ret != -EINVAL) {
> +			dev_err(dev, "Failed to get PWM: %d\n", ret);
> +			return ret;
> +		}
> +
> +		lmu->pwm = NULL;
> +	}
> +
>  	ret = ti_lmu_enable_hw(lmu, id->driver_data);
>  	if (ret)
>  		return ret;
> diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
> index 1ef51ed36be5..246ab5145dff 100644
> --- a/include/linux/mfd/ti-lmu.h
> +++ b/include/linux/mfd/ti-lmu.h
> @@ -17,6 +17,7 @@
>  #include <linux/notifier.h>
>  #include <linux/regmap.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/pwm.h>
>  
>  /* Notifier event */
>  #define LMU_EVENT_MONITOR_DONE		0x01
> @@ -77,12 +78,14 @@ enum lm363x_regulator_id {
>   * @dev:	Parent device pointer
>   * @regmap:	Used for i2c communcation on accessing registers
>   * @en_gpio:	GPIO for HWEN pin [Optional]
> + * @pwm:	PWM for module [Optional]
>   * @notifier:	Notifier for reporting hwmon event
>   */
>  struct ti_lmu {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	struct gpio_desc *en_gpio;
> +	struct pwm_device *pwm;
>  	struct blocking_notifier_head notifier;
>  };
>  #endif
> 


-- 
------------------
Dan Murphy

Re: [PATCHv4 06/10] mfd: ti-lmu: add PWM support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

Hi,

On Wed, Apr 04, 2018 at 02:04:16PM -0500, Dan Murphy wrote:
> Sebastian
> 
> -Milo
> 
> On 03/30/2018 12:24 PM, Sebastian Reichel wrote:
> > This adds support to acquire the optional PWM channel,
> > that can be used by some of the LMU variants.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/mfd/ti-lmu.c       | 11 +++++++++++
> >  include/linux/mfd/ti-lmu.h |  3 +++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
> > index ce16c896879b..f43b8acc30e1 100644
> > --- a/drivers/mfd/ti-lmu.c
> > +++ b/drivers/mfd/ti-lmu.c
> > @@ -183,6 +183,17 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> >  		return ret;
> >  	}
> >  
> > +	lmu->pwm = devm_pwm_get(dev, "lmu-backlight");
> 
> Patch 9 of this series, or a derivative of it, should technically
> be before Patch 6 so that the binding is correct.  Otherwise the
> binding will not match the code.

I guess the binding update should be the first patch. I will change
this in the next revision.

-- Sebastian

> > +	if (IS_ERR(lmu->pwm)) {
> > +		ret = PTR_ERR(lmu->pwm);
> > +		if (ret != -EINVAL) {
> > +			dev_err(dev, "Failed to get PWM: %d\n", ret);
> > +			return ret;
> > +		}
> > +
> > +		lmu->pwm = NULL;
> > +	}
> > +
> >  	ret = ti_lmu_enable_hw(lmu, id->driver_data);
> >  	if (ret)
> >  		return ret;
> > diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
> > index 1ef51ed36be5..246ab5145dff 100644
> > --- a/include/linux/mfd/ti-lmu.h
> > +++ b/include/linux/mfd/ti-lmu.h
> > @@ -17,6 +17,7 @@
> >  #include <linux/notifier.h>
> >  #include <linux/regmap.h>
> >  #include <linux/gpio/consumer.h>
> > +#include <linux/pwm.h>
> >  
> >  /* Notifier event */
> >  #define LMU_EVENT_MONITOR_DONE		0x01
> > @@ -77,12 +78,14 @@ enum lm363x_regulator_id {
> >   * @dev:	Parent device pointer
> >   * @regmap:	Used for i2c communcation on accessing registers
> >   * @en_gpio:	GPIO for HWEN pin [Optional]
> > + * @pwm:	PWM for module [Optional]
> >   * @notifier:	Notifier for reporting hwmon event
> >   */
> >  struct ti_lmu {
> >  	struct device *dev;
> >  	struct regmap *regmap;
> >  	struct gpio_desc *en_gpio;
> > +	struct pwm_device *pwm;
> >  	struct blocking_notifier_head notifier;
> >  };
> >  #endif
> > 
> 
> 
> -- 
> ------------------
> Dan Murphy

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv4 07/10] mfd: ti-lmu: register one backlight device per channel

[Sebastian Reichel]

All LMU devices support multiple channels, that can be controlled
independently. This registers one backlight sub-device per channel.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/mfd/ti-lmu.c       | 62 +++++++++++++++++++++++++++++++++++++---------
 include/linux/mfd/ti-lmu.h |  4 +++
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index f43b8acc30e1..f4311d215dfa 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -56,8 +56,16 @@ static void ti_lmu_disable_hw(void *data)
 
 static const struct mfd_cell lm3532_devices[] = {
 	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3532,
+		.name          = "ti-lmu-led-backlight",
+		.id            = 0,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 1,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 2,
 	},
 };
 
@@ -75,8 +83,12 @@ static const struct mfd_cell lm3631_devices[] = {
 	LM363X_REGULATOR(LM3631_LDO_POS),
 	LM363X_REGULATOR(LM3631_LDO_NEG),
 	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3631,
+		.name          = "ti-lmu-led-backlight",
+		.id            = 0,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 1,
 	},
 };
 
@@ -85,15 +97,27 @@ static const struct mfd_cell lm3632_devices[] = {
 	LM363X_REGULATOR(LM3632_LDO_POS),
 	LM363X_REGULATOR(LM3632_LDO_NEG),
 	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3632,
+		.name          = "ti-lmu-led-backlight",
+		.id            = 0,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 1,
 	},
 };
 
 static const struct mfd_cell lm3633_devices[] = {
 	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3633,
+		.name          = "ti-lmu-led-backlight",
+		.id            = 0,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 1,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 2,
 	},
 	{
 		.name          = "lm3633-leds",
@@ -109,15 +133,27 @@ static const struct mfd_cell lm3633_devices[] = {
 
 static const struct mfd_cell lm3695_devices[] = {
 	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3695,
+		.name          = "ti-lmu-led-backlight",
+		.id            = 0,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 1,
 	},
 };
 
 static const struct mfd_cell lm3697_devices[] = {
 	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3697,
+		.name          = "ti-lmu-led-backlight",
+		.id            = 0,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 1,
+	},
+	{
+		.name          = "ti-lmu-led-backlight",
+		.id            = 2,
 	},
 	/* Monitoring driver for open/short circuit detection */
 	{
@@ -163,6 +199,7 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 		return -ENOMEM;
 
 	lmu->dev = &cl->dev;
+	lmu->id = id->driver_data;
 
 	/* Setup regmap */
 	memset(&regmap_cfg, 0, sizeof(struct regmap_config));
@@ -208,6 +245,7 @@ static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	 * configuration. The notifier enables such kind of handling.
 	 */
 	BLOCKING_INIT_NOTIFIER_HEAD(&lmu->notifier);
+	lmu->backlight_initialized = false;
 
 	i2c_set_clientdata(cl, lmu);
 
diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
index 246ab5145dff..708c79adcabe 100644
--- a/include/linux/mfd/ti-lmu.h
+++ b/include/linux/mfd/ti-lmu.h
@@ -80,6 +80,8 @@ enum lm363x_regulator_id {
  * @en_gpio:	GPIO for HWEN pin [Optional]
  * @pwm:	PWM for module [Optional]
  * @notifier:	Notifier for reporting hwmon event
+ * @id:		Device ID
+ * @backlight_initialized: Global Backlight has been initialized
  */
 struct ti_lmu {
 	struct device *dev;
@@ -87,5 +89,7 @@ struct ti_lmu {
 	struct gpio_desc *en_gpio;
 	struct pwm_device *pwm;
 	struct blocking_notifier_head notifier;
+	enum ti_lmu_id id;
+	bool backlight_initialized;
 };
 #endif
-- 
2.16.2

Re: [PATCHv4 07/10] mfd: ti-lmu: register one backlight device per channel

[Lee Jones]

On Fri, 30 Mar 2018, Sebastian Reichel wrote:

> All LMU devices support multiple channels, that can be controlled
> independently. This registers one backlight sub-device per channel.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/mfd/ti-lmu.c       | 62 +++++++++++++++++++++++++++++++++++++---------
>  include/linux/mfd/ti-lmu.h |  4 +++
>  2 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
> index f43b8acc30e1..f4311d215dfa 100644
> --- a/drivers/mfd/ti-lmu.c
> +++ b/drivers/mfd/ti-lmu.c
> @@ -56,8 +56,16 @@ static void ti_lmu_disable_hw(void *data)
>  
>  static const struct mfd_cell lm3532_devices[] = {
>  	{
> -		.name          = "ti-lmu-backlight",
> -		.id            = LM3532,
> +		.name          = "ti-lmu-led-backlight",
> +		.id            = 0,

What's the reason for you manually setting .id?

> +	},
> +	{
> +		.name          = "ti-lmu-led-backlight",
> +		.id            = 1,
> +	},
> +	{
> +		.name          = "ti-lmu-led-backlight",
> +		.id            = 2,
>  	},
>  };

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCHv4 08/10] backlight: add TI LMU backlight driver

[Sebastian Reichel]

This adds backlight support for the following TI LMU
chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.

Signed-off-by: Milo Kim <milo.kim@ti.com>
[add LED subsystem support for keyboard backlight and rework DT
 binding according to Rob Herrings feedback]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/video/backlight/Kconfig                 |   7 +
 drivers/video/backlight/Makefile                |   3 +
 drivers/video/backlight/ti-lmu-backlight-core.c | 666 ++++++++++++++++++++++++
 drivers/video/backlight/ti-lmu-backlight-data.c | 304 +++++++++++
 drivers/video/backlight/ti-lmu-backlight-data.h |  95 ++++
 5 files changed, 1075 insertions(+)
 create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
 create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
 create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 5d2d0d7e8100..27e6c5a0add8 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -427,6 +427,13 @@ config BACKLIGHT_SKY81452
 	  To compile this driver as a module, choose M here: the module will
 	  be called sky81452-backlight
 
+config BACKLIGHT_TI_LMU
+	tristate "Backlight driver for TI LMU"
+	depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
+	help
+	  Say Y to enable the backlight driver for TI LMU devices.
+	  This supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
+
 config BACKLIGHT_TPS65217
 	tristate "TPS65217 Backlight"
 	depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 19da71d518bf..a1132d3dfd4c 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -53,6 +53,9 @@ obj-$(CONFIG_BACKLIGHT_PM8941_WLED)	+= pm8941-wled.o
 obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
 obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
 obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
+ti-lmu-backlight-objs			:= ti-lmu-backlight-core.o \
+					   ti-lmu-backlight-data.o
+obj-$(CONFIG_BACKLIGHT_TI_LMU)		+= ti-lmu-backlight.o
 obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
 obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
 obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
diff --git a/drivers/video/backlight/ti-lmu-backlight-core.c b/drivers/video/backlight/ti-lmu-backlight-core.c
new file mode 100644
index 000000000000..a6099581edd7
--- /dev/null
+++ b/drivers/video/backlight/ti-lmu-backlight-core.c
@@ -0,0 +1,666 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2015 Texas Instruments
+ * Copyright 2018 Sebastian Reichel
+ *
+ * TI LMU Backlight driver, based on previous work from
+ * Milo Kim <milo.kim@ti.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include "ti-lmu-backlight-data.h"
+
+enum ti_lmu_bl_ctrl_mode {
+	BL_REGISTER_BASED,
+	BL_PWM_BASED,
+};
+
+enum ti_lmu_bl_type {
+	TI_LMU_BL,  /* backlight userspace interface */
+	TI_LMU_LED, /* led userspace interface */
+};
+
+enum ti_lmu_bl_ramp_mode {
+	BL_RAMP_UP,
+	BL_RAMP_DOWN,
+};
+
+#define DEFAULT_PWM_NAME			"lmu-backlight"
+#define NUM_DUAL_CHANNEL			2
+#define LMU_BACKLIGHT_DUAL_CHANNEL_USED		(BIT(0) | BIT(1))
+#define LMU_BACKLIGHT_11BIT_LSB_MASK		(BIT(0) | BIT(1) | BIT(2))
+#define LMU_BACKLIGHT_11BIT_MSB_SHIFT		3
+
+struct ti_lmu_bank {
+	struct device *dev;
+	int bank_id;
+	const struct ti_lmu_bl_cfg *cfg;
+	struct ti_lmu *lmu;
+	const char *label;
+	int leds;
+	int current_brightness;
+	u32 default_brightness;
+	u32 ramp_up_msec;
+	u32 ramp_down_msec;
+	u32 pwm_period;
+	enum ti_lmu_bl_ctrl_mode mode;
+	enum ti_lmu_bl_type type;
+
+	struct notifier_block nb;
+
+	struct backlight_device *backlight;
+	struct led_classdev *led;
+};
+
+static int ti_lmu_bl_enable(struct ti_lmu_bank *lmu_bank, bool enable)
+{
+	struct regmap *regmap = lmu_bank->lmu->regmap;
+	unsigned long enable_time = lmu_bank->cfg->reginfo->enable_usec;
+	u8 *reg = lmu_bank->cfg->reginfo->enable;
+	u8 mask = BIT(lmu_bank->bank_id);
+	u8 val = (enable == true) ? mask : 0;
+	int ret;
+
+	if (!reg)
+		return -EINVAL;
+
+	ret = regmap_update_bits(regmap, *reg, mask, val);
+	if (ret)
+		return ret;
+
+	if (enable_time > 0)
+		usleep_range(enable_time, enable_time + 100);
+
+	return 0;
+}
+
+static int ti_lmu_bl_update_brightness_register(struct ti_lmu_bank *lmu_bank,
+						       int brightness)
+{
+	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
+	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
+	struct regmap *regmap = lmu_bank->lmu->regmap;
+	u8 reg, val;
+	int ret;
+
+	/*
+	 * Brightness register update
+	 *
+	 * 11 bit dimming: update LSB bits and write MSB byte.
+	 *		   MSB brightness should be shifted.
+	 *  8 bit dimming: write MSB byte.
+	 */
+	if (cfg->max_brightness == MAX_BRIGHTNESS_11BIT) {
+		reg = reginfo->brightness_lsb[lmu_bank->bank_id];
+		ret = regmap_update_bits(regmap, reg,
+					 LMU_BACKLIGHT_11BIT_LSB_MASK,
+					 brightness);
+		if (ret)
+			return ret;
+
+		val = brightness >> LMU_BACKLIGHT_11BIT_MSB_SHIFT;
+	} else {
+		val = brightness;
+	}
+
+	reg = reginfo->brightness_msb[lmu_bank->bank_id];
+	return regmap_write(regmap, reg, val);
+}
+
+static int ti_lmu_bl_pwm_ctrl(struct ti_lmu_bank *lmu_bank, int brightness)
+{
+	int max_brightness = lmu_bank->cfg->max_brightness;
+	struct pwm_state state = { };
+	int ret;
+
+	if (!lmu_bank->lmu->pwm) {
+		dev_err(lmu_bank->dev, "Missing PWM device!\n");
+		return -ENODEV;
+	}
+
+	pwm_init_state(lmu_bank->lmu->pwm, &state);
+	state.period = lmu_bank->pwm_period;
+	state.duty_cycle = brightness * state.period / max_brightness;
+
+	if (state.duty_cycle)
+		state.enabled = true;
+	else
+		state.enabled = false;
+
+	ret = pwm_apply_state(lmu_bank->lmu->pwm, &state);
+	if (ret)
+		dev_err(lmu_bank->dev, "Failed to configure PWM: %d", ret);
+
+	return ret;
+}
+
+static int ti_lmu_bl_set_brightness(struct ti_lmu_bank *lmu_bank,
+				    int brightness)
+{
+	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
+	bool enable = brightness > 0;
+	int ret;
+
+	ret = ti_lmu_bl_enable(lmu_bank, enable);
+	if (ret)
+		return ret;
+
+	if (lmu_bank->mode == BL_PWM_BASED) {
+		ti_lmu_bl_pwm_ctrl(lmu_bank, brightness);
+
+		switch (cfg->pwm_action) {
+		case UPDATE_PWM_ONLY:
+			/* No register update is required */
+			return 0;
+		case UPDATE_MAX_BRT:
+			/*
+			 * PWM can start from any non-zero code and dim down
+			 * to zero. So, brightness register should be updated
+			 * even in PWM mode.
+			 */
+			if (brightness > 0)
+				brightness = MAX_BRIGHTNESS_11BIT;
+			else
+				brightness = 0;
+			break;
+		default:
+			break;
+		}
+	}
+
+	lmu_bank->current_brightness = brightness;
+
+	return ti_lmu_bl_update_brightness_register(lmu_bank, brightness);
+}
+
+static int ti_lmu_bl_update_status(struct backlight_device *bl_dev)
+{
+	struct ti_lmu_bank *lmu_bank = bl_get_data(bl_dev);
+	int brightness = bl_dev->props.brightness;
+
+	if (bl_dev->props.state & BL_CORE_SUSPENDED)
+		brightness = 0;
+
+	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
+}
+
+static const struct backlight_ops lmu_backlight_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = ti_lmu_bl_update_status,
+};
+
+static int ti_lmu_bl_set_led_blocking(struct led_classdev *ledc,
+					     enum led_brightness value)
+{
+	struct ti_lmu_bank *lmu_bank = dev_get_drvdata(ledc->dev->parent);
+	int brightness = value;
+
+	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
+}
+
+static int ti_lmu_bl_check_channel(struct ti_lmu_bank *lmu_bank)
+{
+	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
+	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
+
+	if (!reginfo->brightness_msb)
+		return -EINVAL;
+
+	if (cfg->max_brightness > MAX_BRIGHTNESS_8BIT) {
+		if (!reginfo->brightness_lsb)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ti_lmu_bl_create_channel(struct ti_lmu_bank *lmu_bank)
+{
+	struct regmap *regmap = lmu_bank->lmu->regmap;
+	const struct lmu_bl_reg_data *regdata = lmu_bank->cfg->reginfo->channel;
+	int num_channels = lmu_bank->cfg->num_channels;
+	unsigned long led_sources = lmu_bank->leds;
+	int i, ret;
+	u8 shift;
+
+	/*
+	 * How to create backlight output channels:
+	 *   Check 'led_sources' bit and update registers.
+	 *
+	 *   1) Dual channel configuration
+	 *     The 1st register data is used for single channel.
+	 *     The 2nd register data is used for dual channel.
+	 *
+	 *   2) Multiple channel configuration
+	 *     Each register data is mapped to bank ID.
+	 *     Bit shift operation is defined in channel registers.
+	 *
+	 * Channel register data consists of address, mask, value.
+	 */
+
+	if (num_channels == NUM_DUAL_CHANNEL) {
+		if (led_sources == LMU_BACKLIGHT_DUAL_CHANNEL_USED)
+			regdata++;
+
+		return regmap_update_bits(regmap, regdata->reg, regdata->mask,
+					  regdata->val);
+	}
+
+	for (i = 0; regdata && i < num_channels; i++) {
+		/*
+		 * Note that the result of regdata->val is shift bit.
+		 * The bank_id should be shifted for the channel configuration.
+		 */
+		if (test_bit(i, &led_sources)) {
+			shift = regdata->val;
+			ret = regmap_update_bits(regmap, regdata->reg,
+						 regdata->mask,
+						 lmu_bank->bank_id << shift);
+			if (ret)
+				return ret;
+		}
+
+		regdata++;
+	}
+
+	return 0;
+}
+
+static int ti_lmu_bl_update_ctrl_mode(struct ti_lmu_bank *lmu_bank)
+{
+	struct regmap *regmap = lmu_bank->lmu->regmap;
+	const struct lmu_bl_reg_data *regdata =
+		lmu_bank->cfg->reginfo->mode + lmu_bank->bank_id;
+	u8 val = regdata->val;
+
+	if (!regdata)
+		return 0;
+
+	/*
+	 * Update PWM configuration register.
+	 * If the mode is register based, then clear the bit.
+	 */
+	if (lmu_bank->mode != BL_PWM_BASED)
+		val = 0;
+
+	return regmap_update_bits(regmap, regdata->reg, regdata->mask, val);
+}
+
+static int ti_lmu_bl_convert_ramp_to_index(struct ti_lmu_bank *lmu_bank,
+						  enum ti_lmu_bl_ramp_mode mode)
+{
+	const int *ramp_table = lmu_bank->cfg->ramp_table;
+	const int size = lmu_bank->cfg->size_ramp;
+	unsigned int msec;
+	int i;
+
+	if (!ramp_table)
+		return -EINVAL;
+
+	switch (mode) {
+	case BL_RAMP_UP:
+		msec = lmu_bank->ramp_up_msec;
+		break;
+	case BL_RAMP_DOWN:
+		msec = lmu_bank->ramp_down_msec;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (msec <= ramp_table[0])
+		return 0;
+
+	if (msec > ramp_table[size - 1])
+		return size - 1;
+
+	for (i = 1; i < size; i++) {
+		if (msec == ramp_table[i])
+			return i;
+
+		/* Find an approximate index by looking up the table */
+		if (msec > ramp_table[i - 1] && msec < ramp_table[i]) {
+			if (msec - ramp_table[i - 1] < ramp_table[i] - msec)
+				return i - 1;
+			else
+				return i;
+		}
+	}
+
+	return -EINVAL;
+}
+
+
+static int ti_lmu_bl_set_ramp(struct ti_lmu_bank *lmu_bank)
+{
+	struct regmap *regmap = lmu_bank->lmu->regmap;
+	const struct ti_lmu_bl_reg *reginfo = lmu_bank->cfg->reginfo;
+	int offset = reginfo->ramp_reg_offset;
+	int i, ret, index;
+	struct lmu_bl_reg_data regdata;
+
+	for (i = BL_RAMP_UP; i <= BL_RAMP_DOWN; i++) {
+		index = ti_lmu_bl_convert_ramp_to_index(lmu_bank, i);
+		if (index > 0) {
+			if (!reginfo->ramp)
+				break;
+
+			regdata = reginfo->ramp[i];
+			if (lmu_bank->bank_id != 0)
+				regdata.val += offset;
+
+			/* regdata.val is shift bit */
+			ret = regmap_update_bits(regmap, regdata.reg,
+						 regdata.mask,
+						 index << regdata.val);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ti_lmu_bl_configure(struct ti_lmu_bank *lmu_bank)
+{
+	int ret;
+
+	ret = ti_lmu_bl_check_channel(lmu_bank);
+	if (ret)
+		return ret;
+
+	ret = ti_lmu_bl_create_channel(lmu_bank);
+	if (ret)
+		return ret;
+
+	ret = ti_lmu_bl_update_ctrl_mode(lmu_bank);
+	if (ret)
+		return ret;
+
+	return ti_lmu_bl_set_ramp(lmu_bank);
+}
+
+static int ti_lmu_bl_register_backlight(struct ti_lmu_bank *lmu_bank)
+{
+	struct backlight_device *bl_dev;
+	struct backlight_properties props;
+
+	if (lmu_bank->type != TI_LMU_BL)
+		return -EINVAL;
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.type = BACKLIGHT_PLATFORM;
+	props.brightness = lmu_bank->default_brightness;
+	props.max_brightness = lmu_bank->cfg->max_brightness;
+
+	bl_dev = devm_backlight_device_register(lmu_bank->dev, lmu_bank->label,
+						lmu_bank->dev, lmu_bank,
+						&lmu_backlight_ops, &props);
+	if (IS_ERR(bl_dev))
+		return PTR_ERR(bl_dev);
+
+	lmu_bank->backlight = bl_dev;
+
+	return 0;
+}
+
+static int ti_lmu_bl_register_led(struct ti_lmu_bank *lmu_bank)
+{
+	int err;
+
+	if (lmu_bank->type != TI_LMU_LED)
+		return -EINVAL;
+
+	lmu_bank->led = devm_kzalloc(lmu_bank->dev, sizeof(*lmu_bank->led),
+				     GFP_KERNEL);
+	if (!lmu_bank->led)
+		return -ENOMEM;
+
+	lmu_bank->led->name = lmu_bank->label;
+	lmu_bank->led->max_brightness = lmu_bank->cfg->max_brightness;
+	lmu_bank->led->brightness_set_blocking =
+		ti_lmu_bl_set_led_blocking;
+
+	err = devm_led_classdev_register(lmu_bank->dev, lmu_bank->led);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
+{
+	switch (lmu_bank->type) {
+	case TI_LMU_BL:
+		return ti_lmu_bl_register_backlight(lmu_bank);
+	case TI_LMU_LED:
+		return ti_lmu_bl_register_led(lmu_bank);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int setup_of_node(struct platform_device *pdev)
+{
+	struct device_node *parent_node = pdev->dev.parent->of_node;
+	char *name;
+
+	if (!parent_node)
+		return 0;
+
+	name = kasprintf(GFP_KERNEL, "bank%d", pdev->id);
+	if (!name)
+		return -ENOMEM;
+
+	pdev->dev.of_node = of_get_child_by_name(parent_node, name);
+	kfree(name);
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	return 0;
+}
+
+static int ti_lmu_parse_led_sources(struct device *dev)
+{
+	unsigned long mask = 0;
+	int ret;
+	int size, i;
+	u32 *leds;
+
+	size = device_property_read_u32_array(dev, "ti,led-sources", NULL, 0);
+	if (size <= 0) {
+		dev_err(dev, "Missing or malformed property led-sources: %d\n",
+			size);
+		return size < 0 ? size : -EINVAL;
+	}
+
+	leds = kmalloc_array(size, sizeof(u32), GFP_KERNEL);
+	if (!leds)
+		return -ENOMEM;
+
+	ret = device_property_read_u32_array(dev, "ti,led-sources", leds, size);
+	if (ret) {
+		dev_err(dev, "Failed to read led-sources property: %d\n", ret);
+		goto out;
+	}
+
+	for (i = 0; i < size; i++)
+		set_bit(leds[i], &mask);
+
+	ret = mask;
+
+out:
+	kfree(leds);
+	return ret;
+}
+
+static int ti_lmu_bl_init(struct ti_lmu_bank *lmu_bank)
+{
+	struct regmap *regmap = lmu_bank->lmu->regmap;
+	const struct lmu_bl_reg_data *regdata =
+		lmu_bank->cfg->reginfo->init;
+	int num_init = lmu_bank->cfg->reginfo->num_init;
+	int i, ret;
+
+	if (lmu_bank->lmu->backlight_initialized)
+		return 0;
+	lmu_bank->lmu->backlight_initialized = true;
+
+	for (i = 0; regdata && i < num_init; i++) {
+		ret = regmap_update_bits(regmap, regdata->reg, regdata->mask,
+					 regdata->val);
+		if (ret)
+			return ret;
+
+		regdata++;
+	}
+
+	return 0;
+}
+
+static int ti_lmu_bl_reload(struct ti_lmu_bank *lmu_bank)
+{
+	int err;
+
+	ti_lmu_bl_init(lmu_bank);
+
+	err = ti_lmu_bl_configure(lmu_bank);
+	if (err)
+		return err;
+
+	return ti_lmu_bl_set_brightness(lmu_bank, lmu_bank->current_brightness);
+}
+
+static int ti_lmu_bl_monitor_notifier(struct notifier_block *nb,
+					     unsigned long action, void *unused)
+{
+	struct ti_lmu_bank *lmu_bank = container_of(nb, struct ti_lmu_bank, nb);
+
+	if (action == LMU_EVENT_MONITOR_DONE) {
+		if (ti_lmu_bl_reload(lmu_bank))
+			return NOTIFY_STOP;
+	}
+
+	return NOTIFY_OK;
+}
+
+static int ti_lmu_bl_probe(struct platform_device *pdev)
+{
+	struct ti_lmu_bank *lmu_bank;
+	int err;
+
+	err = setup_of_node(pdev);
+	if (err)
+		return err;
+
+	lmu_bank = devm_kzalloc(&pdev->dev, sizeof(*lmu_bank), GFP_KERNEL);
+	if (!lmu_bank)
+		return -ENOMEM;
+	lmu_bank->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, lmu_bank);
+
+	err = device_property_read_string(&pdev->dev, "label",
+					  &lmu_bank->label);
+	if (err)
+		return err;
+
+	lmu_bank->type = TI_LMU_BL;
+	if (!strcmp(lmu_bank->label, "keyboard")) {
+		lmu_bank->type = TI_LMU_LED;
+		lmu_bank->label = "kbd_backlight";
+	}
+
+	lmu_bank->leds = ti_lmu_parse_led_sources(&pdev->dev);
+	if (lmu_bank->leds < 0)
+		return lmu_bank->leds;
+	else if (lmu_bank->leds == 0)
+		return -EINVAL;
+
+	device_property_read_u32(&pdev->dev, "default-brightness-level",
+				 &lmu_bank->default_brightness);
+	device_property_read_u32(&pdev->dev, "ti,ramp-up-ms",
+				 &lmu_bank->ramp_up_msec);
+	device_property_read_u32(&pdev->dev, "ti,ramp-down-ms",
+				 &lmu_bank->ramp_down_msec);
+	device_property_read_u32(&pdev->dev, "pwm-period",
+				 &lmu_bank->pwm_period);
+
+	if (lmu_bank->pwm_period > 0)
+		lmu_bank->mode = BL_PWM_BASED;
+	else
+		lmu_bank->mode = BL_REGISTER_BASED;
+
+	lmu_bank->lmu = dev_get_drvdata(pdev->dev.parent);
+	lmu_bank->cfg = &lmu_bl_cfg[lmu_bank->lmu->id];
+	lmu_bank->bank_id = pdev->id;
+
+	ti_lmu_bl_init(lmu_bank);
+
+	err = ti_lmu_bl_configure(lmu_bank);
+	if (err)
+		return err;
+
+	err = ti_lmu_bl_add_device(lmu_bank);
+	if (err)
+		return err;
+
+	err = ti_lmu_bl_set_brightness(lmu_bank,
+					      lmu_bank->default_brightness);
+	if (err)
+		return err;
+
+	/*
+	 * Notifier callback is required because backlight device needs
+	 * reconfiguration after fault detection procedure is done by
+	 * ti-lmu-fault-monitor driver.
+	 */
+	if (lmu_bank->cfg->fault_monitor_used) {
+		lmu_bank->nb.notifier_call = ti_lmu_bl_monitor_notifier;
+		err = blocking_notifier_chain_register(&lmu_bank->lmu->notifier,
+						       &lmu_bank->nb);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int ti_lmu_bl_remove(struct platform_device *pdev)
+{
+	struct ti_lmu_bank *lmu_bank = platform_get_drvdata(pdev);
+
+	if (lmu_bank->cfg->fault_monitor_used)
+		blocking_notifier_chain_unregister(&lmu_bank->lmu->notifier,
+						   &lmu_bank->nb);
+
+	ti_lmu_bl_set_brightness(lmu_bank, 0);
+
+	return 0;
+}
+
+static struct platform_driver ti_lmu_bl_driver = {
+	.probe  = ti_lmu_bl_probe,
+	.remove  = ti_lmu_bl_remove,
+	.driver = {
+		.name = "ti-lmu-led-backlight",
+	},
+};
+module_platform_driver(ti_lmu_bl_driver)
+
+MODULE_DESCRIPTION("TI LMU Backlight LED Driver");
+MODULE_AUTHOR("Sebastian Reichel");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:ti-lmu-led-backlight");
diff --git a/drivers/video/backlight/ti-lmu-backlight-data.c b/drivers/video/backlight/ti-lmu-backlight-data.c
new file mode 100644
index 000000000000..583136cb934d
--- /dev/null
+++ b/drivers/video/backlight/ti-lmu-backlight-data.c
@@ -0,0 +1,304 @@
+/*
+ * TI LMU (Lighting Management Unit) Backlight Device Data
+ *
+ * Copyright 2015 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "ti-lmu-backlight-data.h"
+
+/* LM3532 */
+static const struct lmu_bl_reg_data lm3532_init_data[] = {
+	{ LM3532_REG_ZONE_CFG_A, LM3532_ZONE_MASK, LM3532_ZONE_0 },
+	{ LM3532_REG_ZONE_CFG_B, LM3532_ZONE_MASK, LM3532_ZONE_1 },
+	{ LM3532_REG_ZONE_CFG_C, LM3532_ZONE_MASK, LM3532_ZONE_2 },
+};
+
+static const struct lmu_bl_reg_data lm3532_channel_data[] = {
+	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED1_CFG_MASK,
+	  LM3532_ILED1_CFG_SHIFT },
+	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED2_CFG_MASK,
+	  LM3532_ILED2_CFG_SHIFT },
+	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED3_CFG_MASK,
+	  LM3532_ILED3_CFG_SHIFT },
+};
+
+static const struct lmu_bl_reg_data lm3532_mode_data[] = {
+	{ LM3532_REG_PWM_A_CFG, LM3532_PWM_A_MASK, LM3532_PWM_ZONE_0 },
+	{ LM3532_REG_PWM_B_CFG, LM3532_PWM_B_MASK, LM3532_PWM_ZONE_1 },
+	{ LM3532_REG_PWM_C_CFG, LM3532_PWM_C_MASK, LM3532_PWM_ZONE_2 },
+};
+
+static const struct lmu_bl_reg_data lm3532_ramp_data[] = {
+	{ LM3532_REG_RAMPUP, LM3532_RAMPUP_MASK, LM3532_RAMPUP_SHIFT },
+	{ LM3532_REG_RAMPDN, LM3532_RAMPDN_MASK, LM3532_RAMPDN_SHIFT },
+};
+
+static u8 lm3532_enable_reg = LM3532_REG_ENABLE;
+
+static u8 lm3532_brightness_regs[] = {
+	LM3532_REG_BRT_A,
+	LM3532_REG_BRT_B,
+	LM3532_REG_BRT_C,
+};
+
+static const struct ti_lmu_bl_reg lm3532_reg_info = {
+	.init		= lm3532_init_data,
+	.num_init	= ARRAY_SIZE(lm3532_init_data),
+	.channel	= lm3532_channel_data,
+	.mode		= lm3532_mode_data,
+	.ramp		= lm3532_ramp_data,
+	.enable		= &lm3532_enable_reg,
+	.brightness_msb	= lm3532_brightness_regs,
+};
+
+/* LM3631 */
+static const struct lmu_bl_reg_data lm3631_init_data[] = {
+	{ LM3631_REG_BRT_MODE, LM3631_MODE_MASK, LM3631_DEFAULT_MODE },
+	{ LM3631_REG_BL_CFG, LM3631_MAP_MASK, LM3631_EXPONENTIAL_MAP },
+};
+
+static const struct lmu_bl_reg_data lm3631_channel_data[] = {
+	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_SINGLE_CHANNEL },
+	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_DUAL_CHANNEL },
+};
+
+static const struct lmu_bl_reg_data lm3631_ramp_data[] = {
+	{ LM3631_REG_SLOPE, LM3631_SLOPE_MASK, LM3631_SLOPE_SHIFT },
+};
+
+static u8 lm3631_enable_reg = LM3631_REG_DEVCTRL;
+static u8 lm3631_brightness_msb_reg = LM3631_REG_BRT_MSB;
+static u8 lm3631_brightness_lsb_reg = LM3631_REG_BRT_LSB;
+
+static const struct ti_lmu_bl_reg lm3631_reg_info = {
+	.init		= lm3631_init_data,
+	.num_init	= ARRAY_SIZE(lm3631_init_data),
+	.channel	= lm3631_channel_data,
+	.ramp		= lm3631_ramp_data,
+	.enable		= &lm3631_enable_reg,
+	.brightness_msb	= &lm3631_brightness_msb_reg,
+	.brightness_lsb	= &lm3631_brightness_lsb_reg,
+};
+
+/* LM3632 */
+static const struct lmu_bl_reg_data lm3632_init_data[] = {
+	{ LM3632_REG_CONFIG1, LM3632_OVP_MASK, LM3632_OVP_25V },
+	{ LM3632_REG_CONFIG2, LM3632_SWFREQ_MASK, LM3632_SWFREQ_1MHZ },
+};
+
+static const struct lmu_bl_reg_data lm3632_channel_data[] = {
+	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_SINGLE_CHANNEL },
+	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_DUAL_CHANNEL },
+};
+
+static const struct lmu_bl_reg_data lm3632_mode_data[] = {
+	{ LM3632_REG_IO_CTRL, LM3632_PWM_MASK, LM3632_PWM_MODE },
+};
+
+static u8 lm3632_enable_reg = LM3632_REG_ENABLE;
+static u8 lm3632_brightness_msb_reg = LM3632_REG_BRT_MSB;
+static u8 lm3632_brightness_lsb_reg = LM3632_REG_BRT_LSB;
+
+static const struct ti_lmu_bl_reg lm3632_reg_info = {
+	.init		= lm3632_init_data,
+	.num_init	= ARRAY_SIZE(lm3632_init_data),
+	.channel	= lm3632_channel_data,
+	.mode		= lm3632_mode_data,
+	.enable		= &lm3632_enable_reg,
+	.brightness_msb	= &lm3632_brightness_msb_reg,
+	.brightness_lsb	= &lm3632_brightness_lsb_reg,
+};
+
+/* LM3633 */
+static const struct lmu_bl_reg_data lm3633_init_data[] = {
+	{ LM3633_REG_BOOST_CFG, LM3633_OVP_MASK, LM3633_OVP_40V },
+	{ LM3633_REG_BL_RAMP_CONF, LM3633_BL_RAMP_MASK, LM3633_BL_RAMP_EACH },
+};
+
+static const struct lmu_bl_reg_data lm3633_channel_data[] = {
+	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED1_CFG_MASK,
+	  LM3633_HVLED1_CFG_SHIFT },
+	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED2_CFG_MASK,
+	  LM3633_HVLED2_CFG_SHIFT },
+	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED3_CFG_MASK,
+	  LM3633_HVLED3_CFG_SHIFT },
+};
+
+static const struct lmu_bl_reg_data lm3633_mode_data[] = {
+	{ LM3633_REG_PWM_CFG, LM3633_PWM_A_MASK, LM3633_PWM_A_MASK },
+	{ LM3633_REG_PWM_CFG, LM3633_PWM_B_MASK, LM3633_PWM_B_MASK },
+};
+
+static const struct lmu_bl_reg_data lm3633_ramp_data[] = {
+	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPUP_MASK, LM3633_BL_RAMPUP_SHIFT },
+	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPDN_MASK, LM3633_BL_RAMPDN_SHIFT },
+};
+
+static u8 lm3633_enable_reg = LM3633_REG_ENABLE;
+
+static u8 lm3633_brightness_msb_regs[] = {
+	LM3633_REG_BRT_HVLED_A_MSB,
+	LM3633_REG_BRT_HVLED_B_MSB,
+};
+
+static u8 lm3633_brightness_lsb_regs[] = {
+	LM3633_REG_BRT_HVLED_A_LSB,
+	LM3633_REG_BRT_HVLED_B_LSB,
+};
+
+static const struct ti_lmu_bl_reg lm3633_reg_info = {
+	.init		 = lm3633_init_data,
+	.num_init	 = ARRAY_SIZE(lm3633_init_data),
+	.channel	 = lm3633_channel_data,
+	.mode		 = lm3633_mode_data,
+	.ramp		 = lm3633_ramp_data,
+	.ramp_reg_offset = 1, /* For LM3633_REG_BL1_RAMPUP/DN */
+	.enable		 = &lm3633_enable_reg,
+	.brightness_msb	 = lm3633_brightness_msb_regs,
+	.brightness_lsb	 = lm3633_brightness_lsb_regs,
+};
+
+/* LM3695 */
+static const struct lmu_bl_reg_data lm3695_init_data[] = {
+	{ LM3695_REG_GP, LM3695_BRT_RW_MASK, LM3695_BRT_RW_MASK },
+};
+
+static const struct lmu_bl_reg_data lm3695_channel_data[] = {
+	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_SINGLE_CHANNEL },
+	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_DUAL_CHANNEL },
+};
+
+static u8 lm3695_enable_reg = LM3695_REG_GP;
+static u8 lm3695_brightness_msb_reg = LM3695_REG_BRT_MSB;
+static u8 lm3695_brightness_lsb_reg = LM3695_REG_BRT_LSB;
+
+static const struct ti_lmu_bl_reg lm3695_reg_info = {
+	.init		= lm3695_init_data,
+	.num_init	= ARRAY_SIZE(lm3695_init_data),
+	.channel	= lm3695_channel_data,
+	.enable		= &lm3695_enable_reg,
+	.enable_usec	= 600,
+	.brightness_msb	= &lm3695_brightness_msb_reg,
+	.brightness_lsb	= &lm3695_brightness_lsb_reg,
+};
+
+/* LM3697 */
+static const struct lmu_bl_reg_data lm3697_init_data[] = {
+	{ LM3697_REG_RAMP_CONF, LM3697_RAMP_MASK, LM3697_RAMP_EACH },
+};
+
+static const struct lmu_bl_reg_data lm3697_channel_data[] = {
+	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED1_CFG_MASK,
+	  LM3697_HVLED1_CFG_SHIFT },
+	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED2_CFG_MASK,
+	  LM3697_HVLED2_CFG_SHIFT },
+	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED3_CFG_MASK,
+	  LM3697_HVLED3_CFG_SHIFT },
+};
+
+static const struct lmu_bl_reg_data lm3697_mode_data[] = {
+	{ LM3697_REG_PWM_CFG, LM3697_PWM_A_MASK, LM3697_PWM_A_MASK },
+	{ LM3697_REG_PWM_CFG, LM3697_PWM_B_MASK, LM3697_PWM_B_MASK },
+};
+
+static const struct lmu_bl_reg_data lm3697_ramp_data[] = {
+	{ LM3697_REG_BL0_RAMP, LM3697_RAMPUP_MASK, LM3697_RAMPUP_SHIFT },
+	{ LM3697_REG_BL0_RAMP, LM3697_RAMPDN_MASK, LM3697_RAMPDN_SHIFT },
+};
+
+static u8 lm3697_enable_reg = LM3697_REG_ENABLE;
+
+static u8 lm3697_brightness_msb_regs[] = {
+	LM3697_REG_BRT_A_MSB,
+	LM3697_REG_BRT_B_MSB,
+};
+
+static u8 lm3697_brightness_lsb_regs[] = {
+	LM3697_REG_BRT_A_LSB,
+	LM3697_REG_BRT_B_LSB,
+};
+
+static const struct ti_lmu_bl_reg lm3697_reg_info = {
+	.init		 = lm3697_init_data,
+	.num_init	 = ARRAY_SIZE(lm3697_init_data),
+	.channel	 = lm3697_channel_data,
+	.mode		 = lm3697_mode_data,
+	.ramp		 = lm3697_ramp_data,
+	.ramp_reg_offset = 1, /* For LM3697_REG_BL1_RAMPUP/DN */
+	.enable		 = &lm3697_enable_reg,
+	.brightness_msb	 = lm3697_brightness_msb_regs,
+	.brightness_lsb	 = lm3697_brightness_lsb_regs,
+};
+
+static int lm3532_ramp_table[] = { 0, 1, 2, 4, 8, 16, 32, 65 };
+
+static int lm3631_ramp_table[] = {
+	   0,   1,   2,    5,   10,   20,   50,  100,
+	 250, 500, 750, 1000, 1500, 2000, 3000, 4000,
+};
+
+static int common_ramp_table[] = {
+	   2, 250, 500, 1000, 2000, 4000, 8000, 16000,
+};
+
+#define LM3532_MAX_CHANNELS		3
+#define LM3631_MAX_CHANNELS		2
+#define LM3632_MAX_CHANNELS		2
+#define LM3633_MAX_CHANNELS		3
+#define LM3695_MAX_CHANNELS		2
+#define LM3697_MAX_CHANNELS		3
+
+const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID] = {
+	{
+		.reginfo		= &lm3532_reg_info,
+		.num_channels		= LM3532_MAX_CHANNELS,
+		.max_brightness		= MAX_BRIGHTNESS_8BIT,
+		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
+		.ramp_table		= lm3532_ramp_table,
+		.size_ramp		= ARRAY_SIZE(lm3532_ramp_table),
+	},
+	{
+		.reginfo		= &lm3631_reg_info,
+		.num_channels		= LM3631_MAX_CHANNELS,
+		.max_brightness		= MAX_BRIGHTNESS_11BIT,
+		.pwm_action		= UPDATE_PWM_ONLY,
+		.ramp_table		= lm3631_ramp_table,
+		.size_ramp		= ARRAY_SIZE(lm3631_ramp_table),
+	},
+	{
+		.reginfo		= &lm3632_reg_info,
+		.num_channels		= LM3632_MAX_CHANNELS,
+		.max_brightness		= MAX_BRIGHTNESS_11BIT,
+		.pwm_action		= UPDATE_PWM_ONLY,
+	},
+	{
+		.reginfo		= &lm3633_reg_info,
+		.num_channels		= LM3633_MAX_CHANNELS,
+		.max_brightness		= MAX_BRIGHTNESS_11BIT,
+		.pwm_action		= UPDATE_MAX_BRT,
+		.ramp_table		= common_ramp_table,
+		.size_ramp		= ARRAY_SIZE(common_ramp_table),
+		.fault_monitor_used	= true,
+	},
+	{
+		.reginfo		= &lm3695_reg_info,
+		.num_channels		= LM3695_MAX_CHANNELS,
+		.max_brightness		= MAX_BRIGHTNESS_11BIT,
+		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
+	},
+	{
+		.reginfo		= &lm3697_reg_info,
+		.num_channels		= LM3697_MAX_CHANNELS,
+		.max_brightness		= MAX_BRIGHTNESS_11BIT,
+		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
+		.ramp_table		= common_ramp_table,
+		.size_ramp		= ARRAY_SIZE(common_ramp_table),
+		.fault_monitor_used	= true,
+	},
+};
diff --git a/drivers/video/backlight/ti-lmu-backlight-data.h b/drivers/video/backlight/ti-lmu-backlight-data.h
new file mode 100644
index 000000000000..c64e8e6513e1
--- /dev/null
+++ b/drivers/video/backlight/ti-lmu-backlight-data.h
@@ -0,0 +1,95 @@
+/*
+ * TI LMU (Lighting Management Unit) Backlight Device Data Definitions
+ *
+ * Copyright 2015 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __TI_LMU_BACKLIGHT_H__
+#define __TI_LMU_BACKLIGHT_H__
+
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+
+#define MAX_BRIGHTNESS_8BIT		255
+#define MAX_BRIGHTNESS_11BIT		2047
+
+enum ti_lmu_bl_pwm_action {
+	/* Update PWM duty, no brightness register update is required */
+	UPDATE_PWM_ONLY,
+	/* Update not only duty but also brightness register */
+	UPDATE_PWM_AND_BRT_REGISTER,
+	/* Update max value in brightness registers */
+	UPDATE_MAX_BRT,
+};
+
+struct lmu_bl_reg_data {
+	u8 reg;
+	u8 mask;
+	u8 val;
+};
+
+/**
+ * struct ti_lmu_bl_reg
+ *
+ * @init:		Device initialization registers
+ * @num_init:		Numbers of initialization registers
+ * @channel:		Backlight channel configuration registers
+ * @mode:		Brightness control mode registers
+ * @ramp:		Ramp registers for lighting effect
+ * @ramp_reg_offset:	Ramp register offset.
+ *			Only used for multiple ramp registers.
+ * @enable:		Enable control register address
+ * @enable_usec:	Delay time for updating enable register.
+ *			Unit is microsecond.
+ * @brightness_msb:	Brightness MSB(Upper 8 bits) registers.
+ *			Concatenated with LSB in 11 bit dimming mode.
+ *			In 8 bit dimming, only MSB is used.
+ * @brightness_lsb:	Brightness LSB(Lower 3 bits) registers.
+ *			Only valid in 11 bit dimming mode.
+ */
+struct ti_lmu_bl_reg {
+	const struct lmu_bl_reg_data *init;
+	int num_init;
+	const struct lmu_bl_reg_data *channel;
+	const struct lmu_bl_reg_data *mode;
+	const struct lmu_bl_reg_data *ramp;
+	int ramp_reg_offset;
+	u8 *enable;
+	unsigned long enable_usec;
+	u8 *brightness_msb;
+	u8 *brightness_lsb;
+};
+
+/**
+ * struct ti_lmu_bl_cfg
+ *
+ * @reginfo:		Device register configuration
+ * @num_channels:	Number of backlight channels
+ * @max_brightness:	Max brightness value of backlight device
+ * @pwm_action:		How to control brightness registers in PWM mode
+ * @ramp_table:		[Optional] Ramp time table for lighting effect.
+ *			It's used for searching approximate register index.
+ * @size_ramp:		[Optional] Size of ramp table
+ * @fault_monitor_used:	[Optional] Set true if the device needs to handle
+ *			LMU fault monitor event.
+ *
+ * This structure is used for device specific data configuration.
+ */
+struct ti_lmu_bl_cfg {
+	const struct ti_lmu_bl_reg *reginfo;
+	int num_channels;
+	int max_brightness;
+	enum ti_lmu_bl_pwm_action pwm_action;
+	int *ramp_table;
+	int size_ramp;
+	bool fault_monitor_used;
+};
+
+extern const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID];
+#endif
-- 
2.16.2

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

Hi!

> +enum ti_lmu_bl_type {
> +	TI_LMU_BL,  /* backlight userspace interface */
> +	TI_LMU_LED, /* led userspace interface */
> +};
...
> +static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
> +{
> +	switch (lmu_bank->type) {
> +	case TI_LMU_BL:
> +		return ti_lmu_bl_register_backlight(lmu_bank);
> +	case TI_LMU_LED:
> +		return ti_lmu_bl_register_led(lmu_bank);
> +	default:
> +		return -EINVAL;
> +	}
> +}

Ok, this is somehow unusual/crazy. Single driver with two interfaces.

Do we need the LED interface for something?

If yes, I believe reasonable solution would be to always provide LED
interface, and then have "backlight-trigger" which that would provide
backlight interface for arbitrary LED.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

Hi,

On Tue, Apr 03, 2018 at 12:49:08PM +0200, Pavel Machek wrote:
> > +enum ti_lmu_bl_type {
> > +	TI_LMU_BL,  /* backlight userspace interface */
> > +	TI_LMU_LED, /* led userspace interface */
> > +};
> ...
> > +static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	switch (lmu_bank->type) {
> > +	case TI_LMU_BL:
> > +		return ti_lmu_bl_register_backlight(lmu_bank);
> > +	case TI_LMU_LED:
> > +		return ti_lmu_bl_register_led(lmu_bank);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> 
> Ok, this is somehow unusual/crazy. Single driver with two
> interfaces.
> 
> Do we need the LED interface for something?
>
> If yes, I believe reasonable solution would be to always provide LED
> interface, and then have "backlight-trigger" which that would provide
> backlight interface for arbitrary LED.

Userspace expects keyboard backlight to be exposed via the LED
subsystem and display backlight via the backlight subsystem.

I considered always exposing the banks via the LED subsystem and
using a generic backlight driver. That brings its own problems,
since there is a dependency between the display and the backlight.
This is described in DT using a phandle. Getting the right backlight
device from the phandle will become very tricky with this approach.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

Hi!

> > > +static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
> > > +{
> > > +	switch (lmu_bank->type) {
> > > +	case TI_LMU_BL:
> > > +		return ti_lmu_bl_register_backlight(lmu_bank);
> > > +	case TI_LMU_LED:
> > > +		return ti_lmu_bl_register_led(lmu_bank);
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > 
> > Ok, this is somehow unusual/crazy. Single driver with two
> > interfaces.
> > 
> > Do we need the LED interface for something?
> >
> > If yes, I believe reasonable solution would be to always provide LED
> > interface, and then have "backlight-trigger" which that would provide
> > backlight interface for arbitrary LED.
> 
> Userspace expects keyboard backlight to be exposed via the LED
> subsystem and display backlight via the backlight subsystem.

Ok.

> I considered always exposing the banks via the LED subsystem and
> using a generic backlight driver. That brings its own problems,
> since there is a dependency between the display and the backlight.
> This is described in DT using a phandle. Getting the right backlight
> device from the phandle will become very tricky with this approach.

I believe we have to do this.

Virtually any LED can be used as a backlight, and we don't really want
to add two personalities to all the LED drivers.

And it should not be too bad: LED will just have default trigger,
which will say this LED corresponds to this display device. I believe
someone wanted to do that for USB/ethernet activity.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Daniel Thompson]

On Fri, Mar 30, 2018 at 07:24:12PM +0200, Sebastian Reichel wrote:
> This adds backlight support for the following TI LMU
> chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> 
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> [add LED subsystem support for keyboard backlight and rework DT

Milo's mail has be bouncing for a very long time now. Did they really 
sign off this code or is this intended to be an authorship credit?


>  binding according to Rob Herrings feedback]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/video/backlight/Kconfig                 |   7 +
>  drivers/video/backlight/Makefile                |   3 +
>  drivers/video/backlight/ti-lmu-backlight-core.c | 666 ++++++++++++++++++++++++
>  drivers/video/backlight/ti-lmu-backlight-data.c | 304 +++++++++++
>  drivers/video/backlight/ti-lmu-backlight-data.h |  95 ++++
>  5 files changed, 1075 insertions(+)
>  create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
>  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
>  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 5d2d0d7e8100..27e6c5a0add8 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -427,6 +427,13 @@ config BACKLIGHT_SKY81452
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called sky81452-backlight
>  
> +config BACKLIGHT_TI_LMU
> +	tristate "Backlight driver for TI LMU"
> +	depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
> +	help
> +	  Say Y to enable the backlight driver for TI LMU devices.
> +	  This supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> +
>  config BACKLIGHT_TPS65217
>  	tristate "TPS65217 Backlight"
>  	depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 19da71d518bf..a1132d3dfd4c 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -53,6 +53,9 @@ obj-$(CONFIG_BACKLIGHT_PM8941_WLED)	+= pm8941-wled.o
>  obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
>  obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
>  obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
> +ti-lmu-backlight-objs			:= ti-lmu-backlight-core.o \
> +					   ti-lmu-backlight-data.o
> +obj-$(CONFIG_BACKLIGHT_TI_LMU)		+= ti-lmu-backlight.o
>  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
>  obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
>  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
> diff --git a/drivers/video/backlight/ti-lmu-backlight-core.c b/drivers/video/backlight/ti-lmu-backlight-core.c
> new file mode 100644
> index 000000000000..a6099581edd7
> --- /dev/null
> +++ b/drivers/video/backlight/ti-lmu-backlight-core.c
> @@ -0,0 +1,666 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2015 Texas Instruments
> + * Copyright 2018 Sebastian Reichel
> + *
> + * TI LMU Backlight driver, based on previous work from
> + * Milo Kim <milo.kim@ti.com>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/ti-lmu.h>
> +#include <linux/mfd/ti-lmu-register.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +#include "ti-lmu-backlight-data.h"
> +
> +enum ti_lmu_bl_ctrl_mode {
> +	BL_REGISTER_BASED,
> +	BL_PWM_BASED,
> +};
> +
> +enum ti_lmu_bl_type {
> +	TI_LMU_BL,  /* backlight userspace interface */
> +	TI_LMU_LED, /* led userspace interface */
> +};
> +
> +enum ti_lmu_bl_ramp_mode {
> +	BL_RAMP_UP,
> +	BL_RAMP_DOWN,
> +};
> +
> +#define DEFAULT_PWM_NAME			"lmu-backlight"
> +#define NUM_DUAL_CHANNEL			2
> +#define LMU_BACKLIGHT_DUAL_CHANNEL_USED		(BIT(0) | BIT(1))
> +#define LMU_BACKLIGHT_11BIT_LSB_MASK		(BIT(0) | BIT(1) | BIT(2))
> +#define LMU_BACKLIGHT_11BIT_MSB_SHIFT		3
> +
> +struct ti_lmu_bank {
> +	struct device *dev;
> +	int bank_id;
> +	const struct ti_lmu_bl_cfg *cfg;
> +	struct ti_lmu *lmu;
> +	const char *label;
> +	int leds;
> +	int current_brightness;
> +	u32 default_brightness;
> +	u32 ramp_up_msec;
> +	u32 ramp_down_msec;
> +	u32 pwm_period;
> +	enum ti_lmu_bl_ctrl_mode mode;
> +	enum ti_lmu_bl_type type;
> +
> +	struct notifier_block nb;
> +
> +	struct backlight_device *backlight;
> +	struct led_classdev *led;
> +};
> +
> +static int ti_lmu_bl_enable(struct ti_lmu_bank *lmu_bank, bool enable)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	unsigned long enable_time = lmu_bank->cfg->reginfo->enable_usec;
> +	u8 *reg = lmu_bank->cfg->reginfo->enable;
> +	u8 mask = BIT(lmu_bank->bank_id);
> +	u8 val = (enable == true) ? mask : 0;
> +	int ret;
> +
> +	if (!reg)
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(regmap, *reg, mask, val);
> +	if (ret)
> +		return ret;
> +
> +	if (enable_time > 0)
> +		usleep_range(enable_time, enable_time + 100);
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_update_brightness_register(struct ti_lmu_bank *lmu_bank,
> +						       int brightness)
> +{
> +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	u8 reg, val;
> +	int ret;
> +
> +	/*
> +	 * Brightness register update
> +	 *
> +	 * 11 bit dimming: update LSB bits and write MSB byte.
> +	 *		   MSB brightness should be shifted.
> +	 *  8 bit dimming: write MSB byte.
> +	 */
> +	if (cfg->max_brightness == MAX_BRIGHTNESS_11BIT) {
> +		reg = reginfo->brightness_lsb[lmu_bank->bank_id];
> +		ret = regmap_update_bits(regmap, reg,
> +					 LMU_BACKLIGHT_11BIT_LSB_MASK,
> +					 brightness);
> +		if (ret)
> +			return ret;
> +
> +		val = brightness >> LMU_BACKLIGHT_11BIT_MSB_SHIFT;
> +	} else {
> +		val = brightness;
> +	}
> +
> +	reg = reginfo->brightness_msb[lmu_bank->bank_id];
> +	return regmap_write(regmap, reg, val);
> +}
> +
> +static int ti_lmu_bl_pwm_ctrl(struct ti_lmu_bank *lmu_bank, int brightness)
> +{
> +	int max_brightness = lmu_bank->cfg->max_brightness;
> +	struct pwm_state state = { };
> +	int ret;
> +
> +	if (!lmu_bank->lmu->pwm) {
> +		dev_err(lmu_bank->dev, "Missing PWM device!\n");
> +		return -ENODEV;
> +	}
> +
> +	pwm_init_state(lmu_bank->lmu->pwm, &state);
> +	state.period = lmu_bank->pwm_period;
> +	state.duty_cycle = brightness * state.period / max_brightness;
> +
> +	if (state.duty_cycle)
> +		state.enabled = true;
> +	else
> +		state.enabled = false;
> +
> +	ret = pwm_apply_state(lmu_bank->lmu->pwm, &state);
> +	if (ret)
> +		dev_err(lmu_bank->dev, "Failed to configure PWM: %d", ret);
> +
> +	return ret;
> +}
> +
> +static int ti_lmu_bl_set_brightness(struct ti_lmu_bank *lmu_bank,
> +				    int brightness)
> +{
> +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> +	bool enable = brightness > 0;
> +	int ret;
> +
> +	ret = ti_lmu_bl_enable(lmu_bank, enable);
> +	if (ret)
> +		return ret;
> +
> +	if (lmu_bank->mode == BL_PWM_BASED) {
> +		ti_lmu_bl_pwm_ctrl(lmu_bank, brightness);
> +
> +		switch (cfg->pwm_action) {
> +		case UPDATE_PWM_ONLY:
> +			/* No register update is required */
> +			return 0;
> +		case UPDATE_MAX_BRT:
> +			/*
> +			 * PWM can start from any non-zero code and dim down
> +			 * to zero. So, brightness register should be updated
> +			 * even in PWM mode.
> +			 */

This comment could do with a little expansion (I assume the bank's
brightness register means something different when in PWM mode but the
flow of the code is tricky to read).


> +			if (brightness > 0)

Isn't this "enable"?


> +				brightness = MAX_BRIGHTNESS_11BIT;
> +			else
> +				brightness = 0;
> +			break;
> +		default:

case UPDATE_PWM_AND_BRT_REGISTER:


> +			break;
> +		}
> +	}
> +
> +	lmu_bank->current_brightness = brightness;
> +
> +	return ti_lmu_bl_update_brightness_register(lmu_bank, brightness);
> +}
> +
> +static int ti_lmu_bl_update_status(struct backlight_device *bl_dev)
> +{
> +	struct ti_lmu_bank *lmu_bank = bl_get_data(bl_dev);
 > +	int brightness = bl_dev->props.brightness;
> +
> +	if (bl_dev->props.state & BL_CORE_SUSPENDED)
> +		brightness = 0;
> +
> +	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
> +}
> +
> +static const struct backlight_ops lmu_backlight_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
> +	.update_status = ti_lmu_bl_update_status,
> +};
> +
> +static int ti_lmu_bl_set_led_blocking(struct led_classdev *ledc,
> +					     enum led_brightness value)
> +{
> +	struct ti_lmu_bank *lmu_bank = dev_get_drvdata(ledc->dev->parent);
> +	int brightness = value;
> +
> +	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
> +}
> +
> +static int ti_lmu_bl_check_channel(struct ti_lmu_bank *lmu_bank)
> +{
> +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> +
> +	if (!reginfo->brightness_msb)
> +		return -EINVAL;
> +
> +	if (cfg->max_brightness > MAX_BRIGHTNESS_8BIT) {
> +		if (!reginfo->brightness_lsb)
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_create_channel(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct lmu_bl_reg_data *regdata = lmu_bank->cfg->reginfo->channel;
> +	int num_channels = lmu_bank->cfg->num_channels;
> +	unsigned long led_sources = lmu_bank->leds;
> +	int i, ret;
> +	u8 shift;
> +
> +	/*
> +	 * How to create backlight output channels:
> +	 *   Check 'led_sources' bit and update registers.
> +	 *
> +	 *   1) Dual channel configuration
> +	 *     The 1st register data is used for single channel.
> +	 *     The 2nd register data is used for dual channel.
> +	 *
> +	 *   2) Multiple channel configuration
> +	 *     Each register data is mapped to bank ID.
> +	 *     Bit shift operation is defined in channel registers.
> +	 *
> +	 * Channel register data consists of address, mask, value.
> +	 */
> +
> +	if (num_channels == NUM_DUAL_CHANNEL) {
> +		if (led_sources == LMU_BACKLIGHT_DUAL_CHANNEL_USED)
> +			regdata++;
> +
> +		return regmap_update_bits(regmap, regdata->reg, regdata->mask,
> +					  regdata->val);
> +	}
> +
> +	for (i = 0; regdata && i < num_channels; i++) {
> +		/*
> +		 * Note that the result of regdata->val is shift bit.
> +		 * The bank_id should be shifted for the channel configuration.
> +		 */
> +		if (test_bit(i, &led_sources)) {
> +			shift = regdata->val;
> +			ret = regmap_update_bits(regmap, regdata->reg,
> +						 regdata->mask,
> +						 lmu_bank->bank_id << shift);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		regdata++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_update_ctrl_mode(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct lmu_bl_reg_data *regdata =
> +		lmu_bank->cfg->reginfo->mode + lmu_bank->bank_id;
> +	u8 val = regdata->val;
> +
> +	if (!regdata)
> +		return 0;
> +
> +	/*
> +	 * Update PWM configuration register.
> +	 * If the mode is register based, then clear the bit.
> +	 */
> +	if (lmu_bank->mode != BL_PWM_BASED)
> +		val = 0;
> +
> +	return regmap_update_bits(regmap, regdata->reg, regdata->mask, val);
> +}
> +
> +static int ti_lmu_bl_convert_ramp_to_index(struct ti_lmu_bank *lmu_bank,
> +						  enum ti_lmu_bl_ramp_mode mode)
> +{
> +	const int *ramp_table = lmu_bank->cfg->ramp_table;
> +	const int size = lmu_bank->cfg->size_ramp;
> +	unsigned int msec;
> +	int i;
> +
> +	if (!ramp_table)
> +		return -EINVAL;
> +
> +	switch (mode) {
> +	case BL_RAMP_UP:
> +		msec = lmu_bank->ramp_up_msec;
> +		break;
> +	case BL_RAMP_DOWN:
> +		msec = lmu_bank->ramp_down_msec;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (msec <= ramp_table[0])
> +		return 0;
> +
> +	if (msec > ramp_table[size - 1])
> +		return size - 1;
> +
> +	for (i = 1; i < size; i++) {
> +		if (msec == ramp_table[i])
> +			return i;
> +
> +		/* Find an approximate index by looking up the table */
> +		if (msec > ramp_table[i - 1] && msec < ramp_table[i]) {
> +			if (msec - ramp_table[i - 1] < ramp_table[i] - msec)
> +				return i - 1;
> +			else
> +				return i;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +
> +static int ti_lmu_bl_set_ramp(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct ti_lmu_bl_reg *reginfo = lmu_bank->cfg->reginfo;
> +	int offset = reginfo->ramp_reg_offset;
> +	int i, ret, index;
> +	struct lmu_bl_reg_data regdata;
> +
> +	for (i = BL_RAMP_UP; i <= BL_RAMP_DOWN; i++) {
> +		index = ti_lmu_bl_convert_ramp_to_index(lmu_bank, i);
> +		if (index > 0) {
> +			if (!reginfo->ramp)
> +				break;
> +
> +			regdata = reginfo->ramp[i];
> +			if (lmu_bank->bank_id != 0)
> +				regdata.val += offset;
> +
> +			/* regdata.val is shift bit */
> +			ret = regmap_update_bits(regmap, regdata.reg,
> +						 regdata.mask,
> +						 index << regdata.val);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_configure(struct ti_lmu_bank *lmu_bank)
> +{
> +	int ret;
> +
> +	ret = ti_lmu_bl_check_channel(lmu_bank);
> +	if (ret)
> +		return ret;
> +
> +	ret = ti_lmu_bl_create_channel(lmu_bank);
> +	if (ret)
> +		return ret;
> +
> +	ret = ti_lmu_bl_update_ctrl_mode(lmu_bank);
> +	if (ret)
> +		return ret;
> +
> +	return ti_lmu_bl_set_ramp(lmu_bank);
> +}
> +
> +static int ti_lmu_bl_register_backlight(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct backlight_device *bl_dev;
> +	struct backlight_properties props;
> +
> +	if (lmu_bank->type != TI_LMU_BL)
> +		return -EINVAL;
> +
> +	memset(&props, 0, sizeof(struct backlight_properties));
> +	props.type = BACKLIGHT_PLATFORM;
> +	props.brightness = lmu_bank->default_brightness;
> +	props.max_brightness = lmu_bank->cfg->max_brightness;
> +
> +	bl_dev = devm_backlight_device_register(lmu_bank->dev, lmu_bank->label,
> +						lmu_bank->dev, lmu_bank,
> +						&lmu_backlight_ops, &props);
> +	if (IS_ERR(bl_dev))
> +		return PTR_ERR(bl_dev);
> +
> +	lmu_bank->backlight = bl_dev;
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_register_led(struct ti_lmu_bank *lmu_bank)
> +{
> +	int err;
> +
> +	if (lmu_bank->type != TI_LMU_LED)
> +		return -EINVAL;
> +
> +	lmu_bank->led = devm_kzalloc(lmu_bank->dev, sizeof(*lmu_bank->led),
> +				     GFP_KERNEL);
> +	if (!lmu_bank->led)
> +		return -ENOMEM;
> +
> +	lmu_bank->led->name = lmu_bank->label;
> +	lmu_bank->led->max_brightness = lmu_bank->cfg->max_brightness;
> +	lmu_bank->led->brightness_set_blocking =
> +		ti_lmu_bl_set_led_blocking;
> +
> +	err = devm_led_classdev_register(lmu_bank->dev, lmu_bank->led);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
> +{
> +	switch (lmu_bank->type) {
> +	case TI_LMU_BL:
> +		return ti_lmu_bl_register_backlight(lmu_bank);
> +	case TI_LMU_LED:
> +		return ti_lmu_bl_register_led(lmu_bank);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int setup_of_node(struct platform_device *pdev)
> +{
> +	struct device_node *parent_node = pdev->dev.parent->of_node;
> +	char *name;
> +
> +	if (!parent_node)
> +		return 0;
> +
> +	name = kasprintf(GFP_KERNEL, "bank%d", pdev->id);
> +	if (!name)
> +		return -ENOMEM;
> +
> +	pdev->dev.of_node = of_get_child_by_name(parent_node, name);
> +	kfree(name);
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_parse_led_sources(struct device *dev)
> +{
> +	unsigned long mask = 0;
> +	int ret;
> +	int size, i;
> +	u32 *leds;
> +
> +	size = device_property_read_u32_array(dev, "ti,led-sources", NULL, 0);
> +	if (size <= 0) {
> +		dev_err(dev, "Missing or malformed property led-sources: %d\n",
> +			size);
> +		return size < 0 ? size : -EINVAL;
> +	}
> +
> +	leds = kmalloc_array(size, sizeof(u32), GFP_KERNEL);
> +	if (!leds)
> +		return -ENOMEM;
> +
> +	ret = device_property_read_u32_array(dev, "ti,led-sources", leds, size);
> +	if (ret) {
> +		dev_err(dev, "Failed to read led-sources property: %d\n", ret);
> +		goto out;
> +	}
> +
> +	for (i = 0; i < size; i++)
> +		set_bit(leds[i], &mask);
> +
> +	ret = mask;
> +
> +out:
> +	kfree(leds);
> +	return ret;
> +}
> +
> +static int ti_lmu_bl_init(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct lmu_bl_reg_data *regdata =
> +		lmu_bank->cfg->reginfo->init;
> +	int num_init = lmu_bank->cfg->reginfo->num_init;
> +	int i, ret;
> +
> +	if (lmu_bank->lmu->backlight_initialized)
> +		return 0;
> +	lmu_bank->lmu->backlight_initialized = true;
> +
> +	for (i = 0; regdata && i < num_init; i++) {
> +		ret = regmap_update_bits(regmap, regdata->reg, regdata->mask,
> +					 regdata->val);
> +		if (ret)
> +			return ret;
> +
> +		regdata++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_reload(struct ti_lmu_bank *lmu_bank)
> +{
> +	int err;
> +
> +	ti_lmu_bl_init(lmu_bank);
> +
> +	err = ti_lmu_bl_configure(lmu_bank);
> +	if (err)
> +		return err;
> +
> +	return ti_lmu_bl_set_brightness(lmu_bank, lmu_bank->current_brightness);
> +}
> +
> +static int ti_lmu_bl_monitor_notifier(struct notifier_block *nb,
> +					     unsigned long action, void *unused)
> +{
> +	struct ti_lmu_bank *lmu_bank = container_of(nb, struct ti_lmu_bank, nb);
> +
> +	if (action == LMU_EVENT_MONITOR_DONE) {
> +		if (ti_lmu_bl_reload(lmu_bank))
> +			return NOTIFY_STOP;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int ti_lmu_bl_probe(struct platform_device *pdev)
> +{
> +	struct ti_lmu_bank *lmu_bank;
> +	int err;
> +
> +	err = setup_of_node(pdev);
> +	if (err)
> +		return err;
> +
> +	lmu_bank = devm_kzalloc(&pdev->dev, sizeof(*lmu_bank), GFP_KERNEL);
> +	if (!lmu_bank)
> +		return -ENOMEM;
> +	lmu_bank->dev = &pdev->dev;
> +	dev_set_drvdata(&pdev->dev, lmu_bank);
> +
> +	err = device_property_read_string(&pdev->dev, "label",
> +					  &lmu_bank->label);
> +	if (err)
> +		return err;
> +
> +	lmu_bank->type = TI_LMU_BL;
> +	if (!strcmp(lmu_bank->label, "keyboard")) {
> +		lmu_bank->type = TI_LMU_LED;
> +		lmu_bank->label = "kbd_backlight";
> +	}
> +
> +	lmu_bank->leds = ti_lmu_parse_led_sources(&pdev->dev);
> +	if (lmu_bank->leds < 0)
> +		return lmu_bank->leds;
> +	else if (lmu_bank->leds == 0)
> +		return -EINVAL;
> +
> +	device_property_read_u32(&pdev->dev, "default-brightness-level",
> +				 &lmu_bank->default_brightness);
> +	device_property_read_u32(&pdev->dev, "ti,ramp-up-ms",
> +				 &lmu_bank->ramp_up_msec);
> +	device_property_read_u32(&pdev->dev, "ti,ramp-down-ms",
> +				 &lmu_bank->ramp_down_msec);
> +	device_property_read_u32(&pdev->dev, "pwm-period",
> +				 &lmu_bank->pwm_period);
> +
> +	if (lmu_bank->pwm_period > 0)
> +		lmu_bank->mode = BL_PWM_BASED;
> +	else
> +		lmu_bank->mode = BL_REGISTER_BASED;
> +
> +	lmu_bank->lmu = dev_get_drvdata(pdev->dev.parent);
> +	lmu_bank->cfg = &lmu_bl_cfg[lmu_bank->lmu->id];
> +	lmu_bank->bank_id = pdev->id;
> +
> +	ti_lmu_bl_init(lmu_bank);
> +
> +	err = ti_lmu_bl_configure(lmu_bank);
> +	if (err)
> +		return err;
> +
> +	err = ti_lmu_bl_add_device(lmu_bank);
> +	if (err)
> +		return err;
> +
> +	err = ti_lmu_bl_set_brightness(lmu_bank,
> +					      lmu_bank->default_brightness);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Notifier callback is required because backlight device needs
> +	 * reconfiguration after fault detection procedure is done by
> +	 * ti-lmu-fault-monitor driver.
> +	 */
> +	if (lmu_bank->cfg->fault_monitor_used) {
> +		lmu_bank->nb.notifier_call = ti_lmu_bl_monitor_notifier;
> +		err = blocking_notifier_chain_register(&lmu_bank->lmu->notifier,
> +						       &lmu_bank->nb);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_remove(struct platform_device *pdev)
> +{
> +	struct ti_lmu_bank *lmu_bank = platform_get_drvdata(pdev);
> +
> +	if (lmu_bank->cfg->fault_monitor_used)
> +		blocking_notifier_chain_unregister(&lmu_bank->lmu->notifier,
> +						   &lmu_bank->nb);
> +
> +	ti_lmu_bl_set_brightness(lmu_bank, 0);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver ti_lmu_bl_driver = {
> +	.probe  = ti_lmu_bl_probe,
> +	.remove  = ti_lmu_bl_remove,
> +	.driver = {
> +		.name = "ti-lmu-led-backlight",
> +	},
> +};
> +module_platform_driver(ti_lmu_bl_driver)
> +
> +MODULE_DESCRIPTION("TI LMU Backlight LED Driver");
> +MODULE_AUTHOR("Sebastian Reichel");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:ti-lmu-led-backlight");
> diff --git a/drivers/video/backlight/ti-lmu-backlight-data.c b/drivers/video/backlight/ti-lmu-backlight-data.c
> new file mode 100644
> index 000000000000..583136cb934d
> --- /dev/null
> +++ b/drivers/video/backlight/ti-lmu-backlight-data.c
> @@ -0,0 +1,304 @@
> +/*
> + * TI LMU (Lighting Management Unit) Backlight Device Data
> + *
> + * Copyright 2015 Texas Instruments
> + *
> + * Author: Milo Kim <milo.kim@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include "ti-lmu-backlight-data.h"
> +
> +/* LM3532 */
> +static const struct lmu_bl_reg_data lm3532_init_data[] = {
> +	{ LM3532_REG_ZONE_CFG_A, LM3532_ZONE_MASK, LM3532_ZONE_0 },
> +	{ LM3532_REG_ZONE_CFG_B, LM3532_ZONE_MASK, LM3532_ZONE_1 },
> +	{ LM3532_REG_ZONE_CFG_C, LM3532_ZONE_MASK, LM3532_ZONE_2 },
> +};
> +
> +static const struct lmu_bl_reg_data lm3532_channel_data[] = {
> +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED1_CFG_MASK,
> +	  LM3532_ILED1_CFG_SHIFT },
> +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED2_CFG_MASK,
> +	  LM3532_ILED2_CFG_SHIFT },
> +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED3_CFG_MASK,
> +	  LM3532_ILED3_CFG_SHIFT },
> +};
> +
> +static const struct lmu_bl_reg_data lm3532_mode_data[] = {
> +	{ LM3532_REG_PWM_A_CFG, LM3532_PWM_A_MASK, LM3532_PWM_ZONE_0 },
> +	{ LM3532_REG_PWM_B_CFG, LM3532_PWM_B_MASK, LM3532_PWM_ZONE_1 },
> +	{ LM3532_REG_PWM_C_CFG, LM3532_PWM_C_MASK, LM3532_PWM_ZONE_2 },
> +};
> +
> +static const struct lmu_bl_reg_data lm3532_ramp_data[] = {
> +	{ LM3532_REG_RAMPUP, LM3532_RAMPUP_MASK, LM3532_RAMPUP_SHIFT },
> +	{ LM3532_REG_RAMPDN, LM3532_RAMPDN_MASK, LM3532_RAMPDN_SHIFT },
> +};
> +
> +static u8 lm3532_enable_reg = LM3532_REG_ENABLE;
> +
> +static u8 lm3532_brightness_regs[] = {
> +	LM3532_REG_BRT_A,
> +	LM3532_REG_BRT_B,
> +	LM3532_REG_BRT_C,
> +};
> +
> +static const struct ti_lmu_bl_reg lm3532_reg_info = {
> +	.init		= lm3532_init_data,
> +	.num_init	= ARRAY_SIZE(lm3532_init_data),
> +	.channel	= lm3532_channel_data,
> +	.mode		= lm3532_mode_data,
> +	.ramp		= lm3532_ramp_data,
> +	.enable		= &lm3532_enable_reg,
> +	.brightness_msb	= lm3532_brightness_regs,
> +};
> +
> +/* LM3631 */
> +static const struct lmu_bl_reg_data lm3631_init_data[] = {
> +	{ LM3631_REG_BRT_MODE, LM3631_MODE_MASK, LM3631_DEFAULT_MODE },
> +	{ LM3631_REG_BL_CFG, LM3631_MAP_MASK, LM3631_EXPONENTIAL_MAP },
> +};
> +
> +static const struct lmu_bl_reg_data lm3631_channel_data[] = {
> +	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_SINGLE_CHANNEL },
> +	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_DUAL_CHANNEL },
> +};
> +
> +static const struct lmu_bl_reg_data lm3631_ramp_data[] = {
> +	{ LM3631_REG_SLOPE, LM3631_SLOPE_MASK, LM3631_SLOPE_SHIFT },
> +};
> +
> +static u8 lm3631_enable_reg = LM3631_REG_DEVCTRL;
> +static u8 lm3631_brightness_msb_reg = LM3631_REG_BRT_MSB;
> +static u8 lm3631_brightness_lsb_reg = LM3631_REG_BRT_LSB;
> +
> +static const struct ti_lmu_bl_reg lm3631_reg_info = {
> +	.init		= lm3631_init_data,
> +	.num_init	= ARRAY_SIZE(lm3631_init_data),
> +	.channel	= lm3631_channel_data,
> +	.ramp		= lm3631_ramp_data,
> +	.enable		= &lm3631_enable_reg,
> +	.brightness_msb	= &lm3631_brightness_msb_reg,
> +	.brightness_lsb	= &lm3631_brightness_lsb_reg,
> +};
> +
> +/* LM3632 */
> +static const struct lmu_bl_reg_data lm3632_init_data[] = {
> +	{ LM3632_REG_CONFIG1, LM3632_OVP_MASK, LM3632_OVP_25V },
> +	{ LM3632_REG_CONFIG2, LM3632_SWFREQ_MASK, LM3632_SWFREQ_1MHZ },
> +};
> +
> +static const struct lmu_bl_reg_data lm3632_channel_data[] = {
> +	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_SINGLE_CHANNEL },
> +	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_DUAL_CHANNEL },
> +};
> +
> +static const struct lmu_bl_reg_data lm3632_mode_data[] = {
> +	{ LM3632_REG_IO_CTRL, LM3632_PWM_MASK, LM3632_PWM_MODE },
> +};
> +
> +static u8 lm3632_enable_reg = LM3632_REG_ENABLE;
> +static u8 lm3632_brightness_msb_reg = LM3632_REG_BRT_MSB;
> +static u8 lm3632_brightness_lsb_reg = LM3632_REG_BRT_LSB;
> +
> +static const struct ti_lmu_bl_reg lm3632_reg_info = {
> +	.init		= lm3632_init_data,
> +	.num_init	= ARRAY_SIZE(lm3632_init_data),
> +	.channel	= lm3632_channel_data,
> +	.mode		= lm3632_mode_data,
> +	.enable		= &lm3632_enable_reg,
> +	.brightness_msb	= &lm3632_brightness_msb_reg,
> +	.brightness_lsb	= &lm3632_brightness_lsb_reg,
> +};
> +
> +/* LM3633 */
> +static const struct lmu_bl_reg_data lm3633_init_data[] = {
> +	{ LM3633_REG_BOOST_CFG, LM3633_OVP_MASK, LM3633_OVP_40V },
> +	{ LM3633_REG_BL_RAMP_CONF, LM3633_BL_RAMP_MASK, LM3633_BL_RAMP_EACH },
> +};
> +
> +static const struct lmu_bl_reg_data lm3633_channel_data[] = {
> +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED1_CFG_MASK,
> +	  LM3633_HVLED1_CFG_SHIFT },
> +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED2_CFG_MASK,
> +	  LM3633_HVLED2_CFG_SHIFT },
> +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED3_CFG_MASK,
> +	  LM3633_HVLED3_CFG_SHIFT },
> +};
> +
> +static const struct lmu_bl_reg_data lm3633_mode_data[] = {
> +	{ LM3633_REG_PWM_CFG, LM3633_PWM_A_MASK, LM3633_PWM_A_MASK },
> +	{ LM3633_REG_PWM_CFG, LM3633_PWM_B_MASK, LM3633_PWM_B_MASK },
> +};
> +
> +static const struct lmu_bl_reg_data lm3633_ramp_data[] = {
> +	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPUP_MASK, LM3633_BL_RAMPUP_SHIFT },
> +	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPDN_MASK, LM3633_BL_RAMPDN_SHIFT },
> +};
> +
> +static u8 lm3633_enable_reg = LM3633_REG_ENABLE;
> +
> +static u8 lm3633_brightness_msb_regs[] = {
> +	LM3633_REG_BRT_HVLED_A_MSB,
> +	LM3633_REG_BRT_HVLED_B_MSB,
> +};
> +
> +static u8 lm3633_brightness_lsb_regs[] = {
> +	LM3633_REG_BRT_HVLED_A_LSB,
> +	LM3633_REG_BRT_HVLED_B_LSB,
> +};
> +
> +static const struct ti_lmu_bl_reg lm3633_reg_info = {
> +	.init		 = lm3633_init_data,
> +	.num_init	 = ARRAY_SIZE(lm3633_init_data),
> +	.channel	 = lm3633_channel_data,
> +	.mode		 = lm3633_mode_data,
> +	.ramp		 = lm3633_ramp_data,
> +	.ramp_reg_offset = 1, /* For LM3633_REG_BL1_RAMPUP/DN */
> +	.enable		 = &lm3633_enable_reg,
> +	.brightness_msb	 = lm3633_brightness_msb_regs,
> +	.brightness_lsb	 = lm3633_brightness_lsb_regs,
> +};
> +
> +/* LM3695 */
> +static const struct lmu_bl_reg_data lm3695_init_data[] = {
> +	{ LM3695_REG_GP, LM3695_BRT_RW_MASK, LM3695_BRT_RW_MASK },
> +};
> +
> +static const struct lmu_bl_reg_data lm3695_channel_data[] = {
> +	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_SINGLE_CHANNEL },
> +	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_DUAL_CHANNEL },
> +};
> +
> +static u8 lm3695_enable_reg = LM3695_REG_GP;
> +static u8 lm3695_brightness_msb_reg = LM3695_REG_BRT_MSB;
> +static u8 lm3695_brightness_lsb_reg = LM3695_REG_BRT_LSB;
> +
> +static const struct ti_lmu_bl_reg lm3695_reg_info = {
> +	.init		= lm3695_init_data,
> +	.num_init	= ARRAY_SIZE(lm3695_init_data),
> +	.channel	= lm3695_channel_data,
> +	.enable		= &lm3695_enable_reg,
> +	.enable_usec	= 600,
> +	.brightness_msb	= &lm3695_brightness_msb_reg,
> +	.brightness_lsb	= &lm3695_brightness_lsb_reg,
> +};
> +
> +/* LM3697 */
> +static const struct lmu_bl_reg_data lm3697_init_data[] = {
> +	{ LM3697_REG_RAMP_CONF, LM3697_RAMP_MASK, LM3697_RAMP_EACH },
> +};
> +
> +static const struct lmu_bl_reg_data lm3697_channel_data[] = {
> +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED1_CFG_MASK,
> +	  LM3697_HVLED1_CFG_SHIFT },
> +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED2_CFG_MASK,
> +	  LM3697_HVLED2_CFG_SHIFT },
> +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED3_CFG_MASK,
> +	  LM3697_HVLED3_CFG_SHIFT },
> +};
> +
> +static const struct lmu_bl_reg_data lm3697_mode_data[] = {
> +	{ LM3697_REG_PWM_CFG, LM3697_PWM_A_MASK, LM3697_PWM_A_MASK },
> +	{ LM3697_REG_PWM_CFG, LM3697_PWM_B_MASK, LM3697_PWM_B_MASK },
> +};
> +
> +static const struct lmu_bl_reg_data lm3697_ramp_data[] = {
> +	{ LM3697_REG_BL0_RAMP, LM3697_RAMPUP_MASK, LM3697_RAMPUP_SHIFT },
> +	{ LM3697_REG_BL0_RAMP, LM3697_RAMPDN_MASK, LM3697_RAMPDN_SHIFT },
> +};
> +
> +static u8 lm3697_enable_reg = LM3697_REG_ENABLE;
> +
> +static u8 lm3697_brightness_msb_regs[] = {
> +	LM3697_REG_BRT_A_MSB,
> +	LM3697_REG_BRT_B_MSB,
> +};
> +
> +static u8 lm3697_brightness_lsb_regs[] = {
> +	LM3697_REG_BRT_A_LSB,
> +	LM3697_REG_BRT_B_LSB,
> +};
> +
> +static const struct ti_lmu_bl_reg lm3697_reg_info = {
> +	.init		 = lm3697_init_data,
> +	.num_init	 = ARRAY_SIZE(lm3697_init_data),
> +	.channel	 = lm3697_channel_data,
> +	.mode		 = lm3697_mode_data,
> +	.ramp		 = lm3697_ramp_data,
> +	.ramp_reg_offset = 1, /* For LM3697_REG_BL1_RAMPUP/DN */
> +	.enable		 = &lm3697_enable_reg,
> +	.brightness_msb	 = lm3697_brightness_msb_regs,
> +	.brightness_lsb	 = lm3697_brightness_lsb_regs,
> +};
> +
> +static int lm3532_ramp_table[] = { 0, 1, 2, 4, 8, 16, 32, 65 };
> +
> +static int lm3631_ramp_table[] = {
> +	   0,   1,   2,    5,   10,   20,   50,  100,
> +	 250, 500, 750, 1000, 1500, 2000, 3000, 4000,
> +};
> +
> +static int common_ramp_table[] = {
> +	   2, 250, 500, 1000, 2000, 4000, 8000, 16000,
> +};
> +
> +#define LM3532_MAX_CHANNELS		3
> +#define LM3631_MAX_CHANNELS		2
> +#define LM3632_MAX_CHANNELS		2
> +#define LM3633_MAX_CHANNELS		3
> +#define LM3695_MAX_CHANNELS		2
> +#define LM3697_MAX_CHANNELS		3
> +
> +const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID] = {
> +	{
> +		.reginfo		= &lm3532_reg_info,
> +		.num_channels		= LM3532_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_8BIT,
> +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> +		.ramp_table		= lm3532_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(lm3532_ramp_table),
> +	},
> +	{
> +		.reginfo		= &lm3631_reg_info,
> +		.num_channels		= LM3631_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_ONLY,
> +		.ramp_table		= lm3631_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(lm3631_ramp_table),
> +	},
> +	{
> +		.reginfo		= &lm3632_reg_info,
> +		.num_channels		= LM3632_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_ONLY,
> +	},
> +	{
> +		.reginfo		= &lm3633_reg_info,
> +		.num_channels		= LM3633_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_MAX_BRT,
> +		.ramp_table		= common_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(common_ramp_table),
> +		.fault_monitor_used	= true,
> +	},
> +	{
> +		.reginfo		= &lm3695_reg_info,
> +		.num_channels		= LM3695_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> +	},
> +	{
> +		.reginfo		= &lm3697_reg_info,
> +		.num_channels		= LM3697_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> +		.ramp_table		= common_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(common_ramp_table),
> +		.fault_monitor_used	= true,
> +	},
> +};
> diff --git a/drivers/video/backlight/ti-lmu-backlight-data.h b/drivers/video/backlight/ti-lmu-backlight-data.h
> new file mode 100644
> index 000000000000..c64e8e6513e1
> --- /dev/null
> +++ b/drivers/video/backlight/ti-lmu-backlight-data.h
> @@ -0,0 +1,95 @@
> +/*
> + * TI LMU (Lighting Management Unit) Backlight Device Data Definitions
> + *
> + * Copyright 2015 Texas Instruments
> + *
> + * Author: Milo Kim <milo.kim@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __TI_LMU_BACKLIGHT_H__
> +#define __TI_LMU_BACKLIGHT_H__
> +
> +#include <linux/mfd/ti-lmu.h>
> +#include <linux/mfd/ti-lmu-register.h>
> +
> +#define MAX_BRIGHTNESS_8BIT		255
> +#define MAX_BRIGHTNESS_11BIT		2047
> +
> +enum ti_lmu_bl_pwm_action {
> +	/* Update PWM duty, no brightness register update is required */
> +	UPDATE_PWM_ONLY,
> +	/* Update not only duty but also brightness register */
> +	UPDATE_PWM_AND_BRT_REGISTER,
> +	/* Update max value in brightness registers */
> +	UPDATE_MAX_BRT,
> +};
> +
> +struct lmu_bl_reg_data {
> +	u8 reg;
> +	u8 mask;
> +	u8 val;
> +};
> +
> +/**
> + * struct ti_lmu_bl_reg
> + *
> + * @init:		Device initialization registers
> + * @num_init:		Numbers of initialization registers
> + * @channel:		Backlight channel configuration registers
> + * @mode:		Brightness control mode registers
> + * @ramp:		Ramp registers for lighting effect
> + * @ramp_reg_offset:	Ramp register offset.
> + *			Only used for multiple ramp registers.
> + * @enable:		Enable control register address
> + * @enable_usec:	Delay time for updating enable register.
> + *			Unit is microsecond.
> + * @brightness_msb:	Brightness MSB(Upper 8 bits) registers.
> + *			Concatenated with LSB in 11 bit dimming mode.
> + *			In 8 bit dimming, only MSB is used.
> + * @brightness_lsb:	Brightness LSB(Lower 3 bits) registers.
> + *			Only valid in 11 bit dimming mode.
> + */
> +struct ti_lmu_bl_reg {
> +	const struct lmu_bl_reg_data *init;
> +	int num_init;
> +	const struct lmu_bl_reg_data *channel;
> +	const struct lmu_bl_reg_data *mode;
> +	const struct lmu_bl_reg_data *ramp;
> +	int ramp_reg_offset;
> +	u8 *enable;
> +	unsigned long enable_usec;
> +	u8 *brightness_msb;
> +	u8 *brightness_lsb;
> +};
> +
> +/**
> + * struct ti_lmu_bl_cfg
> + *
> + * @reginfo:		Device register configuration
> + * @num_channels:	Number of backlight channels
> + * @max_brightness:	Max brightness value of backlight device
> + * @pwm_action:		How to control brightness registers in PWM mode
> + * @ramp_table:		[Optional] Ramp time table for lighting effect.
> + *			It's used for searching approximate register index.
> + * @size_ramp:		[Optional] Size of ramp table
> + * @fault_monitor_used:	[Optional] Set true if the device needs to handle
> + *			LMU fault monitor event.
> + *
> + * This structure is used for device specific data configuration.
> + */
> +struct ti_lmu_bl_cfg {
> +	const struct ti_lmu_bl_reg *reginfo;
> +	int num_channels;
> +	int max_brightness;
> +	enum ti_lmu_bl_pwm_action pwm_action;
> +	int *ramp_table;
> +	int size_ramp;
> +	bool fault_monitor_used;
> +};
> +
> +extern const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID];
> +#endif
> -- 
> 2.16.2
> 

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 9196 bytes --]

Hi Daniel,

On Wed, Apr 04, 2018 at 03:57:39PM +0100, Daniel Thompson wrote:
> On Fri, Mar 30, 2018 at 07:24:12PM +0200, Sebastian Reichel wrote:
> > This adds backlight support for the following TI LMU
> > chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> > 
> > Signed-off-by: Milo Kim <milo.kim@ti.com>
> > [add LED subsystem support for keyboard backlight and rework DT
> 
> Milo's mail has be bouncing for a very long time now. Did they really 
> sign off this code or is this intended to be an authorship credit?

I took over his patches from ~ a year ago and reworked them.

> >  binding according to Rob Herrings feedback]
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/video/backlight/Kconfig                 |   7 +
> >  drivers/video/backlight/Makefile                |   3 +
> >  drivers/video/backlight/ti-lmu-backlight-core.c | 666 ++++++++++++++++++++++++
> >  drivers/video/backlight/ti-lmu-backlight-data.c | 304 +++++++++++
> >  drivers/video/backlight/ti-lmu-backlight-data.h |  95 ++++
> >  5 files changed, 1075 insertions(+)
> >  create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
> >  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
> >  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h
> > 
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index 5d2d0d7e8100..27e6c5a0add8 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -427,6 +427,13 @@ config BACKLIGHT_SKY81452
> >  	  To compile this driver as a module, choose M here: the module will
> >  	  be called sky81452-backlight
> >  
> > +config BACKLIGHT_TI_LMU
> > +	tristate "Backlight driver for TI LMU"
> > +	depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
> > +	help
> > +	  Say Y to enable the backlight driver for TI LMU devices.
> > +	  This supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> > +
> >  config BACKLIGHT_TPS65217
> >  	tristate "TPS65217 Backlight"
> >  	depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
> > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > index 19da71d518bf..a1132d3dfd4c 100644
> > --- a/drivers/video/backlight/Makefile
> > +++ b/drivers/video/backlight/Makefile
> > @@ -53,6 +53,9 @@ obj-$(CONFIG_BACKLIGHT_PM8941_WLED)	+= pm8941-wled.o
> >  obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
> >  obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
> >  obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
> > +ti-lmu-backlight-objs			:= ti-lmu-backlight-core.o \
> > +					   ti-lmu-backlight-data.o
> > +obj-$(CONFIG_BACKLIGHT_TI_LMU)		+= ti-lmu-backlight.o
> >  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
> >  obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
> >  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
> > diff --git a/drivers/video/backlight/ti-lmu-backlight-core.c b/drivers/video/backlight/ti-lmu-backlight-core.c
> > new file mode 100644
> > index 000000000000..a6099581edd7
> > --- /dev/null
> > +++ b/drivers/video/backlight/ti-lmu-backlight-core.c
> > @@ -0,0 +1,666 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2015 Texas Instruments
> > + * Copyright 2018 Sebastian Reichel
> > + *
> > + * TI LMU Backlight driver, based on previous work from
> > + * Milo Kim <milo.kim@ti.com>
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/bitops.h>
> > +#include <linux/device.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/ti-lmu.h>
> > +#include <linux/mfd/ti-lmu-register.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "ti-lmu-backlight-data.h"
> > +
> > +enum ti_lmu_bl_ctrl_mode {
> > +	BL_REGISTER_BASED,
> > +	BL_PWM_BASED,
> > +};
> > +
> > +enum ti_lmu_bl_type {
> > +	TI_LMU_BL,  /* backlight userspace interface */
> > +	TI_LMU_LED, /* led userspace interface */
> > +};
> > +
> > +enum ti_lmu_bl_ramp_mode {
> > +	BL_RAMP_UP,
> > +	BL_RAMP_DOWN,
> > +};
> > +
> > +#define DEFAULT_PWM_NAME			"lmu-backlight"
> > +#define NUM_DUAL_CHANNEL			2
> > +#define LMU_BACKLIGHT_DUAL_CHANNEL_USED		(BIT(0) | BIT(1))
> > +#define LMU_BACKLIGHT_11BIT_LSB_MASK		(BIT(0) | BIT(1) | BIT(2))
> > +#define LMU_BACKLIGHT_11BIT_MSB_SHIFT		3
> > +
> > +struct ti_lmu_bank {
> > +	struct device *dev;
> > +	int bank_id;
> > +	const struct ti_lmu_bl_cfg *cfg;
> > +	struct ti_lmu *lmu;
> > +	const char *label;
> > +	int leds;
> > +	int current_brightness;
> > +	u32 default_brightness;
> > +	u32 ramp_up_msec;
> > +	u32 ramp_down_msec;
> > +	u32 pwm_period;
> > +	enum ti_lmu_bl_ctrl_mode mode;
> > +	enum ti_lmu_bl_type type;
> > +
> > +	struct notifier_block nb;
> > +
> > +	struct backlight_device *backlight;
> > +	struct led_classdev *led;
> > +};
> > +
> > +static int ti_lmu_bl_enable(struct ti_lmu_bank *lmu_bank, bool enable)
> > +{
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	unsigned long enable_time = lmu_bank->cfg->reginfo->enable_usec;
> > +	u8 *reg = lmu_bank->cfg->reginfo->enable;
> > +	u8 mask = BIT(lmu_bank->bank_id);
> > +	u8 val = (enable == true) ? mask : 0;
> > +	int ret;
> > +
> > +	if (!reg)
> > +		return -EINVAL;
> > +
> > +	ret = regmap_update_bits(regmap, *reg, mask, val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (enable_time > 0)
> > +		usleep_range(enable_time, enable_time + 100);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_update_brightness_register(struct ti_lmu_bank *lmu_bank,
> > +						       int brightness)
> > +{
> > +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	u8 reg, val;
> > +	int ret;
> > +
> > +	/*
> > +	 * Brightness register update
> > +	 *
> > +	 * 11 bit dimming: update LSB bits and write MSB byte.
> > +	 *		   MSB brightness should be shifted.
> > +	 *  8 bit dimming: write MSB byte.
> > +	 */
> > +	if (cfg->max_brightness == MAX_BRIGHTNESS_11BIT) {
> > +		reg = reginfo->brightness_lsb[lmu_bank->bank_id];
> > +		ret = regmap_update_bits(regmap, reg,
> > +					 LMU_BACKLIGHT_11BIT_LSB_MASK,
> > +					 brightness);
> > +		if (ret)
> > +			return ret;
> > +
> > +		val = brightness >> LMU_BACKLIGHT_11BIT_MSB_SHIFT;
> > +	} else {
> > +		val = brightness;
> > +	}
> > +
> > +	reg = reginfo->brightness_msb[lmu_bank->bank_id];
> > +	return regmap_write(regmap, reg, val);
> > +}
> > +
> > +static int ti_lmu_bl_pwm_ctrl(struct ti_lmu_bank *lmu_bank, int brightness)
> > +{
> > +	int max_brightness = lmu_bank->cfg->max_brightness;
> > +	struct pwm_state state = { };
> > +	int ret;
> > +
> > +	if (!lmu_bank->lmu->pwm) {
> > +		dev_err(lmu_bank->dev, "Missing PWM device!\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	pwm_init_state(lmu_bank->lmu->pwm, &state);
> > +	state.period = lmu_bank->pwm_period;
> > +	state.duty_cycle = brightness * state.period / max_brightness;
> > +
> > +	if (state.duty_cycle)
> > +		state.enabled = true;
> > +	else
> > +		state.enabled = false;
> > +
> > +	ret = pwm_apply_state(lmu_bank->lmu->pwm, &state);
> > +	if (ret)
> > +		dev_err(lmu_bank->dev, "Failed to configure PWM: %d", ret);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ti_lmu_bl_set_brightness(struct ti_lmu_bank *lmu_bank,
> > +				    int brightness)
> > +{
> > +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > +	bool enable = brightness > 0;
> > +	int ret;
> > +
> > +	ret = ti_lmu_bl_enable(lmu_bank, enable);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (lmu_bank->mode == BL_PWM_BASED) {
> > +		ti_lmu_bl_pwm_ctrl(lmu_bank, brightness);
> > +
> > +		switch (cfg->pwm_action) {
> > +		case UPDATE_PWM_ONLY:
> > +			/* No register update is required */
> > +			return 0;
> > +		case UPDATE_MAX_BRT:
> > +			/*
> > +			 * PWM can start from any non-zero code and dim down
> > +			 * to zero. So, brightness register should be updated
> > +			 * even in PWM mode.
> > +			 */
> 
> This comment could do with a little expansion (I assume the bank's
> brightness register means something different when in PWM mode but the
> flow of the code is tricky to read).

I will consult the manual and see how exactly the PWM stuff
is supposed to work. The platform I need this driver for
(Motorola Droid 4) does not have/use the PWM feature.

> > +			if (brightness > 0)
> 
> Isn't this "enable"?

Yes, good catch!

> > +				brightness = MAX_BRIGHTNESS_11BIT;
> > +			else
> > +				brightness = 0;
> > +			break;
> > +		default:
> 
> case UPDATE_PWM_AND_BRT_REGISTER:

ok.

> 
> > +			break;
> > +		}
> > +	}
> > +
> > +	lmu_bank->current_brightness = brightness;
> > +
> > +	return ti_lmu_bl_update_brightness_register(lmu_bank, brightness);
> > +}
> > [...]

Thanks for the review.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Dan Murphy]

Sebastian

-Milo Kim email is not valid

On 03/30/2018 12:24 PM, Sebastian Reichel wrote:
> This adds backlight support for the following TI LMU
> chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> 
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> [add LED subsystem support for keyboard backlight and rework DT
>  binding according to Rob Herrings feedback]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  drivers/video/backlight/Kconfig                 |   7 +
>  drivers/video/backlight/Makefile                |   3 +
>  drivers/video/backlight/ti-lmu-backlight-core.c | 666 ++++++++++++++++++++++++
>  drivers/video/backlight/ti-lmu-backlight-data.c | 304 +++++++++++
>  drivers/video/backlight/ti-lmu-backlight-data.h |  95 ++++
>  5 files changed, 1075 insertions(+)
>  create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
>  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
>  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 5d2d0d7e8100..27e6c5a0add8 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -427,6 +427,13 @@ config BACKLIGHT_SKY81452
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called sky81452-backlight
>  
> +config BACKLIGHT_TI_LMU
> +	tristate "Backlight driver for TI LMU"
> +	depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU

select REGMAP?

> +	help
> +	  Say Y to enable the backlight driver for TI LMU devices.
> +	  This supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> +
>  config BACKLIGHT_TPS65217
>  	tristate "TPS65217 Backlight"
>  	depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 19da71d518bf..a1132d3dfd4c 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -53,6 +53,9 @@ obj-$(CONFIG_BACKLIGHT_PM8941_WLED)	+= pm8941-wled.o
>  obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
>  obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
>  obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
> +ti-lmu-backlight-objs			:= ti-lmu-backlight-core.o \
> +					   ti-lmu-backlight-data.o
> +obj-$(CONFIG_BACKLIGHT_TI_LMU)		+= ti-lmu-backlight.o
>  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
>  obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
>  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
> diff --git a/drivers/video/backlight/ti-lmu-backlight-core.c b/drivers/video/backlight/ti-lmu-backlight-core.c
> new file mode 100644
> index 000000000000..a6099581edd7
> --- /dev/null
> +++ b/drivers/video/backlight/ti-lmu-backlight-core.c
> @@ -0,0 +1,666 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2015 Texas Instruments
> + * Copyright 2018 Sebastian Reichel
> + *
> + * TI LMU Backlight driver, based on previous work from
> + * Milo Kim <milo.kim@ti.com>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/ti-lmu.h>
> +#include <linux/mfd/ti-lmu-register.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +#include "ti-lmu-backlight-data.h"
> +
> +enum ti_lmu_bl_ctrl_mode {
> +	BL_REGISTER_BASED,
> +	BL_PWM_BASED,
> +};
> +
> +enum ti_lmu_bl_type {
> +	TI_LMU_BL,  /* backlight userspace interface */
> +	TI_LMU_LED, /* led userspace interface */
> +};
> +
> +enum ti_lmu_bl_ramp_mode {
> +	BL_RAMP_UP,
> +	BL_RAMP_DOWN,
> +};
> +
> +#define DEFAULT_PWM_NAME			"lmu-backlight"

I don't see this used.

> +#define NUM_DUAL_CHANNEL			2
> +#define LMU_BACKLIGHT_DUAL_CHANNEL_USED		(BIT(0) | BIT(1))
> +#define LMU_BACKLIGHT_11BIT_LSB_MASK		(BIT(0) | BIT(1) | BIT(2))
> +#define LMU_BACKLIGHT_11BIT_MSB_SHIFT		3
> +

Struct kerneldoc?

> +struct ti_lmu_bank {
> +	struct device *dev;
> +	int bank_id;
> +	const struct ti_lmu_bl_cfg *cfg;
> +	struct ti_lmu *lmu;
> +	const char *label;
> +	int leds;
> +	int current_brightness;
> +	u32 default_brightness;
> +	u32 ramp_up_msec;
> +	u32 ramp_down_msec;
> +	u32 pwm_period;
> +	enum ti_lmu_bl_ctrl_mode mode;
> +	enum ti_lmu_bl_type type;
> +
> +	struct notifier_block nb;
> +
> +	struct backlight_device *backlight;
> +	struct led_classdev *led;
> +};
> +
> +static int ti_lmu_bl_enable(struct ti_lmu_bank *lmu_bank, bool enable)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	unsigned long enable_time = lmu_bank->cfg->reginfo->enable_usec;
> +	u8 *reg = lmu_bank->cfg->reginfo->enable;
> +	u8 mask = BIT(lmu_bank->bank_id);
> +	u8 val = (enable == true) ? mask : 0;
> +	int ret;
> +
> +	if (!reg)
> +		return -EINVAL;
> +
> +	ret = regmap_update_bits(regmap, *reg, mask, val);
> +	if (ret)
> +		return ret;
> +
> +	if (enable_time > 0)
> +		usleep_range(enable_time, enable_time + 100);
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_update_brightness_register(struct ti_lmu_bank *lmu_bank,
> +						       int brightness)
> +{
> +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	u8 reg, val;
> +	int ret;
> +
> +	/*
> +	 * Brightness register update
> +	 *
> +	 * 11 bit dimming: update LSB bits and write MSB byte.
> +	 *		   MSB brightness should be shifted.
> +	 *  8 bit dimming: write MSB byte.
> +	 */
> +	if (cfg->max_brightness == MAX_BRIGHTNESS_11BIT) {
> +		reg = reginfo->brightness_lsb[lmu_bank->bank_id];
> +		ret = regmap_update_bits(regmap, reg,
> +					 LMU_BACKLIGHT_11BIT_LSB_MASK,
> +					 brightness);
> +		if (ret)
> +			return ret;
> +
> +		val = brightness >> LMU_BACKLIGHT_11BIT_MSB_SHIFT;
> +	} else {
> +		val = brightness;
> +	}
> +
> +	reg = reginfo->brightness_msb[lmu_bank->bank_id];
> +	return regmap_write(regmap, reg, val);
> +}
> +
> +static int ti_lmu_bl_pwm_ctrl(struct ti_lmu_bank *lmu_bank, int brightness)
> +{
> +	int max_brightness = lmu_bank->cfg->max_brightness;
> +	struct pwm_state state = { };
> +	int ret;
> +
> +	if (!lmu_bank->lmu->pwm) {
> +		dev_err(lmu_bank->dev, "Missing PWM device!\n");
> +		return -ENODEV;
> +	}
> +
> +	pwm_init_state(lmu_bank->lmu->pwm, &state);
> +	state.period = lmu_bank->pwm_period;
> +	state.duty_cycle = brightness * state.period / max_brightness;
> +
> +	if (state.duty_cycle)
> +		state.enabled = true;
> +	else
> +		state.enabled = false;
> +
> +	ret = pwm_apply_state(lmu_bank->lmu->pwm, &state);
> +	if (ret)
> +		dev_err(lmu_bank->dev, "Failed to configure PWM: %d", ret);
> +
> +	return ret;
> +}
> +
> +static int ti_lmu_bl_set_brightness(struct ti_lmu_bank *lmu_bank,
> +				    int brightness)
> +{
> +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> +	bool enable = brightness > 0;
> +	int ret;
> +

Is there anyway that update_status and set_led_blocking can be called at the same time causing
a race condition in this api?

> +	ret = ti_lmu_bl_enable(lmu_bank, enable);
> +	if (ret)
> +		return ret;
> +
> +	if (lmu_bank->mode == BL_PWM_BASED) {
> +		ti_lmu_bl_pwm_ctrl(lmu_bank, brightness);
> +
> +		switch (cfg->pwm_action) {
> +		case UPDATE_PWM_ONLY:
> +			/* No register update is required */
> +			return 0;
> +		case UPDATE_MAX_BRT:
> +			/*
> +			 * PWM can start from any non-zero code and dim down
> +			 * to zero. So, brightness register should be updated
> +			 * even in PWM mode.
> +			 */
> +			if (brightness > 0)
> +				brightness = MAX_BRIGHTNESS_11BIT;
> +			else
> +				brightness = 0;
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	lmu_bank->current_brightness = brightness;
> +
> +	return ti_lmu_bl_update_brightness_register(lmu_bank, brightness);
> +}
> +
> +static int ti_lmu_bl_update_status(struct backlight_device *bl_dev)
> +{
> +	struct ti_lmu_bank *lmu_bank = bl_get_data(bl_dev);
> +	int brightness = bl_dev->props.brightness;
> +
> +	if (bl_dev->props.state & BL_CORE_SUSPENDED)
> +		brightness = 0;
> +
> +	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
> +}
> +
> +static const struct backlight_ops lmu_backlight_ops = {
> +	.options = BL_CORE_SUSPENDRESUME,
> +	.update_status = ti_lmu_bl_update_status,
> +};
> +
> +static int ti_lmu_bl_set_led_blocking(struct led_classdev *ledc,
> +					     enum led_brightness value)
> +{
> +	struct ti_lmu_bank *lmu_bank = dev_get_drvdata(ledc->dev->parent);
> +	int brightness = value;
> +
> +	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
> +}
> +
> +static int ti_lmu_bl_check_channel(struct ti_lmu_bank *lmu_bank)
> +{
> +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> +
> +	if (!reginfo->brightness_msb)
> +		return -EINVAL;
> +
> +	if (cfg->max_brightness > MAX_BRIGHTNESS_8BIT) {
> +		if (!reginfo->brightness_lsb)
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_create_channel(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct lmu_bl_reg_data *regdata = lmu_bank->cfg->reginfo->channel;
> +	int num_channels = lmu_bank->cfg->num_channels;
> +	unsigned long led_sources = lmu_bank->leds;
> +	int i, ret;
> +	u8 shift;
> +
> +	/*
> +	 * How to create backlight output channels:
> +	 *   Check 'led_sources' bit and update registers.
> +	 *
> +	 *   1) Dual channel configuration
> +	 *     The 1st register data is used for single channel.
> +	 *     The 2nd register data is used for dual channel.
> +	 *
> +	 *   2) Multiple channel configuration
> +	 *     Each register data is mapped to bank ID.
> +	 *     Bit shift operation is defined in channel registers.
> +	 *
> +	 * Channel register data consists of address, mask, value.
> +	 */
> +
> +	if (num_channels == NUM_DUAL_CHANNEL) {
> +		if (led_sources == LMU_BACKLIGHT_DUAL_CHANNEL_USED)
> +			regdata++;
> +
> +		return regmap_update_bits(regmap, regdata->reg, regdata->mask,
> +					  regdata->val);
> +	}
> +
> +	for (i = 0; regdata && i < num_channels; i++) {
> +		/*
> +		 * Note that the result of regdata->val is shift bit.
> +		 * The bank_id should be shifted for the channel configuration.
> +		 */
> +		if (test_bit(i, &led_sources)) {
> +			shift = regdata->val;
> +			ret = regmap_update_bits(regmap, regdata->reg,
> +						 regdata->mask,
> +						 lmu_bank->bank_id << shift);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		regdata++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_update_ctrl_mode(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct lmu_bl_reg_data *regdata =
> +		lmu_bank->cfg->reginfo->mode + lmu_bank->bank_id;
> +	u8 val = regdata->val;
> +
> +	if (!regdata)
> +		return 0;
> +
> +	/*
> +	 * Update PWM configuration register.
> +	 * If the mode is register based, then clear the bit.
> +	 */
> +	if (lmu_bank->mode != BL_PWM_BASED)
> +		val = 0;
> +
> +	return regmap_update_bits(regmap, regdata->reg, regdata->mask, val);
> +}
> +
> +static int ti_lmu_bl_convert_ramp_to_index(struct ti_lmu_bank *lmu_bank,
> +						  enum ti_lmu_bl_ramp_mode mode)
> +{
> +	const int *ramp_table = lmu_bank->cfg->ramp_table;
> +	const int size = lmu_bank->cfg->size_ramp;
> +	unsigned int msec;
> +	int i;
> +
> +	if (!ramp_table)
> +		return -EINVAL;
> +
> +	switch (mode) {
> +	case BL_RAMP_UP:
> +		msec = lmu_bank->ramp_up_msec;
> +		break;
> +	case BL_RAMP_DOWN:
> +		msec = lmu_bank->ramp_down_msec;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (msec <= ramp_table[0])
> +		return 0;
> +
> +	if (msec > ramp_table[size - 1])
> +		return size - 1;
> +
> +	for (i = 1; i < size; i++) {
> +		if (msec == ramp_table[i])
> +			return i;
> +
> +		/* Find an approximate index by looking up the table */
> +		if (msec > ramp_table[i - 1] && msec < ramp_table[i]) {
> +			if (msec - ramp_table[i - 1] < ramp_table[i] - msec)
> +				return i - 1;
> +			else
> +				return i;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +

Extra new line

> +static int ti_lmu_bl_set_ramp(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct ti_lmu_bl_reg *reginfo = lmu_bank->cfg->reginfo;
> +	int offset = reginfo->ramp_reg_offset;
> +	int i, ret, index;
> +	struct lmu_bl_reg_data regdata;
> +
> +	for (i = BL_RAMP_UP; i <= BL_RAMP_DOWN; i++) {
> +		index = ti_lmu_bl_convert_ramp_to_index(lmu_bank, i);
> +		if (index > 0) {
> +			if (!reginfo->ramp)
> +				break;
> +
> +			regdata = reginfo->ramp[i];
> +			if (lmu_bank->bank_id != 0)
> +				regdata.val += offset;
> +
> +			/* regdata.val is shift bit */
> +			ret = regmap_update_bits(regmap, regdata.reg,
> +						 regdata.mask,
> +						 index << regdata.val);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_configure(struct ti_lmu_bank *lmu_bank)
> +{
> +	int ret;
> +
> +	ret = ti_lmu_bl_check_channel(lmu_bank);
> +	if (ret)
> +		return ret;
> +
> +	ret = ti_lmu_bl_create_channel(lmu_bank);
> +	if (ret)
> +		return ret;
> +
> +	ret = ti_lmu_bl_update_ctrl_mode(lmu_bank);
> +	if (ret)
> +		return ret;
> +
> +	return ti_lmu_bl_set_ramp(lmu_bank);
> +}
> +
> +static int ti_lmu_bl_register_backlight(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct backlight_device *bl_dev;
> +	struct backlight_properties props;
> +
> +	if (lmu_bank->type != TI_LMU_BL)
> +		return -EINVAL;
> +
> +	memset(&props, 0, sizeof(struct backlight_properties));
> +	props.type = BACKLIGHT_PLATFORM;
> +	props.brightness = lmu_bank->default_brightness;
> +	props.max_brightness = lmu_bank->cfg->max_brightness;
> +
> +	bl_dev = devm_backlight_device_register(lmu_bank->dev, lmu_bank->label,
> +						lmu_bank->dev, lmu_bank,
> +						&lmu_backlight_ops, &props);
> +	if (IS_ERR(bl_dev))
> +		return PTR_ERR(bl_dev);
> +
> +	lmu_bank->backlight = bl_dev;
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_register_led(struct ti_lmu_bank *lmu_bank)
> +{
> +	int err;
> +
> +	if (lmu_bank->type != TI_LMU_LED)
> +		return -EINVAL;
> +
> +	lmu_bank->led = devm_kzalloc(lmu_bank->dev, sizeof(*lmu_bank->led),
> +				     GFP_KERNEL);
> +	if (!lmu_bank->led)
> +		return -ENOMEM;
> +
> +	lmu_bank->led->name = lmu_bank->label;
> +	lmu_bank->led->max_brightness = lmu_bank->cfg->max_brightness;
> +	lmu_bank->led->brightness_set_blocking =
> +		ti_lmu_bl_set_led_blocking;
> +
> +	err = devm_led_classdev_register(lmu_bank->dev, lmu_bank->led);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
> +{
> +	switch (lmu_bank->type) {
> +	case TI_LMU_BL:
> +		return ti_lmu_bl_register_backlight(lmu_bank);
> +	case TI_LMU_LED:
> +		return ti_lmu_bl_register_led(lmu_bank);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int setup_of_node(struct platform_device *pdev)

This seems a bit generic.  Maybe keep with the naming convention

ti_lmu_setup_of_node

> +{
> +	struct device_node *parent_node = pdev->dev.parent->of_node;
> +	char *name;
> +
> +	if (!parent_node)
> +		return 0;
> +
> +	name = kasprintf(GFP_KERNEL, "bank%d", pdev->id);
> +	if (!name)
> +		return -ENOMEM;
> +
> +	pdev->dev.of_node = of_get_child_by_name(parent_node, name);
> +	kfree(name);
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_parse_led_sources(struct device *dev)
> +{
> +	unsigned long mask = 0;
> +	int ret;
> +	int size, i;
> +	u32 *leds;
> +
> +	size = device_property_read_u32_array(dev, "ti,led-sources", NULL, 0);
> +	if (size <= 0) {
> +		dev_err(dev, "Missing or malformed property led-sources: %d\n",
> +			size);
> +		return size < 0 ? size : -EINVAL;

Why the additional check? Why not just return -EINVAL?

> +	}
> +
> +	leds = kmalloc_array(size, sizeof(u32), GFP_KERNEL);
> +	if (!leds)
> +		return -ENOMEM;
> +
> +	ret = device_property_read_u32_array(dev, "ti,led-sources", leds, size);
> +	if (ret) {
> +		dev_err(dev, "Failed to read led-sources property: %d\n", ret);
> +		goto out;
> +	}
> +
> +	for (i = 0; i < size; i++)
> +		set_bit(leds[i], &mask);
> +
> +	ret = mask;
> +
> +out:
> +	kfree(leds);
> +	return ret;
> +}
> +
> +static int ti_lmu_bl_init(struct ti_lmu_bank *lmu_bank)
> +{
> +	struct regmap *regmap = lmu_bank->lmu->regmap;
> +	const struct lmu_bl_reg_data *regdata =
> +		lmu_bank->cfg->reginfo->init;
> +	int num_init = lmu_bank->cfg->reginfo->num_init;
> +	int i, ret;
> +
> +	if (lmu_bank->lmu->backlight_initialized)
> +		return 0;

Add a new line here

> +	lmu_bank->lmu->backlight_initialized = true;
> +
> +	for (i = 0; regdata && i < num_init; i++) {
> +		ret = regmap_update_bits(regmap, regdata->reg, regdata->mask,
> +					 regdata->val);
> +		if (ret)
> +			return ret;
> +
> +		regdata++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_reload(struct ti_lmu_bank *lmu_bank)
> +{
> +	int err;
> +
> +	ti_lmu_bl_init(lmu_bank);
> +
> +	err = ti_lmu_bl_configure(lmu_bank);
> +	if (err)
> +		return err;
> +
> +	return ti_lmu_bl_set_brightness(lmu_bank, lmu_bank->current_brightness);
> +}
> +
> +static int ti_lmu_bl_monitor_notifier(struct notifier_block *nb,
> +					     unsigned long action, void *unused)
> +{
> +	struct ti_lmu_bank *lmu_bank = container_of(nb, struct ti_lmu_bank, nb);
> +
> +	if (action == LMU_EVENT_MONITOR_DONE) {
> +		if (ti_lmu_bl_reload(lmu_bank))
> +			return NOTIFY_STOP;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static int ti_lmu_bl_probe(struct platform_device *pdev)
> +{
> +	struct ti_lmu_bank *lmu_bank;
> +	int err;
> +
> +	err = setup_of_node(pdev);
> +	if (err)
> +		return err;
> +
> +	lmu_bank = devm_kzalloc(&pdev->dev, sizeof(*lmu_bank), GFP_KERNEL);
> +	if (!lmu_bank)
> +		return -ENOMEM;

Add a new line here

> +	lmu_bank->dev = &pdev->dev;
> +	dev_set_drvdata(&pdev->dev, lmu_bank);
> +
> +	err = device_property_read_string(&pdev->dev, "label",
> +					  &lmu_bank->label);
> +	if (err)
> +		return err;
> +
> +	lmu_bank->type = TI_LMU_BL;
> +	if (!strcmp(lmu_bank->label, "keyboard")) {
> +		lmu_bank->type = TI_LMU_LED;
> +		lmu_bank->label = "kbd_backlight";

What is the reason for changing the label?  Why can't the label in the DT be kbd_backlight?

> +	}
> +
> +	lmu_bank->leds = ti_lmu_parse_led_sources(&pdev->dev);
> +	if (lmu_bank->leds < 0)
> +		return lmu_bank->leds;
> +	else if (lmu_bank->leds == 0)
> +		return -EINVAL;
> +
> +	device_property_read_u32(&pdev->dev, "default-brightness-level",
> +				 &lmu_bank->default_brightness);
> +	device_property_read_u32(&pdev->dev, "ti,ramp-up-ms",
> +				 &lmu_bank->ramp_up_msec);
> +	device_property_read_u32(&pdev->dev, "ti,ramp-down-ms",
> +				 &lmu_bank->ramp_down_msec);
> +	device_property_read_u32(&pdev->dev, "pwm-period",
> +				 &lmu_bank->pwm_period);
> +
> +	if (lmu_bank->pwm_period > 0)
> +		lmu_bank->mode = BL_PWM_BASED;
> +	else
> +		lmu_bank->mode = BL_REGISTER_BASED;
> +
> +	lmu_bank->lmu = dev_get_drvdata(pdev->dev.parent);
> +	lmu_bank->cfg = &lmu_bl_cfg[lmu_bank->lmu->id];
> +	lmu_bank->bank_id = pdev->id;
> +
> +	ti_lmu_bl_init(lmu_bank);
> +
> +	err = ti_lmu_bl_configure(lmu_bank);
> +	if (err)
> +		return err;
> +
> +	err = ti_lmu_bl_add_device(lmu_bank);
> +	if (err)
> +		return err;
> +
> +	err = ti_lmu_bl_set_brightness(lmu_bank,
> +					      lmu_bank->default_brightness);
> +	if (err)
> +		return err;
> +
> +	/*
> +	 * Notifier callback is required because backlight device needs
> +	 * reconfiguration after fault detection procedure is done by
> +	 * ti-lmu-fault-monitor driver.
> +	 */
> +	if (lmu_bank->cfg->fault_monitor_used) {
> +		lmu_bank->nb.notifier_call = ti_lmu_bl_monitor_notifier;
> +		err = blocking_notifier_chain_register(&lmu_bank->lmu->notifier,
> +						       &lmu_bank->nb);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_lmu_bl_remove(struct platform_device *pdev)
> +{
> +	struct ti_lmu_bank *lmu_bank = platform_get_drvdata(pdev);
> +
> +	if (lmu_bank->cfg->fault_monitor_used)
> +		blocking_notifier_chain_unregister(&lmu_bank->lmu->notifier,
> +						   &lmu_bank->nb);
> +
> +	ti_lmu_bl_set_brightness(lmu_bank, 0);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver ti_lmu_bl_driver = {
> +	.probe  = ti_lmu_bl_probe,
> +	.remove  = ti_lmu_bl_remove,
> +	.driver = {
> +		.name = "ti-lmu-led-backlight",
> +	},
> +};
> +module_platform_driver(ti_lmu_bl_driver)
> +
> +MODULE_DESCRIPTION("TI LMU Backlight LED Driver");
> +MODULE_AUTHOR("Sebastian Reichel");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:ti-lmu-led-backlight");
> diff --git a/drivers/video/backlight/ti-lmu-backlight-data.c b/drivers/video/backlight/ti-lmu-backlight-data.c
> new file mode 100644
> index 000000000000..583136cb934d
> --- /dev/null
> +++ b/drivers/video/backlight/ti-lmu-backlight-data.c
> @@ -0,0 +1,304 @@
> +/*

Inconsistent licensing here the core has
// SPDX-License-Identifier: GPL-2.0

> + * TI LMU (Lighting Management Unit) Backlight Device Data
> + *
> + * Copyright 2015 Texas Instruments

Copyright?

> + *
> + * Author: Milo Kim <milo.kim@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include "ti-lmu-backlight-data.h"
> +
> +/* LM3532 */
> +static const struct lmu_bl_reg_data lm3532_init_data[] = {
> +	{ LM3532_REG_ZONE_CFG_A, LM3532_ZONE_MASK, LM3532_ZONE_0 },
> +	{ LM3532_REG_ZONE_CFG_B, LM3532_ZONE_MASK, LM3532_ZONE_1 },
> +	{ LM3532_REG_ZONE_CFG_C, LM3532_ZONE_MASK, LM3532_ZONE_2 },
> +};
> +
> +static const struct lmu_bl_reg_data lm3532_channel_data[] = {
> +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED1_CFG_MASK,
> +	  LM3532_ILED1_CFG_SHIFT },
> +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED2_CFG_MASK,
> +	  LM3532_ILED2_CFG_SHIFT },
> +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED3_CFG_MASK,
> +	  LM3532_ILED3_CFG_SHIFT },
> +};
> +
> +static const struct lmu_bl_reg_data lm3532_mode_data[] = {
> +	{ LM3532_REG_PWM_A_CFG, LM3532_PWM_A_MASK, LM3532_PWM_ZONE_0 },
> +	{ LM3532_REG_PWM_B_CFG, LM3532_PWM_B_MASK, LM3532_PWM_ZONE_1 },
> +	{ LM3532_REG_PWM_C_CFG, LM3532_PWM_C_MASK, LM3532_PWM_ZONE_2 },
> +};
> +
> +static const struct lmu_bl_reg_data lm3532_ramp_data[] = {
> +	{ LM3532_REG_RAMPUP, LM3532_RAMPUP_MASK, LM3532_RAMPUP_SHIFT },
> +	{ LM3532_REG_RAMPDN, LM3532_RAMPDN_MASK, LM3532_RAMPDN_SHIFT },
> +};
> +
> +static u8 lm3532_enable_reg = LM3532_REG_ENABLE;
> +
> +static u8 lm3532_brightness_regs[] = {
> +	LM3532_REG_BRT_A,
> +	LM3532_REG_BRT_B,
> +	LM3532_REG_BRT_C,
> +};
> +
> +static const struct ti_lmu_bl_reg lm3532_reg_info = {
> +	.init		= lm3532_init_data,
> +	.num_init	= ARRAY_SIZE(lm3532_init_data),
> +	.channel	= lm3532_channel_data,
> +	.mode		= lm3532_mode_data,
> +	.ramp		= lm3532_ramp_data,
> +	.enable		= &lm3532_enable_reg,
> +	.brightness_msb	= lm3532_brightness_regs,
> +};
> +
> +/* LM3631 */
> +static const struct lmu_bl_reg_data lm3631_init_data[] = {
> +	{ LM3631_REG_BRT_MODE, LM3631_MODE_MASK, LM3631_DEFAULT_MODE },
> +	{ LM3631_REG_BL_CFG, LM3631_MAP_MASK, LM3631_EXPONENTIAL_MAP },
> +};
> +
> +static const struct lmu_bl_reg_data lm3631_channel_data[] = {
> +	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_SINGLE_CHANNEL },
> +	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_DUAL_CHANNEL },
> +};
> +
> +static const struct lmu_bl_reg_data lm3631_ramp_data[] = {
> +	{ LM3631_REG_SLOPE, LM3631_SLOPE_MASK, LM3631_SLOPE_SHIFT },
> +};
> +
> +static u8 lm3631_enable_reg = LM3631_REG_DEVCTRL;
> +static u8 lm3631_brightness_msb_reg = LM3631_REG_BRT_MSB;
> +static u8 lm3631_brightness_lsb_reg = LM3631_REG_BRT_LSB;
> +
> +static const struct ti_lmu_bl_reg lm3631_reg_info = {
> +	.init		= lm3631_init_data,
> +	.num_init	= ARRAY_SIZE(lm3631_init_data),
> +	.channel	= lm3631_channel_data,
> +	.ramp		= lm3631_ramp_data,
> +	.enable		= &lm3631_enable_reg,
> +	.brightness_msb	= &lm3631_brightness_msb_reg,
> +	.brightness_lsb	= &lm3631_brightness_lsb_reg,
> +};
> +
> +/* LM3632 */
> +static const struct lmu_bl_reg_data lm3632_init_data[] = {
> +	{ LM3632_REG_CONFIG1, LM3632_OVP_MASK, LM3632_OVP_25V },
> +	{ LM3632_REG_CONFIG2, LM3632_SWFREQ_MASK, LM3632_SWFREQ_1MHZ },
> +};
> +
> +static const struct lmu_bl_reg_data lm3632_channel_data[] = {
> +	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_SINGLE_CHANNEL },
> +	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_DUAL_CHANNEL },
> +};
> +
> +static const struct lmu_bl_reg_data lm3632_mode_data[] = {
> +	{ LM3632_REG_IO_CTRL, LM3632_PWM_MASK, LM3632_PWM_MODE },
> +};
> +
> +static u8 lm3632_enable_reg = LM3632_REG_ENABLE;
> +static u8 lm3632_brightness_msb_reg = LM3632_REG_BRT_MSB;
> +static u8 lm3632_brightness_lsb_reg = LM3632_REG_BRT_LSB;
> +
> +static const struct ti_lmu_bl_reg lm3632_reg_info = {
> +	.init		= lm3632_init_data,
> +	.num_init	= ARRAY_SIZE(lm3632_init_data),
> +	.channel	= lm3632_channel_data,
> +	.mode		= lm3632_mode_data,
> +	.enable		= &lm3632_enable_reg,
> +	.brightness_msb	= &lm3632_brightness_msb_reg,
> +	.brightness_lsb	= &lm3632_brightness_lsb_reg,
> +};
> +
> +/* LM3633 */
> +static const struct lmu_bl_reg_data lm3633_init_data[] = {
> +	{ LM3633_REG_BOOST_CFG, LM3633_OVP_MASK, LM3633_OVP_40V },
> +	{ LM3633_REG_BL_RAMP_CONF, LM3633_BL_RAMP_MASK, LM3633_BL_RAMP_EACH },
> +};
> +
> +static const struct lmu_bl_reg_data lm3633_channel_data[] = {
> +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED1_CFG_MASK,
> +	  LM3633_HVLED1_CFG_SHIFT },
> +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED2_CFG_MASK,
> +	  LM3633_HVLED2_CFG_SHIFT },
> +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED3_CFG_MASK,
> +	  LM3633_HVLED3_CFG_SHIFT },
> +};
> +
> +static const struct lmu_bl_reg_data lm3633_mode_data[] = {
> +	{ LM3633_REG_PWM_CFG, LM3633_PWM_A_MASK, LM3633_PWM_A_MASK },
> +	{ LM3633_REG_PWM_CFG, LM3633_PWM_B_MASK, LM3633_PWM_B_MASK },
> +};
> +
> +static const struct lmu_bl_reg_data lm3633_ramp_data[] = {
> +	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPUP_MASK, LM3633_BL_RAMPUP_SHIFT },
> +	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPDN_MASK, LM3633_BL_RAMPDN_SHIFT },
> +};
> +
> +static u8 lm3633_enable_reg = LM3633_REG_ENABLE;
> +
> +static u8 lm3633_brightness_msb_regs[] = {
> +	LM3633_REG_BRT_HVLED_A_MSB,
> +	LM3633_REG_BRT_HVLED_B_MSB,
> +};
> +
> +static u8 lm3633_brightness_lsb_regs[] = {
> +	LM3633_REG_BRT_HVLED_A_LSB,
> +	LM3633_REG_BRT_HVLED_B_LSB,
> +};
> +
> +static const struct ti_lmu_bl_reg lm3633_reg_info = {
> +	.init		 = lm3633_init_data,
> +	.num_init	 = ARRAY_SIZE(lm3633_init_data),
> +	.channel	 = lm3633_channel_data,
> +	.mode		 = lm3633_mode_data,
> +	.ramp		 = lm3633_ramp_data,
> +	.ramp_reg_offset = 1, /* For LM3633_REG_BL1_RAMPUP/DN */
> +	.enable		 = &lm3633_enable_reg,
> +	.brightness_msb	 = lm3633_brightness_msb_regs,
> +	.brightness_lsb	 = lm3633_brightness_lsb_regs,
> +};
> +
> +/* LM3695 */
> +static const struct lmu_bl_reg_data lm3695_init_data[] = {
> +	{ LM3695_REG_GP, LM3695_BRT_RW_MASK, LM3695_BRT_RW_MASK },
> +};
> +
> +static const struct lmu_bl_reg_data lm3695_channel_data[] = {
> +	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_SINGLE_CHANNEL },
> +	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_DUAL_CHANNEL },
> +};
> +
> +static u8 lm3695_enable_reg = LM3695_REG_GP;
> +static u8 lm3695_brightness_msb_reg = LM3695_REG_BRT_MSB;
> +static u8 lm3695_brightness_lsb_reg = LM3695_REG_BRT_LSB;
> +
> +static const struct ti_lmu_bl_reg lm3695_reg_info = {
> +	.init		= lm3695_init_data,
> +	.num_init	= ARRAY_SIZE(lm3695_init_data),
> +	.channel	= lm3695_channel_data,
> +	.enable		= &lm3695_enable_reg,
> +	.enable_usec	= 600,
> +	.brightness_msb	= &lm3695_brightness_msb_reg,
> +	.brightness_lsb	= &lm3695_brightness_lsb_reg,
> +};
> +
> +/* LM3697 */
> +static const struct lmu_bl_reg_data lm3697_init_data[] = {
> +	{ LM3697_REG_RAMP_CONF, LM3697_RAMP_MASK, LM3697_RAMP_EACH },
> +};
> +
> +static const struct lmu_bl_reg_data lm3697_channel_data[] = {
> +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED1_CFG_MASK,
> +	  LM3697_HVLED1_CFG_SHIFT },
> +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED2_CFG_MASK,
> +	  LM3697_HVLED2_CFG_SHIFT },
> +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED3_CFG_MASK,
> +	  LM3697_HVLED3_CFG_SHIFT },
> +};
> +
> +static const struct lmu_bl_reg_data lm3697_mode_data[] = {
> +	{ LM3697_REG_PWM_CFG, LM3697_PWM_A_MASK, LM3697_PWM_A_MASK },
> +	{ LM3697_REG_PWM_CFG, LM3697_PWM_B_MASK, LM3697_PWM_B_MASK },
> +};
> +
> +static const struct lmu_bl_reg_data lm3697_ramp_data[] = {
> +	{ LM3697_REG_BL0_RAMP, LM3697_RAMPUP_MASK, LM3697_RAMPUP_SHIFT },
> +	{ LM3697_REG_BL0_RAMP, LM3697_RAMPDN_MASK, LM3697_RAMPDN_SHIFT },
> +};
> +
> +static u8 lm3697_enable_reg = LM3697_REG_ENABLE;
> +
> +static u8 lm3697_brightness_msb_regs[] = {
> +	LM3697_REG_BRT_A_MSB,
> +	LM3697_REG_BRT_B_MSB,
> +};
> +
> +static u8 lm3697_brightness_lsb_regs[] = {
> +	LM3697_REG_BRT_A_LSB,
> +	LM3697_REG_BRT_B_LSB,
> +};
> +
> +static const struct ti_lmu_bl_reg lm3697_reg_info = {
> +	.init		 = lm3697_init_data,
> +	.num_init	 = ARRAY_SIZE(lm3697_init_data),
> +	.channel	 = lm3697_channel_data,
> +	.mode		 = lm3697_mode_data,
> +	.ramp		 = lm3697_ramp_data,
> +	.ramp_reg_offset = 1, /* For LM3697_REG_BL1_RAMPUP/DN */
> +	.enable		 = &lm3697_enable_reg,
> +	.brightness_msb	 = lm3697_brightness_msb_regs,
> +	.brightness_lsb	 = lm3697_brightness_lsb_regs,
> +};
> +
> +static int lm3532_ramp_table[] = { 0, 1, 2, 4, 8, 16, 32, 65 };
> +
> +static int lm3631_ramp_table[] = {
> +	   0,   1,   2,    5,   10,   20,   50,  100,
> +	 250, 500, 750, 1000, 1500, 2000, 3000, 4000,
> +};
> +
> +static int common_ramp_table[] = {
> +	   2, 250, 500, 1000, 2000, 4000, 8000, 16000,
> +};
> +
> +#define LM3532_MAX_CHANNELS		3
> +#define LM3631_MAX_CHANNELS		2
> +#define LM3632_MAX_CHANNELS		2
> +#define LM3633_MAX_CHANNELS		3
> +#define LM3695_MAX_CHANNELS		2
> +#define LM3697_MAX_CHANNELS		3
> +
> +const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID] = {
> +	{
> +		.reginfo		= &lm3532_reg_info,
> +		.num_channels		= LM3532_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_8BIT,
> +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> +		.ramp_table		= lm3532_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(lm3532_ramp_table),
> +	},
> +	{
> +		.reginfo		= &lm3631_reg_info,
> +		.num_channels		= LM3631_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_ONLY,
> +		.ramp_table		= lm3631_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(lm3631_ramp_table),
> +	},
> +	{
> +		.reginfo		= &lm3632_reg_info,
> +		.num_channels		= LM3632_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_ONLY,
> +	},
> +	{
> +		.reginfo		= &lm3633_reg_info,
> +		.num_channels		= LM3633_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_MAX_BRT,
> +		.ramp_table		= common_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(common_ramp_table),
> +		.fault_monitor_used	= true,
> +	},
> +	{
> +		.reginfo		= &lm3695_reg_info,
> +		.num_channels		= LM3695_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> +	},
> +	{
> +		.reginfo		= &lm3697_reg_info,
> +		.num_channels		= LM3697_MAX_CHANNELS,
> +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> +		.ramp_table		= common_ramp_table,
> +		.size_ramp		= ARRAY_SIZE(common_ramp_table),
> +		.fault_monitor_used	= true,
> +	},
> +};
> diff --git a/drivers/video/backlight/ti-lmu-backlight-data.h b/drivers/video/backlight/ti-lmu-backlight-data.h
> new file mode 100644
> index 000000000000..c64e8e6513e1
> --- /dev/null
> +++ b/drivers/video/backlight/ti-lmu-backlight-data.h
> @@ -0,0 +1,95 @@
> +/*

Inconsistent licensing here the core has
// SPDX-License-Identifier: GPL-2.0

> + * TI LMU (Lighting Management Unit) Backlight Device Data Definitions
> + *
> + * Copyright 2015 Texas Instruments
> + *
> + * Author: Milo Kim <milo.kim@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __TI_LMU_BACKLIGHT_H__
> +#define __TI_LMU_BACKLIGHT_H__
> +
> +#include <linux/mfd/ti-lmu.h>
> +#include <linux/mfd/ti-lmu-register.h>
> +
> +#define MAX_BRIGHTNESS_8BIT		255
> +#define MAX_BRIGHTNESS_11BIT		2047
> +
> +enum ti_lmu_bl_pwm_action {
> +	/* Update PWM duty, no brightness register update is required */
> +	UPDATE_PWM_ONLY,
> +	/* Update not only duty but also brightness register */
> +	UPDATE_PWM_AND_BRT_REGISTER,
> +	/* Update max value in brightness registers */
> +	UPDATE_MAX_BRT,
> +};
> +
> +struct lmu_bl_reg_data {
> +	u8 reg;
> +	u8 mask;
> +	u8 val;
> +};
> +
> +/**
> + * struct ti_lmu_bl_reg
> + *
> + * @init:		Device initialization registers
> + * @num_init:		Numbers of initialization registers
> + * @channel:		Backlight channel configuration registers
> + * @mode:		Brightness control mode registers
> + * @ramp:		Ramp registers for lighting effect
> + * @ramp_reg_offset:	Ramp register offset.
> + *			Only used for multiple ramp registers.
> + * @enable:		Enable control register address
> + * @enable_usec:	Delay time for updating enable register.
> + *			Unit is microsecond.
> + * @brightness_msb:	Brightness MSB(Upper 8 bits) registers.
> + *			Concatenated with LSB in 11 bit dimming mode.
> + *			In 8 bit dimming, only MSB is used.
> + * @brightness_lsb:	Brightness LSB(Lower 3 bits) registers.
> + *			Only valid in 11 bit dimming mode.
> + */
> +struct ti_lmu_bl_reg {
> +	const struct lmu_bl_reg_data *init;
> +	int num_init;
> +	const struct lmu_bl_reg_data *channel;
> +	const struct lmu_bl_reg_data *mode;
> +	const struct lmu_bl_reg_data *ramp;
> +	int ramp_reg_offset;
> +	u8 *enable;
> +	unsigned long enable_usec;
> +	u8 *brightness_msb;
> +	u8 *brightness_lsb;
> +};
> +
> +/**
> + * struct ti_lmu_bl_cfg
> + *
> + * @reginfo:		Device register configuration
> + * @num_channels:	Number of backlight channels
> + * @max_brightness:	Max brightness value of backlight device
> + * @pwm_action:		How to control brightness registers in PWM mode
> + * @ramp_table:		[Optional] Ramp time table for lighting effect.
> + *			It's used for searching approximate register index.
> + * @size_ramp:		[Optional] Size of ramp table
> + * @fault_monitor_used:	[Optional] Set true if the device needs to handle
> + *			LMU fault monitor event.
> + *
> + * This structure is used for device specific data configuration.
> + */
> +struct ti_lmu_bl_cfg {
> +	const struct ti_lmu_bl_reg *reginfo;
> +	int num_channels;
> +	int max_brightness;
> +	enum ti_lmu_bl_pwm_action pwm_action;
> +	int *ramp_table;
> +	int size_ramp;
> +	bool fault_monitor_used;
> +};
> +
> +extern const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID];
> +#endif
> 


-- 
------------------
Dan Murphy

Re: [PATCHv4 08/10] backlight: add TI LMU backlight driver

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 39324 bytes --]

Hi,

On Wed, Apr 04, 2018 at 01:30:37PM -0500, Dan Murphy wrote:
> Sebastian
> 
> -Milo Kim email is not valid

Thanks for your review! Milo was CC'd, since git took over the
SoB. I will make sure to avoid it next time.

> On 03/30/2018 12:24 PM, Sebastian Reichel wrote:
> > This adds backlight support for the following TI LMU
> > chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> > 
> > Signed-off-by: Milo Kim <milo.kim@ti.com>
> > [add LED subsystem support for keyboard backlight and rework DT
> >  binding according to Rob Herrings feedback]
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  drivers/video/backlight/Kconfig                 |   7 +
> >  drivers/video/backlight/Makefile                |   3 +
> >  drivers/video/backlight/ti-lmu-backlight-core.c | 666 ++++++++++++++++++++++++
> >  drivers/video/backlight/ti-lmu-backlight-data.c | 304 +++++++++++
> >  drivers/video/backlight/ti-lmu-backlight-data.h |  95 ++++
> >  5 files changed, 1075 insertions(+)
> >  create mode 100644 drivers/video/backlight/ti-lmu-backlight-core.c
> >  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.c
> >  create mode 100644 drivers/video/backlight/ti-lmu-backlight-data.h
> > 
> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > index 5d2d0d7e8100..27e6c5a0add8 100644
> > --- a/drivers/video/backlight/Kconfig
> > +++ b/drivers/video/backlight/Kconfig
> > @@ -427,6 +427,13 @@ config BACKLIGHT_SKY81452
> >  	  To compile this driver as a module, choose M here: the module will
> >  	  be called sky81452-backlight
> >  
> > +config BACKLIGHT_TI_LMU
> > +	tristate "Backlight driver for TI LMU"
> > +	depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
> 
> select REGMAP?

Right.

> > +	help
> > +	  Say Y to enable the backlight driver for TI LMU devices.
> > +	  This supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
> > +
> >  config BACKLIGHT_TPS65217
> >  	tristate "TPS65217 Backlight"
> >  	depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
> > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > index 19da71d518bf..a1132d3dfd4c 100644
> > --- a/drivers/video/backlight/Makefile
> > +++ b/drivers/video/backlight/Makefile
> > @@ -53,6 +53,9 @@ obj-$(CONFIG_BACKLIGHT_PM8941_WLED)	+= pm8941-wled.o
> >  obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
> >  obj-$(CONFIG_BACKLIGHT_SAHARA)		+= kb3886_bl.o
> >  obj-$(CONFIG_BACKLIGHT_SKY81452)	+= sky81452-backlight.o
> > +ti-lmu-backlight-objs			:= ti-lmu-backlight-core.o \
> > +					   ti-lmu-backlight-data.o
> > +obj-$(CONFIG_BACKLIGHT_TI_LMU)		+= ti-lmu-backlight.o
> >  obj-$(CONFIG_BACKLIGHT_TOSA)		+= tosa_bl.o
> >  obj-$(CONFIG_BACKLIGHT_TPS65217)	+= tps65217_bl.o
> >  obj-$(CONFIG_BACKLIGHT_WM831X)		+= wm831x_bl.o
> > diff --git a/drivers/video/backlight/ti-lmu-backlight-core.c b/drivers/video/backlight/ti-lmu-backlight-core.c
> > new file mode 100644
> > index 000000000000..a6099581edd7
> > --- /dev/null
> > +++ b/drivers/video/backlight/ti-lmu-backlight-core.c
> > @@ -0,0 +1,666 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2015 Texas Instruments
> > + * Copyright 2018 Sebastian Reichel
> > + *
> > + * TI LMU Backlight driver, based on previous work from
> > + * Milo Kim <milo.kim@ti.com>
> > + */
> > +
> > +#include <linux/backlight.h>
> > +#include <linux/bitops.h>
> > +#include <linux/device.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/ti-lmu.h>
> > +#include <linux/mfd/ti-lmu-register.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "ti-lmu-backlight-data.h"
> > +
> > +enum ti_lmu_bl_ctrl_mode {
> > +	BL_REGISTER_BASED,
> > +	BL_PWM_BASED,
> > +};
> > +
> > +enum ti_lmu_bl_type {
> > +	TI_LMU_BL,  /* backlight userspace interface */
> > +	TI_LMU_LED, /* led userspace interface */
> > +};
> > +
> > +enum ti_lmu_bl_ramp_mode {
> > +	BL_RAMP_UP,
> > +	BL_RAMP_DOWN,
> > +};
> > +
> > +#define DEFAULT_PWM_NAME			"lmu-backlight"
> 
> I don't see this used.

I guess I can remove this :)

> > +#define NUM_DUAL_CHANNEL			2
> > +#define LMU_BACKLIGHT_DUAL_CHANNEL_USED		(BIT(0) | BIT(1))
> > +#define LMU_BACKLIGHT_11BIT_LSB_MASK		(BIT(0) | BIT(1) | BIT(2))
> > +#define LMU_BACKLIGHT_11BIT_MSB_SHIFT		3
> > +
> 
> Struct kerneldoc?

Ok.

> > +struct ti_lmu_bank {
> > +	struct device *dev;
> > +	int bank_id;
> > +	const struct ti_lmu_bl_cfg *cfg;
> > +	struct ti_lmu *lmu;
> > +	const char *label;
> > +	int leds;
> > +	int current_brightness;
> > +	u32 default_brightness;
> > +	u32 ramp_up_msec;
> > +	u32 ramp_down_msec;
> > +	u32 pwm_period;
> > +	enum ti_lmu_bl_ctrl_mode mode;
> > +	enum ti_lmu_bl_type type;
> > +
> > +	struct notifier_block nb;
> > +
> > +	struct backlight_device *backlight;
> > +	struct led_classdev *led;
> > +};
> > +
> > +static int ti_lmu_bl_enable(struct ti_lmu_bank *lmu_bank, bool enable)
> > +{
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	unsigned long enable_time = lmu_bank->cfg->reginfo->enable_usec;
> > +	u8 *reg = lmu_bank->cfg->reginfo->enable;
> > +	u8 mask = BIT(lmu_bank->bank_id);
> > +	u8 val = (enable == true) ? mask : 0;
> > +	int ret;
> > +
> > +	if (!reg)
> > +		return -EINVAL;
> > +
> > +	ret = regmap_update_bits(regmap, *reg, mask, val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (enable_time > 0)
> > +		usleep_range(enable_time, enable_time + 100);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_update_brightness_register(struct ti_lmu_bank *lmu_bank,
> > +						       int brightness)
> > +{
> > +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	u8 reg, val;
> > +	int ret;
> > +
> > +	/*
> > +	 * Brightness register update
> > +	 *
> > +	 * 11 bit dimming: update LSB bits and write MSB byte.
> > +	 *		   MSB brightness should be shifted.
> > +	 *  8 bit dimming: write MSB byte.
> > +	 */
> > +	if (cfg->max_brightness == MAX_BRIGHTNESS_11BIT) {
> > +		reg = reginfo->brightness_lsb[lmu_bank->bank_id];
> > +		ret = regmap_update_bits(regmap, reg,
> > +					 LMU_BACKLIGHT_11BIT_LSB_MASK,
> > +					 brightness);
> > +		if (ret)
> > +			return ret;
> > +
> > +		val = brightness >> LMU_BACKLIGHT_11BIT_MSB_SHIFT;
> > +	} else {
> > +		val = brightness;
> > +	}
> > +
> > +	reg = reginfo->brightness_msb[lmu_bank->bank_id];
> > +	return regmap_write(regmap, reg, val);
> > +}
> > +
> > +static int ti_lmu_bl_pwm_ctrl(struct ti_lmu_bank *lmu_bank, int brightness)
> > +{
> > +	int max_brightness = lmu_bank->cfg->max_brightness;
> > +	struct pwm_state state = { };
> > +	int ret;
> > +
> > +	if (!lmu_bank->lmu->pwm) {
> > +		dev_err(lmu_bank->dev, "Missing PWM device!\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	pwm_init_state(lmu_bank->lmu->pwm, &state);
> > +	state.period = lmu_bank->pwm_period;
> > +	state.duty_cycle = brightness * state.period / max_brightness;
> > +
> > +	if (state.duty_cycle)
> > +		state.enabled = true;
> > +	else
> > +		state.enabled = false;
> > +
> > +	ret = pwm_apply_state(lmu_bank->lmu->pwm, &state);
> > +	if (ret)
> > +		dev_err(lmu_bank->dev, "Failed to configure PWM: %d", ret);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ti_lmu_bl_set_brightness(struct ti_lmu_bank *lmu_bank,
> > +				    int brightness)
> > +{
> > +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > +	bool enable = brightness > 0;
> > +	int ret;
> > +
> 
> Is there anyway that update_status and set_led_blocking can be
> called at the same time causing a race condition in this api?

Each bank is either exclusively exposed via LED or via backlight
subsystem.

> > +	ret = ti_lmu_bl_enable(lmu_bank, enable);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (lmu_bank->mode == BL_PWM_BASED) {
> > +		ti_lmu_bl_pwm_ctrl(lmu_bank, brightness);
> > +
> > +		switch (cfg->pwm_action) {
> > +		case UPDATE_PWM_ONLY:
> > +			/* No register update is required */
> > +			return 0;
> > +		case UPDATE_MAX_BRT:
> > +			/*
> > +			 * PWM can start from any non-zero code and dim down
> > +			 * to zero. So, brightness register should be updated
> > +			 * even in PWM mode.
> > +			 */
> > +			if (brightness > 0)
> > +				brightness = MAX_BRIGHTNESS_11BIT;
> > +			else
> > +				brightness = 0;
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +
> > +	lmu_bank->current_brightness = brightness;
> > +
> > +	return ti_lmu_bl_update_brightness_register(lmu_bank, brightness);
> > +}
> > +
> > +static int ti_lmu_bl_update_status(struct backlight_device *bl_dev)
> > +{
> > +	struct ti_lmu_bank *lmu_bank = bl_get_data(bl_dev);
> > +	int brightness = bl_dev->props.brightness;
> > +
> > +	if (bl_dev->props.state & BL_CORE_SUSPENDED)
> > +		brightness = 0;
> > +
> > +	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
> > +}
> > +
> > +static const struct backlight_ops lmu_backlight_ops = {
> > +	.options = BL_CORE_SUSPENDRESUME,
> > +	.update_status = ti_lmu_bl_update_status,
> > +};
> > +
> > +static int ti_lmu_bl_set_led_blocking(struct led_classdev *ledc,
> > +					     enum led_brightness value)
> > +{
> > +	struct ti_lmu_bank *lmu_bank = dev_get_drvdata(ledc->dev->parent);
> > +	int brightness = value;
> > +
> > +	return ti_lmu_bl_set_brightness(lmu_bank, brightness);
> > +}
> > +
> > +static int ti_lmu_bl_check_channel(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	const struct ti_lmu_bl_cfg *cfg = lmu_bank->cfg;
> > +	const struct ti_lmu_bl_reg *reginfo = cfg->reginfo;
> > +
> > +	if (!reginfo->brightness_msb)
> > +		return -EINVAL;
> > +
> > +	if (cfg->max_brightness > MAX_BRIGHTNESS_8BIT) {
> > +		if (!reginfo->brightness_lsb)
> > +			return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_create_channel(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	const struct lmu_bl_reg_data *regdata = lmu_bank->cfg->reginfo->channel;
> > +	int num_channels = lmu_bank->cfg->num_channels;
> > +	unsigned long led_sources = lmu_bank->leds;
> > +	int i, ret;
> > +	u8 shift;
> > +
> > +	/*
> > +	 * How to create backlight output channels:
> > +	 *   Check 'led_sources' bit and update registers.
> > +	 *
> > +	 *   1) Dual channel configuration
> > +	 *     The 1st register data is used for single channel.
> > +	 *     The 2nd register data is used for dual channel.
> > +	 *
> > +	 *   2) Multiple channel configuration
> > +	 *     Each register data is mapped to bank ID.
> > +	 *     Bit shift operation is defined in channel registers.
> > +	 *
> > +	 * Channel register data consists of address, mask, value.
> > +	 */
> > +
> > +	if (num_channels == NUM_DUAL_CHANNEL) {
> > +		if (led_sources == LMU_BACKLIGHT_DUAL_CHANNEL_USED)
> > +			regdata++;
> > +
> > +		return regmap_update_bits(regmap, regdata->reg, regdata->mask,
> > +					  regdata->val);
> > +	}
> > +
> > +	for (i = 0; regdata && i < num_channels; i++) {
> > +		/*
> > +		 * Note that the result of regdata->val is shift bit.
> > +		 * The bank_id should be shifted for the channel configuration.
> > +		 */
> > +		if (test_bit(i, &led_sources)) {
> > +			shift = regdata->val;
> > +			ret = regmap_update_bits(regmap, regdata->reg,
> > +						 regdata->mask,
> > +						 lmu_bank->bank_id << shift);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +		regdata++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_update_ctrl_mode(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	const struct lmu_bl_reg_data *regdata =
> > +		lmu_bank->cfg->reginfo->mode + lmu_bank->bank_id;
> > +	u8 val = regdata->val;
> > +
> > +	if (!regdata)
> > +		return 0;
> > +
> > +	/*
> > +	 * Update PWM configuration register.
> > +	 * If the mode is register based, then clear the bit.
> > +	 */
> > +	if (lmu_bank->mode != BL_PWM_BASED)
> > +		val = 0;
> > +
> > +	return regmap_update_bits(regmap, regdata->reg, regdata->mask, val);
> > +}
> > +
> > +static int ti_lmu_bl_convert_ramp_to_index(struct ti_lmu_bank *lmu_bank,
> > +						  enum ti_lmu_bl_ramp_mode mode)
> > +{
> > +	const int *ramp_table = lmu_bank->cfg->ramp_table;
> > +	const int size = lmu_bank->cfg->size_ramp;
> > +	unsigned int msec;
> > +	int i;
> > +
> > +	if (!ramp_table)
> > +		return -EINVAL;
> > +
> > +	switch (mode) {
> > +	case BL_RAMP_UP:
> > +		msec = lmu_bank->ramp_up_msec;
> > +		break;
> > +	case BL_RAMP_DOWN:
> > +		msec = lmu_bank->ramp_down_msec;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (msec <= ramp_table[0])
> > +		return 0;
> > +
> > +	if (msec > ramp_table[size - 1])
> > +		return size - 1;
> > +
> > +	for (i = 1; i < size; i++) {
> > +		if (msec == ramp_table[i])
> > +			return i;
> > +
> > +		/* Find an approximate index by looking up the table */
> > +		if (msec > ramp_table[i - 1] && msec < ramp_table[i]) {
> > +			if (msec - ramp_table[i - 1] < ramp_table[i] - msec)
> > +				return i - 1;
> > +			else
> > +				return i;
> > +		}
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +
> 
> Extra new line

oops.

> 
> > +static int ti_lmu_bl_set_ramp(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	const struct ti_lmu_bl_reg *reginfo = lmu_bank->cfg->reginfo;
> > +	int offset = reginfo->ramp_reg_offset;
> > +	int i, ret, index;
> > +	struct lmu_bl_reg_data regdata;
> > +
> > +	for (i = BL_RAMP_UP; i <= BL_RAMP_DOWN; i++) {
> > +		index = ti_lmu_bl_convert_ramp_to_index(lmu_bank, i);
> > +		if (index > 0) {
> > +			if (!reginfo->ramp)
> > +				break;
> > +
> > +			regdata = reginfo->ramp[i];
> > +			if (lmu_bank->bank_id != 0)
> > +				regdata.val += offset;
> > +
> > +			/* regdata.val is shift bit */
> > +			ret = regmap_update_bits(regmap, regdata.reg,
> > +						 regdata.mask,
> > +						 index << regdata.val);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_configure(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	int ret;
> > +
> > +	ret = ti_lmu_bl_check_channel(lmu_bank);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = ti_lmu_bl_create_channel(lmu_bank);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = ti_lmu_bl_update_ctrl_mode(lmu_bank);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ti_lmu_bl_set_ramp(lmu_bank);
> > +}
> > +
> > +static int ti_lmu_bl_register_backlight(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	struct backlight_device *bl_dev;
> > +	struct backlight_properties props;
> > +
> > +	if (lmu_bank->type != TI_LMU_BL)
> > +		return -EINVAL;
> > +
> > +	memset(&props, 0, sizeof(struct backlight_properties));
> > +	props.type = BACKLIGHT_PLATFORM;
> > +	props.brightness = lmu_bank->default_brightness;
> > +	props.max_brightness = lmu_bank->cfg->max_brightness;
> > +
> > +	bl_dev = devm_backlight_device_register(lmu_bank->dev, lmu_bank->label,
> > +						lmu_bank->dev, lmu_bank,
> > +						&lmu_backlight_ops, &props);
> > +	if (IS_ERR(bl_dev))
> > +		return PTR_ERR(bl_dev);
> > +
> > +	lmu_bank->backlight = bl_dev;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_register_led(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	int err;
> > +
> > +	if (lmu_bank->type != TI_LMU_LED)
> > +		return -EINVAL;
> > +
> > +	lmu_bank->led = devm_kzalloc(lmu_bank->dev, sizeof(*lmu_bank->led),
> > +				     GFP_KERNEL);
> > +	if (!lmu_bank->led)
> > +		return -ENOMEM;
> > +
> > +	lmu_bank->led->name = lmu_bank->label;
> > +	lmu_bank->led->max_brightness = lmu_bank->cfg->max_brightness;
> > +	lmu_bank->led->brightness_set_blocking =
> > +		ti_lmu_bl_set_led_blocking;
> > +
> > +	err = devm_led_classdev_register(lmu_bank->dev, lmu_bank->led);
> > +	if (err)
> > +		return err;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_add_device(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	switch (lmu_bank->type) {
> > +	case TI_LMU_BL:
> > +		return ti_lmu_bl_register_backlight(lmu_bank);
> > +	case TI_LMU_LED:
> > +		return ti_lmu_bl_register_led(lmu_bank);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int setup_of_node(struct platform_device *pdev)
> 
> This seems a bit generic.  Maybe keep with the naming convention
> 
> ti_lmu_setup_of_node

right.

> > +{
> > +	struct device_node *parent_node = pdev->dev.parent->of_node;
> > +	char *name;
> > +
> > +	if (!parent_node)
> > +		return 0;
> > +
> > +	name = kasprintf(GFP_KERNEL, "bank%d", pdev->id);
> > +	if (!name)
> > +		return -ENOMEM;
> > +
> > +	pdev->dev.of_node = of_get_child_by_name(parent_node, name);
> > +	kfree(name);
> > +
> > +	if (!pdev->dev.of_node)
> > +		return -ENODEV;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_parse_led_sources(struct device *dev)
> > +{
> > +	unsigned long mask = 0;
> > +	int ret;
> > +	int size, i;
> > +	u32 *leds;
> > +
> > +	size = device_property_read_u32_array(dev, "ti,led-sources", NULL, 0);
> > +	if (size <= 0) {
> > +		dev_err(dev, "Missing or malformed property led-sources: %d\n",
> > +			size);
> > +		return size < 0 ? size : -EINVAL;
> 
> Why the additional check? Why not just return -EINVAL?

I think it makes sense to pass the actual error code, but I can
simplify it when you insist.

> > +	}
> > +
> > +	leds = kmalloc_array(size, sizeof(u32), GFP_KERNEL);
> > +	if (!leds)
> > +		return -ENOMEM;
> > +
> > +	ret = device_property_read_u32_array(dev, "ti,led-sources", leds, size);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to read led-sources property: %d\n", ret);
> > +		goto out;
> > +	}
> > +
> > +	for (i = 0; i < size; i++)
> > +		set_bit(leds[i], &mask);
> > +
> > +	ret = mask;
> > +
> > +out:
> > +	kfree(leds);
> > +	return ret;
> > +}
> > +
> > +static int ti_lmu_bl_init(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	struct regmap *regmap = lmu_bank->lmu->regmap;
> > +	const struct lmu_bl_reg_data *regdata =
> > +		lmu_bank->cfg->reginfo->init;
> > +	int num_init = lmu_bank->cfg->reginfo->num_init;
> > +	int i, ret;
> > +
> > +	if (lmu_bank->lmu->backlight_initialized)
> > +		return 0;
> 
> Add a new line here

ok

> > +	lmu_bank->lmu->backlight_initialized = true;
> > +
> > +	for (i = 0; regdata && i < num_init; i++) {
> > +		ret = regmap_update_bits(regmap, regdata->reg, regdata->mask,
> > +					 regdata->val);
> > +		if (ret)
> > +			return ret;
> > +
> > +		regdata++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_reload(struct ti_lmu_bank *lmu_bank)
> > +{
> > +	int err;
> > +
> > +	ti_lmu_bl_init(lmu_bank);
> > +
> > +	err = ti_lmu_bl_configure(lmu_bank);
> > +	if (err)
> > +		return err;
> > +
> > +	return ti_lmu_bl_set_brightness(lmu_bank, lmu_bank->current_brightness);
> > +}
> > +
> > +static int ti_lmu_bl_monitor_notifier(struct notifier_block *nb,
> > +					     unsigned long action, void *unused)
> > +{
> > +	struct ti_lmu_bank *lmu_bank = container_of(nb, struct ti_lmu_bank, nb);
> > +
> > +	if (action == LMU_EVENT_MONITOR_DONE) {
> > +		if (ti_lmu_bl_reload(lmu_bank))
> > +			return NOTIFY_STOP;
> > +	}
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static int ti_lmu_bl_probe(struct platform_device *pdev)
> > +{
> > +	struct ti_lmu_bank *lmu_bank;
> > +	int err;
> > +
> > +	err = setup_of_node(pdev);
> > +	if (err)
> > +		return err;
> > +
> > +	lmu_bank = devm_kzalloc(&pdev->dev, sizeof(*lmu_bank), GFP_KERNEL);
> > +	if (!lmu_bank)
> > +		return -ENOMEM;
> 
> Add a new line here

ok

> > +	lmu_bank->dev = &pdev->dev;
> > +	dev_set_drvdata(&pdev->dev, lmu_bank);
> > +
> > +	err = device_property_read_string(&pdev->dev, "label",
> > +					  &lmu_bank->label);
> > +	if (err)
> > +		return err;
> > +
> > +	lmu_bank->type = TI_LMU_BL;
> > +	if (!strcmp(lmu_bank->label, "keyboard")) {
> > +		lmu_bank->type = TI_LMU_LED;
> > +		lmu_bank->label = "kbd_backlight";
> 
> What is the reason for changing the label?  Why can't the label in
> the DT be kbd_backlight?

It can, I tried to avoid Linuxism in DT.

> > +	}
> > +
> > +	lmu_bank->leds = ti_lmu_parse_led_sources(&pdev->dev);
> > +	if (lmu_bank->leds < 0)
> > +		return lmu_bank->leds;
> > +	else if (lmu_bank->leds == 0)
> > +		return -EINVAL;
> > +
> > +	device_property_read_u32(&pdev->dev, "default-brightness-level",
> > +				 &lmu_bank->default_brightness);
> > +	device_property_read_u32(&pdev->dev, "ti,ramp-up-ms",
> > +				 &lmu_bank->ramp_up_msec);
> > +	device_property_read_u32(&pdev->dev, "ti,ramp-down-ms",
> > +				 &lmu_bank->ramp_down_msec);
> > +	device_property_read_u32(&pdev->dev, "pwm-period",
> > +				 &lmu_bank->pwm_period);
> > +
> > +	if (lmu_bank->pwm_period > 0)
> > +		lmu_bank->mode = BL_PWM_BASED;
> > +	else
> > +		lmu_bank->mode = BL_REGISTER_BASED;
> > +
> > +	lmu_bank->lmu = dev_get_drvdata(pdev->dev.parent);
> > +	lmu_bank->cfg = &lmu_bl_cfg[lmu_bank->lmu->id];
> > +	lmu_bank->bank_id = pdev->id;
> > +
> > +	ti_lmu_bl_init(lmu_bank);
> > +
> > +	err = ti_lmu_bl_configure(lmu_bank);
> > +	if (err)
> > +		return err;
> > +
> > +	err = ti_lmu_bl_add_device(lmu_bank);
> > +	if (err)
> > +		return err;
> > +
> > +	err = ti_lmu_bl_set_brightness(lmu_bank,
> > +					      lmu_bank->default_brightness);
> > +	if (err)
> > +		return err;
> > +
> > +	/*
> > +	 * Notifier callback is required because backlight device needs
> > +	 * reconfiguration after fault detection procedure is done by
> > +	 * ti-lmu-fault-monitor driver.
> > +	 */
> > +	if (lmu_bank->cfg->fault_monitor_used) {
> > +		lmu_bank->nb.notifier_call = ti_lmu_bl_monitor_notifier;
> > +		err = blocking_notifier_chain_register(&lmu_bank->lmu->notifier,
> > +						       &lmu_bank->nb);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ti_lmu_bl_remove(struct platform_device *pdev)
> > +{
> > +	struct ti_lmu_bank *lmu_bank = platform_get_drvdata(pdev);
> > +
> > +	if (lmu_bank->cfg->fault_monitor_used)
> > +		blocking_notifier_chain_unregister(&lmu_bank->lmu->notifier,
> > +						   &lmu_bank->nb);
> > +
> > +	ti_lmu_bl_set_brightness(lmu_bank, 0);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver ti_lmu_bl_driver = {
> > +	.probe  = ti_lmu_bl_probe,
> > +	.remove  = ti_lmu_bl_remove,
> > +	.driver = {
> > +		.name = "ti-lmu-led-backlight",
> > +	},
> > +};
> > +module_platform_driver(ti_lmu_bl_driver)
> > +
> > +MODULE_DESCRIPTION("TI LMU Backlight LED Driver");
> > +MODULE_AUTHOR("Sebastian Reichel");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("platform:ti-lmu-led-backlight");
> > diff --git a/drivers/video/backlight/ti-lmu-backlight-data.c b/drivers/video/backlight/ti-lmu-backlight-data.c
> > new file mode 100644
> > index 000000000000..583136cb934d
> > --- /dev/null
> > +++ b/drivers/video/backlight/ti-lmu-backlight-data.c
> > @@ -0,0 +1,304 @@
> > +/*
> 
> Inconsistent licensing here the core has
> // SPDX-License-Identifier: GPL-2.0
> 
> > + * TI LMU (Lighting Management Unit) Backlight Device Data
> > + *
> > + * Copyright 2015 Texas Instruments
> 
> Copyright?

The original driver is from Milo Kim / TI. I changed almost all of
the core driver, so I took the liberty to update the copyright and
license header. The data part has been taken over almost unmodified,
so I kept the original header. I suppose TI is fine with using the
abbreviated header?

> > + * Author: Milo Kim <milo.kim@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include "ti-lmu-backlight-data.h"
> > +
> > +/* LM3532 */
> > +static const struct lmu_bl_reg_data lm3532_init_data[] = {
> > +	{ LM3532_REG_ZONE_CFG_A, LM3532_ZONE_MASK, LM3532_ZONE_0 },
> > +	{ LM3532_REG_ZONE_CFG_B, LM3532_ZONE_MASK, LM3532_ZONE_1 },
> > +	{ LM3532_REG_ZONE_CFG_C, LM3532_ZONE_MASK, LM3532_ZONE_2 },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3532_channel_data[] = {
> > +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED1_CFG_MASK,
> > +	  LM3532_ILED1_CFG_SHIFT },
> > +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED2_CFG_MASK,
> > +	  LM3532_ILED2_CFG_SHIFT },
> > +	{ LM3532_REG_OUTPUT_CFG, LM3532_ILED3_CFG_MASK,
> > +	  LM3532_ILED3_CFG_SHIFT },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3532_mode_data[] = {
> > +	{ LM3532_REG_PWM_A_CFG, LM3532_PWM_A_MASK, LM3532_PWM_ZONE_0 },
> > +	{ LM3532_REG_PWM_B_CFG, LM3532_PWM_B_MASK, LM3532_PWM_ZONE_1 },
> > +	{ LM3532_REG_PWM_C_CFG, LM3532_PWM_C_MASK, LM3532_PWM_ZONE_2 },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3532_ramp_data[] = {
> > +	{ LM3532_REG_RAMPUP, LM3532_RAMPUP_MASK, LM3532_RAMPUP_SHIFT },
> > +	{ LM3532_REG_RAMPDN, LM3532_RAMPDN_MASK, LM3532_RAMPDN_SHIFT },
> > +};
> > +
> > +static u8 lm3532_enable_reg = LM3532_REG_ENABLE;
> > +
> > +static u8 lm3532_brightness_regs[] = {
> > +	LM3532_REG_BRT_A,
> > +	LM3532_REG_BRT_B,
> > +	LM3532_REG_BRT_C,
> > +};
> > +
> > +static const struct ti_lmu_bl_reg lm3532_reg_info = {
> > +	.init		= lm3532_init_data,
> > +	.num_init	= ARRAY_SIZE(lm3532_init_data),
> > +	.channel	= lm3532_channel_data,
> > +	.mode		= lm3532_mode_data,
> > +	.ramp		= lm3532_ramp_data,
> > +	.enable		= &lm3532_enable_reg,
> > +	.brightness_msb	= lm3532_brightness_regs,
> > +};
> > +
> > +/* LM3631 */
> > +static const struct lmu_bl_reg_data lm3631_init_data[] = {
> > +	{ LM3631_REG_BRT_MODE, LM3631_MODE_MASK, LM3631_DEFAULT_MODE },
> > +	{ LM3631_REG_BL_CFG, LM3631_MAP_MASK, LM3631_EXPONENTIAL_MAP },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3631_channel_data[] = {
> > +	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_SINGLE_CHANNEL },
> > +	{ LM3631_REG_BL_CFG, LM3631_BL_CHANNEL_MASK, LM3631_BL_DUAL_CHANNEL },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3631_ramp_data[] = {
> > +	{ LM3631_REG_SLOPE, LM3631_SLOPE_MASK, LM3631_SLOPE_SHIFT },
> > +};
> > +
> > +static u8 lm3631_enable_reg = LM3631_REG_DEVCTRL;
> > +static u8 lm3631_brightness_msb_reg = LM3631_REG_BRT_MSB;
> > +static u8 lm3631_brightness_lsb_reg = LM3631_REG_BRT_LSB;
> > +
> > +static const struct ti_lmu_bl_reg lm3631_reg_info = {
> > +	.init		= lm3631_init_data,
> > +	.num_init	= ARRAY_SIZE(lm3631_init_data),
> > +	.channel	= lm3631_channel_data,
> > +	.ramp		= lm3631_ramp_data,
> > +	.enable		= &lm3631_enable_reg,
> > +	.brightness_msb	= &lm3631_brightness_msb_reg,
> > +	.brightness_lsb	= &lm3631_brightness_lsb_reg,
> > +};
> > +
> > +/* LM3632 */
> > +static const struct lmu_bl_reg_data lm3632_init_data[] = {
> > +	{ LM3632_REG_CONFIG1, LM3632_OVP_MASK, LM3632_OVP_25V },
> > +	{ LM3632_REG_CONFIG2, LM3632_SWFREQ_MASK, LM3632_SWFREQ_1MHZ },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3632_channel_data[] = {
> > +	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_SINGLE_CHANNEL },
> > +	{ LM3632_REG_ENABLE, LM3632_BL_CHANNEL_MASK, LM3632_BL_DUAL_CHANNEL },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3632_mode_data[] = {
> > +	{ LM3632_REG_IO_CTRL, LM3632_PWM_MASK, LM3632_PWM_MODE },
> > +};
> > +
> > +static u8 lm3632_enable_reg = LM3632_REG_ENABLE;
> > +static u8 lm3632_brightness_msb_reg = LM3632_REG_BRT_MSB;
> > +static u8 lm3632_brightness_lsb_reg = LM3632_REG_BRT_LSB;
> > +
> > +static const struct ti_lmu_bl_reg lm3632_reg_info = {
> > +	.init		= lm3632_init_data,
> > +	.num_init	= ARRAY_SIZE(lm3632_init_data),
> > +	.channel	= lm3632_channel_data,
> > +	.mode		= lm3632_mode_data,
> > +	.enable		= &lm3632_enable_reg,
> > +	.brightness_msb	= &lm3632_brightness_msb_reg,
> > +	.brightness_lsb	= &lm3632_brightness_lsb_reg,
> > +};
> > +
> > +/* LM3633 */
> > +static const struct lmu_bl_reg_data lm3633_init_data[] = {
> > +	{ LM3633_REG_BOOST_CFG, LM3633_OVP_MASK, LM3633_OVP_40V },
> > +	{ LM3633_REG_BL_RAMP_CONF, LM3633_BL_RAMP_MASK, LM3633_BL_RAMP_EACH },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3633_channel_data[] = {
> > +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED1_CFG_MASK,
> > +	  LM3633_HVLED1_CFG_SHIFT },
> > +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED2_CFG_MASK,
> > +	  LM3633_HVLED2_CFG_SHIFT },
> > +	{ LM3633_REG_HVLED_OUTPUT_CFG, LM3633_HVLED3_CFG_MASK,
> > +	  LM3633_HVLED3_CFG_SHIFT },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3633_mode_data[] = {
> > +	{ LM3633_REG_PWM_CFG, LM3633_PWM_A_MASK, LM3633_PWM_A_MASK },
> > +	{ LM3633_REG_PWM_CFG, LM3633_PWM_B_MASK, LM3633_PWM_B_MASK },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3633_ramp_data[] = {
> > +	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPUP_MASK, LM3633_BL_RAMPUP_SHIFT },
> > +	{ LM3633_REG_BL0_RAMP, LM3633_BL_RAMPDN_MASK, LM3633_BL_RAMPDN_SHIFT },
> > +};
> > +
> > +static u8 lm3633_enable_reg = LM3633_REG_ENABLE;
> > +
> > +static u8 lm3633_brightness_msb_regs[] = {
> > +	LM3633_REG_BRT_HVLED_A_MSB,
> > +	LM3633_REG_BRT_HVLED_B_MSB,
> > +};
> > +
> > +static u8 lm3633_brightness_lsb_regs[] = {
> > +	LM3633_REG_BRT_HVLED_A_LSB,
> > +	LM3633_REG_BRT_HVLED_B_LSB,
> > +};
> > +
> > +static const struct ti_lmu_bl_reg lm3633_reg_info = {
> > +	.init		 = lm3633_init_data,
> > +	.num_init	 = ARRAY_SIZE(lm3633_init_data),
> > +	.channel	 = lm3633_channel_data,
> > +	.mode		 = lm3633_mode_data,
> > +	.ramp		 = lm3633_ramp_data,
> > +	.ramp_reg_offset = 1, /* For LM3633_REG_BL1_RAMPUP/DN */
> > +	.enable		 = &lm3633_enable_reg,
> > +	.brightness_msb	 = lm3633_brightness_msb_regs,
> > +	.brightness_lsb	 = lm3633_brightness_lsb_regs,
> > +};
> > +
> > +/* LM3695 */
> > +static const struct lmu_bl_reg_data lm3695_init_data[] = {
> > +	{ LM3695_REG_GP, LM3695_BRT_RW_MASK, LM3695_BRT_RW_MASK },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3695_channel_data[] = {
> > +	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_SINGLE_CHANNEL },
> > +	{ LM3695_REG_GP, LM3695_BL_CHANNEL_MASK, LM3695_BL_DUAL_CHANNEL },
> > +};
> > +
> > +static u8 lm3695_enable_reg = LM3695_REG_GP;
> > +static u8 lm3695_brightness_msb_reg = LM3695_REG_BRT_MSB;
> > +static u8 lm3695_brightness_lsb_reg = LM3695_REG_BRT_LSB;
> > +
> > +static const struct ti_lmu_bl_reg lm3695_reg_info = {
> > +	.init		= lm3695_init_data,
> > +	.num_init	= ARRAY_SIZE(lm3695_init_data),
> > +	.channel	= lm3695_channel_data,
> > +	.enable		= &lm3695_enable_reg,
> > +	.enable_usec	= 600,
> > +	.brightness_msb	= &lm3695_brightness_msb_reg,
> > +	.brightness_lsb	= &lm3695_brightness_lsb_reg,
> > +};
> > +
> > +/* LM3697 */
> > +static const struct lmu_bl_reg_data lm3697_init_data[] = {
> > +	{ LM3697_REG_RAMP_CONF, LM3697_RAMP_MASK, LM3697_RAMP_EACH },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3697_channel_data[] = {
> > +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED1_CFG_MASK,
> > +	  LM3697_HVLED1_CFG_SHIFT },
> > +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED2_CFG_MASK,
> > +	  LM3697_HVLED2_CFG_SHIFT },
> > +	{ LM3697_REG_HVLED_OUTPUT_CFG, LM3697_HVLED3_CFG_MASK,
> > +	  LM3697_HVLED3_CFG_SHIFT },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3697_mode_data[] = {
> > +	{ LM3697_REG_PWM_CFG, LM3697_PWM_A_MASK, LM3697_PWM_A_MASK },
> > +	{ LM3697_REG_PWM_CFG, LM3697_PWM_B_MASK, LM3697_PWM_B_MASK },
> > +};
> > +
> > +static const struct lmu_bl_reg_data lm3697_ramp_data[] = {
> > +	{ LM3697_REG_BL0_RAMP, LM3697_RAMPUP_MASK, LM3697_RAMPUP_SHIFT },
> > +	{ LM3697_REG_BL0_RAMP, LM3697_RAMPDN_MASK, LM3697_RAMPDN_SHIFT },
> > +};
> > +
> > +static u8 lm3697_enable_reg = LM3697_REG_ENABLE;
> > +
> > +static u8 lm3697_brightness_msb_regs[] = {
> > +	LM3697_REG_BRT_A_MSB,
> > +	LM3697_REG_BRT_B_MSB,
> > +};
> > +
> > +static u8 lm3697_brightness_lsb_regs[] = {
> > +	LM3697_REG_BRT_A_LSB,
> > +	LM3697_REG_BRT_B_LSB,
> > +};
> > +
> > +static const struct ti_lmu_bl_reg lm3697_reg_info = {
> > +	.init		 = lm3697_init_data,
> > +	.num_init	 = ARRAY_SIZE(lm3697_init_data),
> > +	.channel	 = lm3697_channel_data,
> > +	.mode		 = lm3697_mode_data,
> > +	.ramp		 = lm3697_ramp_data,
> > +	.ramp_reg_offset = 1, /* For LM3697_REG_BL1_RAMPUP/DN */
> > +	.enable		 = &lm3697_enable_reg,
> > +	.brightness_msb	 = lm3697_brightness_msb_regs,
> > +	.brightness_lsb	 = lm3697_brightness_lsb_regs,
> > +};
> > +
> > +static int lm3532_ramp_table[] = { 0, 1, 2, 4, 8, 16, 32, 65 };
> > +
> > +static int lm3631_ramp_table[] = {
> > +	   0,   1,   2,    5,   10,   20,   50,  100,
> > +	 250, 500, 750, 1000, 1500, 2000, 3000, 4000,
> > +};
> > +
> > +static int common_ramp_table[] = {
> > +	   2, 250, 500, 1000, 2000, 4000, 8000, 16000,
> > +};
> > +
> > +#define LM3532_MAX_CHANNELS		3
> > +#define LM3631_MAX_CHANNELS		2
> > +#define LM3632_MAX_CHANNELS		2
> > +#define LM3633_MAX_CHANNELS		3
> > +#define LM3695_MAX_CHANNELS		2
> > +#define LM3697_MAX_CHANNELS		3
> > +
> > +const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID] = {
> > +	{
> > +		.reginfo		= &lm3532_reg_info,
> > +		.num_channels		= LM3532_MAX_CHANNELS,
> > +		.max_brightness		= MAX_BRIGHTNESS_8BIT,
> > +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> > +		.ramp_table		= lm3532_ramp_table,
> > +		.size_ramp		= ARRAY_SIZE(lm3532_ramp_table),
> > +	},
> > +	{
> > +		.reginfo		= &lm3631_reg_info,
> > +		.num_channels		= LM3631_MAX_CHANNELS,
> > +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> > +		.pwm_action		= UPDATE_PWM_ONLY,
> > +		.ramp_table		= lm3631_ramp_table,
> > +		.size_ramp		= ARRAY_SIZE(lm3631_ramp_table),
> > +	},
> > +	{
> > +		.reginfo		= &lm3632_reg_info,
> > +		.num_channels		= LM3632_MAX_CHANNELS,
> > +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> > +		.pwm_action		= UPDATE_PWM_ONLY,
> > +	},
> > +	{
> > +		.reginfo		= &lm3633_reg_info,
> > +		.num_channels		= LM3633_MAX_CHANNELS,
> > +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> > +		.pwm_action		= UPDATE_MAX_BRT,
> > +		.ramp_table		= common_ramp_table,
> > +		.size_ramp		= ARRAY_SIZE(common_ramp_table),
> > +		.fault_monitor_used	= true,
> > +	},
> > +	{
> > +		.reginfo		= &lm3695_reg_info,
> > +		.num_channels		= LM3695_MAX_CHANNELS,
> > +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> > +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> > +	},
> > +	{
> > +		.reginfo		= &lm3697_reg_info,
> > +		.num_channels		= LM3697_MAX_CHANNELS,
> > +		.max_brightness		= MAX_BRIGHTNESS_11BIT,
> > +		.pwm_action		= UPDATE_PWM_AND_BRT_REGISTER,
> > +		.ramp_table		= common_ramp_table,
> > +		.size_ramp		= ARRAY_SIZE(common_ramp_table),
> > +		.fault_monitor_used	= true,
> > +	},
> > +};
> > diff --git a/drivers/video/backlight/ti-lmu-backlight-data.h b/drivers/video/backlight/ti-lmu-backlight-data.h
> > new file mode 100644
> > index 000000000000..c64e8e6513e1
> > --- /dev/null
> > +++ b/drivers/video/backlight/ti-lmu-backlight-data.h
> > @@ -0,0 +1,95 @@
> > +/*
> 
> Inconsistent licensing here the core has
> // SPDX-License-Identifier: GPL-2.0

See above.

> > + * TI LMU (Lighting Management Unit) Backlight Device Data Definitions
> > + *
> > + * Copyright 2015 Texas Instruments
> > + *
> > + * Author: Milo Kim <milo.kim@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __TI_LMU_BACKLIGHT_H__
> > +#define __TI_LMU_BACKLIGHT_H__
> > +
> > +#include <linux/mfd/ti-lmu.h>
> > +#include <linux/mfd/ti-lmu-register.h>
> > +
> > +#define MAX_BRIGHTNESS_8BIT		255
> > +#define MAX_BRIGHTNESS_11BIT		2047
> > +
> > +enum ti_lmu_bl_pwm_action {
> > +	/* Update PWM duty, no brightness register update is required */
> > +	UPDATE_PWM_ONLY,
> > +	/* Update not only duty but also brightness register */
> > +	UPDATE_PWM_AND_BRT_REGISTER,
> > +	/* Update max value in brightness registers */
> > +	UPDATE_MAX_BRT,
> > +};
> > +
> > +struct lmu_bl_reg_data {
> > +	u8 reg;
> > +	u8 mask;
> > +	u8 val;
> > +};
> > +
> > +/**
> > + * struct ti_lmu_bl_reg
> > + *
> > + * @init:		Device initialization registers
> > + * @num_init:		Numbers of initialization registers
> > + * @channel:		Backlight channel configuration registers
> > + * @mode:		Brightness control mode registers
> > + * @ramp:		Ramp registers for lighting effect
> > + * @ramp_reg_offset:	Ramp register offset.
> > + *			Only used for multiple ramp registers.
> > + * @enable:		Enable control register address
> > + * @enable_usec:	Delay time for updating enable register.
> > + *			Unit is microsecond.
> > + * @brightness_msb:	Brightness MSB(Upper 8 bits) registers.
> > + *			Concatenated with LSB in 11 bit dimming mode.
> > + *			In 8 bit dimming, only MSB is used.
> > + * @brightness_lsb:	Brightness LSB(Lower 3 bits) registers.
> > + *			Only valid in 11 bit dimming mode.
> > + */
> > +struct ti_lmu_bl_reg {
> > +	const struct lmu_bl_reg_data *init;
> > +	int num_init;
> > +	const struct lmu_bl_reg_data *channel;
> > +	const struct lmu_bl_reg_data *mode;
> > +	const struct lmu_bl_reg_data *ramp;
> > +	int ramp_reg_offset;
> > +	u8 *enable;
> > +	unsigned long enable_usec;
> > +	u8 *brightness_msb;
> > +	u8 *brightness_lsb;
> > +};
> > +
> > +/**
> > + * struct ti_lmu_bl_cfg
> > + *
> > + * @reginfo:		Device register configuration
> > + * @num_channels:	Number of backlight channels
> > + * @max_brightness:	Max brightness value of backlight device
> > + * @pwm_action:		How to control brightness registers in PWM mode
> > + * @ramp_table:		[Optional] Ramp time table for lighting effect.
> > + *			It's used for searching approximate register index.
> > + * @size_ramp:		[Optional] Size of ramp table
> > + * @fault_monitor_used:	[Optional] Set true if the device needs to handle
> > + *			LMU fault monitor event.
> > + *
> > + * This structure is used for device specific data configuration.
> > + */
> > +struct ti_lmu_bl_cfg {
> > +	const struct ti_lmu_bl_reg *reginfo;
> > +	int num_channels;
> > +	int max_brightness;
> > +	enum ti_lmu_bl_pwm_action pwm_action;
> > +	int *ramp_table;
> > +	int size_ramp;
> > +	bool fault_monitor_used;
> > +};
> > +
> > +extern const struct ti_lmu_bl_cfg lmu_bl_cfg[LMU_MAX_ID];
> > +#endif
> > 

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv4 09/10] dt-bindings: mfd: ti-lmu: update for backlight

[Sebastian Reichel]

Update binding to integrate the backlight feature directly into
the main node, as suggested by Rob Herring.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 Documentation/devicetree/bindings/mfd/ti-lmu.txt | 119 ++++++++++++-----------
 1 file changed, 60 insertions(+), 59 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index c885cf89b8ce..b3433e95dfa5 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -28,10 +28,9 @@ Required properties:
 
 Optional property:
   - enable-gpios: A GPIO specifier for hardware enable pin.
-
-Required node:
-  - backlight: All LMU devices have backlight child nodes.
-               For the properties, please refer to [1].
+  - pwm-names: Should be either "lmu-backlight" or unset
+  - pwm: This should be a PWM specifier following ../pwm/pwm.txt and must
+         only be specified, if the backlight should be used in PWM mode.
 
 Optional nodes:
   - fault-monitor: Hardware fault monitoring driver for LM3633 and LM3697.
@@ -42,8 +41,31 @@ Optional nodes:
   - leds: LED properties for LM3633. Please refer to [2].
   - regulators: Regulator properties for LM3631 and LM3632.
                 Please refer to [3].
+  - bank0, bank1, bank2: This contains the backlight configuration
+                         for each backlight control bank.
+
+Required properties in the bank subnodes:
+  - label: A string describing the backlight. Should contain "keyboard"
+           for a keyboard backlight and "lcd" for LCD panel backlights.
+  - ti,led-sources: A list of channels, that should be driven. Each channel
+                    should only be driven by one bank.
+
+Optional properties in the bank subnodes:
+  - default-brightness-level: Backlight initial brightness value.
+                              Type is <u32>. It is set as soon as backlight
+			      device is created.
+			      0 ~ 2047 = LM3631, LM3632, LM3633, LM3695 and
+					 LM3697
+			      0 ~ 255  = LM3532
+  - ti,ramp-up-ms, ti,ramp-down-ms: Light dimming effect properties.
+				    Type is <u32>. Unit is millisecond.
+					0 ~ 65 ms    = LM3532
+					0 ~ 4000 ms  = LM3631
+					0 ~ 16000 ms = LM3633 and LM3697
+  - pwm-period: PWM period. Only valid in PWM brightness mode.
+                Type is <u32>. If this property is missing, then control
+		mode is set to I2C by default.
 
-[1] ../leds/backlight/ti-lmu-backlight.txt
 [2] ../leds/leds-lm3633.txt
 [3] ../regulator/lm363x-regulator.txt
 
@@ -53,14 +75,11 @@ lm3532@38 {
 
 	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
 
-	backlight {
-		compatible = "ti,lm3532-backlight";
-
-		lcd {
-			led-sources = <0 1 2>;
-			ramp-up-msec = <30>;
-			ramp-down-msec = <0>;
-		};
+	bank0 {
+		label = "lcd";
+		led-sources = <0 1 2>;
+		ramp-up-msec = <30>;
+		ramp-down-msec = <0>;
 	};
 };
 
@@ -105,13 +124,10 @@ lm3631@29 {
 		};
 	};
 
-	backlight {
-		compatible = "ti,lm3631-backlight";
-
-		lcd_bl {
-			led-sources = <0 1>;
-			ramp-up-msec = <300>;
-		};
+	bank0 {
+		label = "lcd_bl";
+		led-sources = <0 1>;
+		ramp-up-msec = <300>;
 	};
 };
 
@@ -147,16 +163,13 @@ lm3632@11 {
 		};
 	};
 
-	backlight {
-		compatible = "ti,lm3632-backlight";
-
-		pwms = <&pwm0 0 10000 0>; /* pwm number, period, polarity */
-		pwm-names = "lmu-backlight";
+	pwms = <&pwm0 0 10000 0>; /* pwm number, period, polarity */
+	pwm-names = "lmu-backlight";
 
-		lcd {
-			led-sources = <0 1>;
-			pwm-period = <10000>;
-		};
+	bank0 {
+		label = "lcd";
+		led-sources = <0 1>;
+		pwm-period = <10000>;
 	};
 };
 
@@ -166,22 +179,18 @@ lm3633@36 {
 
 	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
 
-	backlight {
-		compatible = "ti,lm3633-backlight";
-
-		main {
-			label = "main_lcd";
-			led-sources = <1 2>;
-			ramp-up-msec = <500>;
-			ramp-down-msec = <500>;
-		};
+	bank0 {
+		label = "main_lcd";
+		led-sources = <1 2>;
+		ramp-up-msec = <500>;
+		ramp-down-msec = <500>;
+	};
 
-		front {
-			label = "front_lcd";
-			led-sources = <0>;
-			ramp-up-msec = <1000>;
-			ramp-down-msec = <0>;
-		};
+	bank1 {
+		label = "front_lcd";
+		led-sources = <0>;
+		ramp-up-msec = <1000>;
+		ramp-down-msec = <0>;
 	};
 
 	leds {
@@ -211,13 +220,9 @@ lm3695@63 {
 
 	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
 
-	backlight {
-		compatible = "ti,lm3695-backlight";
-
-		lcd {
-			label = "bl";
-			led-sources = <0 1>;
-		};
+	bank0 {
+		label = "lcd_bl";
+		led-sources = <0 1>;
 	};
 };
 
@@ -227,14 +232,10 @@ lm3697@36 {
 
 	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
 
-	backlight {
-		compatible = "ti,lm3697-backlight";
-
-		lcd {
-			led-sources = <0 1 2>;
-			ramp-up-msec = <200>;
-			ramp-down-msec = <200>;
-		};
+	bank0 {
+		led-sources = <0 1 2>;
+		ramp-up-msec = <200>;
+		ramp-down-msec = <200>;
 	};
 
 	fault-monitor {
-- 
2.16.2

Re: [PATCHv4 09/10] dt-bindings: mfd: ti-lmu: update for backlight

[Rob Herring]

On Fri, Mar 30, 2018 at 07:24:13PM +0200, Sebastian Reichel wrote:
> Update binding to integrate the backlight feature directly into
> the main node, as suggested by Rob Herring.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  Documentation/devicetree/bindings/mfd/ti-lmu.txt | 119 ++++++++++++-----------
>  1 file changed, 60 insertions(+), 59 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> index c885cf89b8ce..b3433e95dfa5 100644
> --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> @@ -28,10 +28,9 @@ Required properties:
>  
>  Optional property:
>    - enable-gpios: A GPIO specifier for hardware enable pin.
> -
> -Required node:
> -  - backlight: All LMU devices have backlight child nodes.
> -               For the properties, please refer to [1].
> +  - pwm-names: Should be either "lmu-backlight" or unset
> +  - pwm: This should be a PWM specifier following ../pwm/pwm.txt and must
> +         only be specified, if the backlight should be used in PWM mode.
>  
>  Optional nodes:
>    - fault-monitor: Hardware fault monitoring driver for LM3633 and LM3697.
> @@ -42,8 +41,31 @@ Optional nodes:
>    - leds: LED properties for LM3633. Please refer to [2].
>    - regulators: Regulator properties for LM3631 and LM3632.
>                  Please refer to [3].
> +  - bank0, bank1, bank2: This contains the backlight configuration
> +                         for each backlight control bank.
> +
> +Required properties in the bank subnodes:
> +  - label: A string describing the backlight. Should contain "keyboard"
> +           for a keyboard backlight and "lcd" for LCD panel backlights.
> +  - ti,led-sources: A list of channels, that should be driven. Each channel
> +                    should only be driven by one bank.
> +
> +Optional properties in the bank subnodes:
> +  - default-brightness-level: Backlight initial brightness value.
> +                              Type is <u32>. It is set as soon as backlight
> +			      device is created.
> +			      0 ~ 2047 = LM3631, LM3632, LM3633, LM3695 and
> +					 LM3697
> +			      0 ~ 255  = LM3532
> +  - ti,ramp-up-ms, ti,ramp-down-ms: Light dimming effect properties.
> +				    Type is <u32>. Unit is millisecond.
> +					0 ~ 65 ms    = LM3532
> +					0 ~ 4000 ms  = LM3631
> +					0 ~ 16000 ms = LM3633 and LM3697
> +  - pwm-period: PWM period. Only valid in PWM brightness mode.
> +                Type is <u32>. If this property is missing, then control
> +		mode is set to I2C by default.
>  
> -[1] ../leds/backlight/ti-lmu-backlight.txt
>  [2] ../leds/leds-lm3633.txt
>  [3] ../regulator/lm363x-regulator.txt
>  
> @@ -53,14 +75,11 @@ lm3532@38 {
>  
>  	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
>  
> -	backlight {
> -		compatible = "ti,lm3532-backlight";
> -
> -		lcd {
> -			led-sources = <0 1 2>;
> -			ramp-up-msec = <30>;
> -			ramp-down-msec = <0>;
> -		};
> +	bank0 {
> +		label = "lcd";
> +		led-sources = <0 1 2>;
> +		ramp-up-msec = <30>;
> +		ramp-down-msec = <0>;

Looks like it was already wrong, but these are missing the vendor 
prefixes.

>  	};
>  };
>  
> @@ -105,13 +124,10 @@ lm3631@29 {

Are so many examples really needed? Examples aren't supposed to 
enumerate all possible options...

Rob

Re: [PATCHv4 09/10] dt-bindings: mfd: ti-lmu: update for backlight

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3812 bytes --]

Hi,

On Mon, Apr 09, 2018 at 04:06:38PM -0500, Rob Herring wrote:
> On Fri, Mar 30, 2018 at 07:24:13PM +0200, Sebastian Reichel wrote:
> > Update binding to integrate the backlight feature directly into
> > the main node, as suggested by Rob Herring.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> > ---
> >  Documentation/devicetree/bindings/mfd/ti-lmu.txt | 119 ++++++++++++-----------
> >  1 file changed, 60 insertions(+), 59 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> > index c885cf89b8ce..b3433e95dfa5 100644
> > --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> > +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> > @@ -28,10 +28,9 @@ Required properties:
> >  
> >  Optional property:
> >    - enable-gpios: A GPIO specifier for hardware enable pin.
> > -
> > -Required node:
> > -  - backlight: All LMU devices have backlight child nodes.
> > -               For the properties, please refer to [1].
> > +  - pwm-names: Should be either "lmu-backlight" or unset
> > +  - pwm: This should be a PWM specifier following ../pwm/pwm.txt and must
> > +         only be specified, if the backlight should be used in PWM mode.
> >  
> >  Optional nodes:
> >    - fault-monitor: Hardware fault monitoring driver for LM3633 and LM3697.
> > @@ -42,8 +41,31 @@ Optional nodes:
> >    - leds: LED properties for LM3633. Please refer to [2].
> >    - regulators: Regulator properties for LM3631 and LM3632.
> >                  Please refer to [3].
> > +  - bank0, bank1, bank2: This contains the backlight configuration
> > +                         for each backlight control bank.
> > +
> > +Required properties in the bank subnodes:
> > +  - label: A string describing the backlight. Should contain "keyboard"
> > +           for a keyboard backlight and "lcd" for LCD panel backlights.
> > +  - ti,led-sources: A list of channels, that should be driven. Each channel
> > +                    should only be driven by one bank.
> > +
> > +Optional properties in the bank subnodes:
> > +  - default-brightness-level: Backlight initial brightness value.
> > +                              Type is <u32>. It is set as soon as backlight
> > +			      device is created.
> > +			      0 ~ 2047 = LM3631, LM3632, LM3633, LM3695 and
> > +					 LM3697
> > +			      0 ~ 255  = LM3532
> > +  - ti,ramp-up-ms, ti,ramp-down-ms: Light dimming effect properties.
> > +				    Type is <u32>. Unit is millisecond.
> > +					0 ~ 65 ms    = LM3532
> > +					0 ~ 4000 ms  = LM3631
> > +					0 ~ 16000 ms = LM3633 and LM3697
> > +  - pwm-period: PWM period. Only valid in PWM brightness mode.
> > +                Type is <u32>. If this property is missing, then control
> > +		mode is set to I2C by default.
> >  
> > -[1] ../leds/backlight/ti-lmu-backlight.txt
> >  [2] ../leds/leds-lm3633.txt
> >  [3] ../regulator/lm363x-regulator.txt
> >  
> > @@ -53,14 +75,11 @@ lm3532@38 {
> >  
> >  	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
> >  
> > -	backlight {
> > -		compatible = "ti,lm3532-backlight";
> > -
> > -		lcd {
> > -			led-sources = <0 1 2>;
> > -			ramp-up-msec = <30>;
> > -			ramp-down-msec = <0>;
> > -		};
> > +	bank0 {
> > +		label = "lcd";
> > +		led-sources = <0 1 2>;
> > +		ramp-up-msec = <30>;
> > +		ramp-down-msec = <0>;
> 
> Looks like it was already wrong, but these are missing the vendor 
> prefixes.

Oops. I will add the vendor prefix.

> 
> >  	};
> >  };
> >  
> > @@ -105,13 +124,10 @@ lm3631@29 {
> 
> Are so many examples really needed? Examples aren't supposed to 
> enumerate all possible options...

I will remove a few in the next version.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCHv4 10/10] ARM: dts: omap4-droid4: update backlight led-controller

[Sebastian Reichel]

From: Sebastian Reichel <sre@kernel.org>

This updates the backlight led-controller node to follow the
new binding instead of the legacy out-of-tree binding.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 arch/arm/boot/dts/omap4-droid4-xt894.dts | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index 3ed090a9db6c..da1571c24039 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -380,20 +380,24 @@
 };
 
 &i2c1 {
-	lm3532@38 {
+	led-controller@38 {
 		compatible = "ti,lm3532";
 		reg = <0x38>;
 
 		enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
 
-		lcd_backlight: backlight {
-			compatible = "ti,lm3532-backlight";
+		lcd_backlight: bank0 {
+			label = "lcd";
+			ti,led-sources = <0>;
+			ti,ramp-up-ms = <1>;
+			ti,ramp-down-ms = <0>;
+		};
 
-			lcd {
-				led-sources = <0 1 2>;
-				ramp-up-msec = <1>;
-				ramp-down-msec = <0>;
-			};
+		keyboard_backlight: bank1 {
+			label = "keyboard";
+			ti,led-sources = <1>;
+			ti,ramp-up-msec = <1>;
+			ti,ramp-down-msec = <0>;
 		};
 	};
 };
-- 
2.16.2

Re: [PATCHv4 10/10] ARM: dts: omap4-droid4: update backlight led-controller

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

On Fri 2018-03-30 19:24:14, Sebastian Reichel wrote:
> From: Sebastian Reichel <sre@kernel.org>
> 
> This updates the backlight led-controller node to follow the
> new binding instead of the legacy out-of-tree binding.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

9,10: Acked-by: Pavel Machek <pavel@ucw.cz>
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]