diff mbox series

[v6,5/5] leds: lm36274: Introduce the TI LM36274 LED driver

Message ID 20190605125634.7042-6-dmurphy@ti.com
State New
Headers show
Series LM36274 Introduction | expand

Commit Message

Dan Murphy June 5, 2019, 12:56 p.m. UTC
Introduce the LM36274 LED driver.  This driver uses the ti-lmu
MFD driver to probe this LED driver.  The driver configures only the
LED registers and enables the outputs according to the config file.

The driver utilizes the TI LMU (Lighting Management Unit) LED common
framework to set the brightness bits.

Signed-off-by: Dan Murphy <dmurphy@ti.com>

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

---
 drivers/leds/Kconfig        |   8 ++
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++++++++++++++++++++
 3 files changed, 183 insertions(+)
 create mode 100644 drivers/leds/leds-lm36274.c

-- 
2.21.0.5.gaeb582a983

Comments

Pavel Machek June 6, 2019, 10:07 a.m. UTC | #1
Hi!

> Introduce the LM36274 LED driver.  This driver uses the ti-lmu

> MFD driver to probe this LED driver.  The driver configures only the

> LED registers and enables the outputs according to the config file.

> 

> The driver utilizes the TI LMU (Lighting Management Unit) LED common

> framework to set the brightness bits.


Nothing too bad, but...

> +static int lm36274_parse_dt(struct lm36274 *lm36274_data)

> +{

> +	struct fwnode_handle *child = NULL;

> +	char label[LED_MAX_NAME_SIZE];

> +	struct device *dev = &lm36274_data->pdev->dev;

> +	const char *name;

> +	int child_cnt;

> +	int ret = -EINVAL;

> +

> +	/* There should only be 1 node */

> +	child_cnt = device_get_child_node_count(dev);

> +	if (child_cnt != 1)

> +		return ret;


I'd do direct "return -EINVAL" here.

> +	device_for_each_child_node(dev, child) {

> +		ret = fwnode_property_read_string(child, "label", &name);

> +		if (ret)

> +			snprintf(label, sizeof(label),

> +				"%s::", lm36274_data->pdev->name);

> +		else

> +			snprintf(label, sizeof(label),

> +				 "%s:%s", lm36274_data->pdev->name, name);

> +

> +		lm36274_data->num_leds = fwnode_property_read_u32_array(child,

> +							  "led-sources",

> +							  NULL, 0);

> +		if (lm36274_data->num_leds <= 0)

> +			return -ENODEV;

> +

> +		ret = fwnode_property_read_u32_array(child, "led-sources",

> +						     lm36274_data->led_sources,

> +						     lm36274_data->num_leds);

> +		if (ret) {

> +			dev_err(dev, "led-sources property missing\n");

> +			return -EINVAL;


Should it return ret here? If read array failed with -ENOMEM, we may
want to propagate that.

> +		}

> +

> +		fwnode_property_read_string(child, "linux,default-trigger",

> +					&lm36274_data->led_dev.default_trigger);

> +

> +	}

> +

> +	lm36274_data->lmu_data.regmap = lm36274_data->regmap;

> +	lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;

> +	lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;

> +	lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;

> +

> +	lm36274_data->led_dev.name = label;

> +	lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;

> +	lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;

> +

> +	return ret;


I'd do "return 0" here. It is success. Yes, ret will always have that
value at this moment, but...

> +static int lm36274_probe(struct platform_device *pdev)

> +{

> +	struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);

> +	struct lm36274 *lm36274_data;

> +	int ret;

> +

> +	lm36274_data = devm_kzalloc(&pdev->dev, sizeof(*lm36274_data),

> +				    GFP_KERNEL);

> +	if (!lm36274_data) {

> +		ret = -ENOMEM;

> +		return ret;

> +	}


Just do return -ENOMEM;

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
Pavel Machek June 6, 2019, 10:07 a.m. UTC | #2
On Wed 2019-06-05 07:56:34, Dan Murphy wrote:
> Introduce the LM36274 LED driver.  This driver uses the ti-lmu

> MFD driver to probe this LED driver.  The driver configures only the

> LED registers and enables the outputs according to the config file.

> 

> The driver utilizes the TI LMU (Lighting Management Unit) LED common

> framework to set the brightness bits.

> 

> Signed-off-by: Dan Murphy <dmurphy@ti.com>

> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>


Actually, one more thing. Not too important, but.. how did Jacek's
signoff get there? He should add his signoff when he applies it to the
git...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Dan Murphy June 6, 2019, 11:25 a.m. UTC | #3
Pavel

On 6/6/19 5:07 AM, Pavel Machek wrote:
> On Wed 2019-06-05 07:56:34, Dan Murphy wrote:

>> Introduce the LM36274 LED driver.  This driver uses the ti-lmu

>> MFD driver to probe this LED driver.  The driver configures only the

>> LED registers and enables the outputs according to the config file.

>>

>> The driver utilizes the TI LMU (Lighting Management Unit) LED common

>> framework to set the brightness bits.

>>

>> Signed-off-by: Dan Murphy <dmurphy@ti.com>

>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

> Actually, one more thing. Not too important, but.. how did Jacek's

> signoff get there? He should add his signoff when he applies it to the

> git...

>

This signoff came from the ti-lmu-led-drivers branch that Jacek created 
in his linux-leds repo.

Unfortunately that branch does not seem to exist now so Jacek would need 
to confirm this.

Dan
diff mbox series

Patch

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 91cb047059a0..61c585049b2d 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -798,6 +798,14 @@  config LEDS_LM3697
 	  Say Y to enable the LM3697 LED driver for TI LMU devices.
 	  This supports the LED device LM3697.
 
+config LEDS_LM36274
+	tristate "LED driver for LM36274"
+	depends on LEDS_TI_LMU_COMMON
+	depends on MFD_TI_LMU
+	help
+	  Say Y to enable the LM36274 LED driver for TI LMU devices.
+	  This supports the LED device LM36274.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 6c3350404ede..c52934732c1a 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -83,6 +83,7 @@  obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
 obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
+obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
new file mode 100644
index 000000000000..b47786d36d21
--- /dev/null
+++ b/drivers/leds/leds-lm36274.c
@@ -0,0 +1,174 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// TI LM36274 LED chip family driver
+// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/leds-ti-lmu-common.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+
+#include <uapi/linux/uleds.h>
+
+#define LM36274_MAX_STRINGS	4
+#define LM36274_BL_EN		BIT(4)
+
+/**
+ * struct lm36274
+ * @pdev: platform device
+ * @led_dev: led class device
+ * @lmu_data: Register and setting values for common code
+ * @regmap: Devices register map
+ * @dev: Pointer to the devices device struct
+ * @led_sources - The LED strings supported in this array
+ * @num_leds - Number of LED strings are supported in this array
+ */
+struct lm36274 {
+	struct platform_device *pdev;
+	struct led_classdev led_dev;
+	struct ti_lmu_bank lmu_data;
+	struct regmap *regmap;
+	struct device *dev;
+
+	u32 led_sources[LM36274_MAX_STRINGS];
+	int num_leds;
+};
+
+static int lm36274_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm36274 *led = container_of(led_cdev, struct lm36274, led_dev);
+
+	return ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+}
+
+static int lm36274_init(struct lm36274 *lm36274_data)
+{
+	int enable_val = 0;
+	int i;
+
+	for (i = 0; i < lm36274_data->num_leds; i++)
+		enable_val |= (1 << lm36274_data->led_sources[i]);
+
+	if (!enable_val) {
+		dev_err(lm36274_data->dev, "No LEDs were enabled\n");
+		return -EINVAL;
+	}
+
+	enable_val |= LM36274_BL_EN;
+
+	return regmap_write(lm36274_data->regmap, LM36274_REG_BL_EN,
+			    enable_val);
+}
+
+static int lm36274_parse_dt(struct lm36274 *lm36274_data)
+{
+	struct fwnode_handle *child = NULL;
+	char label[LED_MAX_NAME_SIZE];
+	struct device *dev = &lm36274_data->pdev->dev;
+	const char *name;
+	int child_cnt;
+	int ret = -EINVAL;
+
+	/* There should only be 1 node */
+	child_cnt = device_get_child_node_count(dev);
+	if (child_cnt != 1)
+		return ret;
+
+	device_for_each_child_node(dev, child) {
+		ret = fwnode_property_read_string(child, "label", &name);
+		if (ret)
+			snprintf(label, sizeof(label),
+				"%s::", lm36274_data->pdev->name);
+		else
+			snprintf(label, sizeof(label),
+				 "%s:%s", lm36274_data->pdev->name, name);
+
+		lm36274_data->num_leds = fwnode_property_read_u32_array(child,
+							  "led-sources",
+							  NULL, 0);
+		if (lm36274_data->num_leds <= 0)
+			return -ENODEV;
+
+		ret = fwnode_property_read_u32_array(child, "led-sources",
+						     lm36274_data->led_sources,
+						     lm36274_data->num_leds);
+		if (ret) {
+			dev_err(dev, "led-sources property missing\n");
+			return -EINVAL;
+		}
+
+		fwnode_property_read_string(child, "linux,default-trigger",
+					&lm36274_data->led_dev.default_trigger);
+
+	}
+
+	lm36274_data->lmu_data.regmap = lm36274_data->regmap;
+	lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+	lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+	lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+
+	lm36274_data->led_dev.name = label;
+	lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+	lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;
+
+	return ret;
+}
+
+static int lm36274_probe(struct platform_device *pdev)
+{
+	struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+	struct lm36274 *lm36274_data;
+	int ret;
+
+	lm36274_data = devm_kzalloc(&pdev->dev, sizeof(*lm36274_data),
+				    GFP_KERNEL);
+	if (!lm36274_data) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	lm36274_data->pdev = pdev;
+	lm36274_data->dev = lmu->dev;
+	lm36274_data->regmap = lmu->regmap;
+	dev_set_drvdata(&pdev->dev, lm36274_data);
+
+	ret = lm36274_parse_dt(lm36274_data);
+	if (ret) {
+		dev_err(lm36274_data->dev, "Failed to parse DT node\n");
+		return ret;
+	}
+
+	ret = lm36274_init(lm36274_data);
+	if (ret) {
+		dev_err(lm36274_data->dev, "Failed to init the device\n");
+		return ret;
+	}
+
+	return devm_led_classdev_register(lm36274_data->dev,
+					 &lm36274_data->led_dev);
+}
+
+static const struct of_device_id of_lm36274_leds_match[] = {
+	{ .compatible = "ti,lm36274-backlight", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lm36274_leds_match);
+
+static struct platform_driver lm36274_driver = {
+	.probe  = lm36274_probe,
+	.driver = {
+		.name = "lm36274-leds",
+	},
+};
+module_platform_driver(lm36274_driver)
+
+MODULE_DESCRIPTION("Texas Instruments LM36274 LED driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");