Skip to contentSkip to conversation

leds: Add support for Palmas LEDs

PATCH Jingoo Han 2 participants 3 messages Feb 28, 2013

lore
Share & tools
lore

Jingoo Han

author
#0 lore raw
Quoted 4 lines · Ian Lartey

The Palmas familly of chips has LED support. This is not always muxed to output pins so depending on the setting of the mux this driver will create the appropriate LED class devices.

Hi Ian Lartey,

I added some minor comments :)
Good luck.

Quoted 15 lines
Signed-off-by Graeme Gregory <[email protected]>
Signed-off-by Ian Lartey <[email protected]>
---
 drivers/leds/leds-palmas.c |  574 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/palmas.h |   60 +++++
 2 files changed, 634 insertions(+), 0 deletions(-)
 create mode 100644 drivers/leds/leds-palmas.c
drivers/leds/leds-palmas.c new file +0 -0
new file mode 100644
index 0000000..4cb73aa

drivers/leds/leds-palmas.c line 4

Quoted 4 lines
drivers/leds/leds-palmas.c new file +4 -0
@@ -0,0 +1,574 @@
+/*
+ * Driver for LED part of Palmas PMIC Chips
+ *
+ * Copyright 2011 Texas Instruments Inc.

2013 ?

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+ *
+ * Author: Graeme Gregory <[email protected]>
+ * Author: Ian Lartey <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under  the terms of the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the License, or (at your
+ *  option) any later version.
+ *
+ */

drivers/leds/leds-palmas.c line 24

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/leds.h>
+#include <linux/err.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>

Would you order header files according to alphabetical ordering? It would be better for the readability.

Quoted 229 lines
drivers/leds/leds-palmas.c new file +229 -0
+
+struct palmas_leds_data;
+
+struct palmas_led {
+	struct led_classdev cdev;
+	struct palmas_leds_data *leds_data;
+	int led_no;
+	struct work_struct work;
+	struct mutex mutex;
+
+	spinlock_t value_lock;
+
+	int blink;
+	int on_time;
+	int period;
+	enum led_brightness brightness;
+};
+
+struct palmas_leds_data {
+	struct device *dev;
+	struct led_classdev cdev;
+	struct palmas *palmas;
+
+	struct palmas_led *palmas_led;
+	int no_leds;
+};
+
+#define to_palmas_led(led_cdev) \
+	container_of(led_cdev, struct palmas_led, cdev)
+
+#define LED_ON_62_5MS		0x00
+#define LED_ON_125MS		0x01
+#define LED_ON_250MS		0x02
+#define LED_ON_500MS		0x03
+
+#define LED_PERIOD_OFF		0x00
+#define LED_PERIOD_125MS	0x01
+#define LED_PERIOD_250MS	0x02
+#define LED_PERIOD_500MS	0x03
+#define LED_PERIOD_1000MS	0x04
+#define LED_PERIOD_2000MS	0x05
+#define LED_PERIOD_4000MS	0x06
+#define LED_PERIOD_8000MS	0x07
+
+
+static char *palmas_led_names[] = {
+	"palmas:led1",
+	"palmas:led2",
+	"palmas:led3",
+	"palmas:led4",
+};
+
+static int palmas_led_read(struct palmas_leds_data *leds, unsigned int reg,
+		unsigned int *dest)
+{
+	unsigned int addr;
+
+	addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+	return palmas_read(leds->palmas, PALMAS_LED_BASE, addr, dest);
+}
+
+static int palmas_led_write(struct palmas_leds_data *leds, unsigned int reg,
+		unsigned int value)
+{
+	unsigned int addr;
+
+	addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+	return palmas_write(leds->palmas, PALMAS_LED_BASE, addr, value);
+}
+
+static int palmas_led_update_bits(struct palmas_leds_data *leds,
+		unsigned int reg, unsigned int mask, unsigned int value)
+{
+	unsigned int addr;
+
+	addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+	return palmas_update_bits(leds->palmas, PALMAS_LED_BASE,
+			addr, mask, value);
+}
+
+static void palmas_leds_work(struct work_struct *work)
+{
+	struct palmas_led *led = container_of(work, struct palmas_led, work);
+	unsigned int period, ctrl;
+	unsigned long flags;
+
+	mutex_lock(&led->mutex);
+
+	palmas_led_read(led->leds_data, PALMAS_LED_CTRL, &ctrl);
+	palmas_led_read(led->leds_data, PALMAS_LED_PERIOD_CTRL, &period);
+
+	spin_lock_irqsave(&led->value_lock, flags);
+
+	switch (led->led_no) {
+	case 1:
+		ctrl &= ~PALMAS_LED_CTRL_LED_1_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL_LED_1_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL_LED_1_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_SHIFT;
+			}
+		}
+	case 2:
+		ctrl &= ~PALMAS_LED_CTRL_LED_2_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL_LED_2_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL_LED_2_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_SHIFT;
+			}
+		}
+	case 3:
+		ctrl &= ~PALMAS_LED_CTRL2_LED_3_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT;
+			}
+		}
+		break;
+	case 4:
+		ctrl &= ~PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT;
+			}
+		}
+		break;
+	}
+	spin_unlock_irqrestore(&led->value_lock, flags);
+
+	if (led->led_no < 3) {
+		palmas_led_write(led->leds_data, PALMAS_LED_CTRL, ctrl);
+		palmas_led_write(led->leds_data, PALMAS_LED_PERIOD_CTRL,
+				period);
+	} else {
+		palmas_led_write(led->leds_data, PALMAS_LED_CTRL2, ctrl);
+		palmas_led_write(led->leds_data, PALMAS_LED_PERIOD2_CTRL,
+				period);
+	}
+
+	if (is_palmas_charger(led->leds_data->palmas->product_id)) {
+		if (led->brightness || led->blink)
+			palmas_led_update_bits(led->leds_data, PALMAS_LED_EN,
+					1 << (led->led_no - 1), 0xFF);
+		else
+			palmas_led_update_bits(led->leds_data, PALMAS_LED_EN,
+					1 << (led->led_no - 1), 0x00);
+	}
+	mutex_unlock(&led->mutex);
+}
+
+static void palmas_leds_set(struct led_classdev *led_cdev,
+			   enum led_brightness value)
+{
+	struct palmas_led *led = to_palmas_led(led_cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&led->value_lock, flags);
+	led->brightness = value;
+	led->blink = 0;
+	schedule_work(&led->work);
+	spin_unlock_irqrestore(&led->value_lock, flags);
+}
+
+static int palmas_leds_blink_set(struct led_classdev *led_cdev,
+				   unsigned long *delay_on,
+				   unsigned long *delay_off)
+{
+	struct palmas_led *led = to_palmas_led(led_cdev);
+	unsigned long flags;
+	int ret = 0;
+	int period;
+

drivers/leds/leds-palmas.c line 263

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+	/* Pick some defaults if we've not been given times */
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = 250;
+		*delay_off = 250;
+	}
+
+	spin_lock_irqsave(&led->value_lock, flags);
+
+	/* We only have a limited selection of settings, see if we can
+	 * support the configuration we're being given */

According to the Coding Style, the preferred style for multi-line comments is:

+ /*
+ * We only have a limited selection of settings, see if we can
+ * support the configuration we're being given
+ */

Quoted 51 lines
drivers/leds/leds-palmas.c new file +51 -0
+	switch (*delay_on) {
+	case 500:
+		led->on_time = LED_ON_500MS;
+		break;
+	case 250:
+		led->on_time = LED_ON_250MS;
+		break;
+	case 125:
+		led->on_time = LED_ON_125MS;
+		break;
+	case 62:
+	case 63:
+		/* Actually 62.5ms */
+		led->on_time = LED_ON_62_5MS;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	period = *delay_on + *delay_off;
+
+	if (ret == 0) {
+		switch (period) {
+		case 124:
+		case 125:
+		case 126:
+			/* All possible variations of 62.5 + 62.5 */
+			led->period = LED_PERIOD_125MS;
+			break;
+		case 250:
+			led->period = LED_PERIOD_250MS;
+			break;
+		case 500:
+			led->period = LED_PERIOD_500MS;
+			break;
+		case 1000:
+			led->period = LED_PERIOD_1000MS;
+			break;
+		case 2000:
+			led->period = LED_PERIOD_2000MS;
+			break;
+		case 4000:
+			led->period = LED_PERIOD_4000MS;
+			break;
+		case 8000:
+			led->period = LED_PERIOD_8000MS;
+			break;
+		default:
+			ret = -EINVAL;
+			break;

drivers/leds/leds-palmas.c line 324

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+		}
+	}
+
+	if (ret == 0)
+		led->blink = 1;
+	else
+		led->blink = 0;
+
+	/* Always update; if we fail turn off blinking since we expect
+	 * a software fallback. */

Same as above.

Quoted 83 lines
drivers/leds/leds-palmas.c new file +83 -0
+	schedule_work(&led->work);
+
+	spin_unlock_irqrestore(&led->value_lock, flags);
+
+	return ret;
+}
+
+static void palmas_init_led(struct palmas_leds_data *leds_data, int offset,
+		int led_no)
+{
+	mutex_init(&leds_data->palmas_led[offset].mutex);
+	INIT_WORK(&leds_data->palmas_led[offset].work, palmas_leds_work);
+	spin_lock_init(&leds_data->palmas_led[offset].value_lock);
+
+	leds_data->palmas_led[offset].leds_data = leds_data;
+	leds_data->palmas_led[offset].led_no = led_no;
+	leds_data->palmas_led[offset].cdev.name = palmas_led_names[led_no - 1];
+	leds_data->palmas_led[offset].cdev.default_trigger = NULL;
+	leds_data->palmas_led[offset].cdev.brightness_set = palmas_leds_set;
+	leds_data->palmas_led[offset].cdev.blink_set = palmas_leds_blink_set;
+}
+
+static int  palmas_dt_to_pdata(struct device *dev,
+		struct device_node *node,
+		struct palmas_leds_platform_data *pdata)
+{
+	struct device_node *child_node;
+	int ret;
+	u32 prop;
+
+	child_node = of_get_child_by_name(node, "palmas_leds");
+	if (!child_node) {
+		dev_err(dev, "child node 'palmas_leds' not found\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(child_node, "ti,led1_current", &prop);
+	if (!ret)
+		pdata->led1_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,led2_current", &prop);
+	if (!ret)
+		pdata->led2_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,led3_current", &prop);
+	if (!ret)
+		pdata->led3_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,led4_current", &prop);
+	if (!ret)
+		pdata->led4_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,chrg_led_mode", &prop);
+	if (!ret)
+		pdata->chrg_led_mode = prop;
+
+	ret = of_property_read_u32(child_node, "ti,chrg_led_vbat_low", &prop);
+	if (!ret)
+		pdata->chrg_led_vbat_low = prop;
+
+	return 0;
+}
+
+static int palmas_leds_probe(struct platform_device *pdev)
+{
+	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+	struct palmas_leds_platform_data *pdata = pdev->dev.platform_data;
+	struct palmas_leds_data *leds_data;
+	struct device_node *node = pdev->dev.of_node;
+	int ret, i;
+	int offset = 0;
+
+	if (!palmas->led_muxed && !is_palmas_charger(palmas->product_id)) {
+		dev_err(&pdev->dev, "there are no LEDs muxed\n");
+		return -EINVAL;
+	}
+
+	/* Palmas charger requires platform data */
+	if (is_palmas_charger(palmas->product_id) && node && !pdata) {
+		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+
+		if (!pdata)
+			return -ENOMEM;

drivers/leds/leds-palmas.c line 417

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+		ret = palmas_dt_to_pdata(&pdev->dev, node, pdata);
+		if (ret)
+			return ret;
+	}
+
+	if (is_palmas_charger(palmas->product_id) && !pdata)
+		return -EINVAL;
+
+	leds_data = kzalloc(sizeof(*leds_data), GFP_KERNEL);

How about using devm_kzalloc()?
Memory allocated with this function is automatically freed on driver detach. So it makes error path more simple.

Quoted 18 lines
drivers/leds/leds-palmas.c new file +18 -0
+	if (!leds_data)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, leds_data);
+
+	leds_data->palmas = palmas;
+
+	switch (palmas->led_muxed) {
+	case PALMAS_LED1_MUXED | PALMAS_LED2_MUXED:
+		leds_data->no_leds = 2;
+		break;
+	case PALMAS_LED1_MUXED:
+	case PALMAS_LED2_MUXED:
+		leds_data->no_leds = 1;
+		break;
+	default:
+		leds_data->no_leds = 0;
+		break;
+	}

drivers/leds/leds-palmas.c line 445

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+	if (is_palmas_charger(palmas->product_id)) {
+		if (pdata->chrg_led_mode)
+			leds_data->no_leds += 2;
+		else
+			leds_data->no_leds++;
+	}
+
+	leds_data->palmas_led = kcalloc(leds_data->no_leds,
+			sizeof(struct palmas_led), GFP_KERNEL);

How about using devm_kzalloc()?

Quoted 70 lines
drivers/leds/leds-palmas.c new file +70 -0
+
+	/* Initialise LED1 */
+	if (palmas->led_muxed & PALMAS_LED1_MUXED) {
+		palmas_init_led(leds_data, offset, 1);
+		offset++;
+	}
+
+	/* Initialise LED2 */
+	if (palmas->led_muxed & PALMAS_LED2_MUXED) {
+		palmas_init_led(leds_data, offset, 2);
+		offset++;
+	}
+
+	if (is_palmas_charger(palmas->product_id)) {
+		palmas_init_led(leds_data, offset, 3);
+		offset++;
+
+		if (pdata->chrg_led_mode) {
+			palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,
+					PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE,
+					PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE);
+
+			palmas_init_led(leds_data, offset, 4);
+		}
+	}
+
+	for (i = 0; i < leds_data->no_leds; i++) {
+		ret = led_classdev_register(leds_data->dev,
+				&leds_data->palmas_led[i].cdev);
+		if (ret < 0) {
+			dev_err(&pdev->dev,
+				"Failed to register LED no: %d err: %d\n",
+				i, ret);
+			goto err_led;
+		}
+	}
+
+	if (!is_palmas_charger(palmas->product_id))
+		return 0;
+
+	/* Set the LED current registers if set in platform data */
+	if (pdata->led1_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL1,
+				PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_MASK,
+				pdata->led1_current);
+
+	if (pdata->led2_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL1,
+				PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_MASK,
+				pdata->led2_current <<
+				PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_SHIFT);
+
+	if (pdata->led3_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL2,
+				PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK,
+				pdata->led3_current);
+
+	if (pdata->led3_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL2,
+				PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK,
+				pdata->led3_current);
+
+	if (pdata->led4_current)
+		palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,
+				PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_MASK,
+				pdata->led4_current <<
+				PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_SHIFT);
+
+	if (pdata->chrg_led_vbat_low)
+		palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,

drivers/leds/leds-palmas.c line 525

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+				PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS,
+				PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS);
+
+	return 0;
+
+err_led:
+	for (i = 0; i < leds_data->no_leds; i++)
+		led_classdev_unregister(&leds_data->palmas_led[i].cdev);
+	kfree(leds_data->palmas_led);
+	kfree(leds_data);

If you use devm_kzalloc() above, you don't need to use kfree().

drivers/leds/leds-palmas.c line 538

Quoted 13 lines
drivers/leds/leds-palmas.c new file +13 -0
+	return ret;
+}
+
+static int palmas_leds_remove(struct platform_device *pdev)
+{
+	struct palmas_leds_data *leds_data = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < leds_data->no_leds; i++)
+		led_classdev_unregister(&leds_data->palmas_led[i].cdev);
+	if (i)
+		kfree(leds_data->palmas_led);
+	kfree(leds_data);

If you use devm_kzalloc() above, you don't need to use kfree().

Quoted 20 lines
drivers/leds/leds-palmas.c new file +20 -0
+
+	return 0;
+}
+
+static struct of_device_id of_palmas_match_tbl[] = {
+	{ .compatible = "ti,palmas-leds", },
+	{ /* end */ }
+};
+
+static struct platform_driver palmas_leds_driver = {
+	.driver = {
+		.name = "palmas-leds",
+		.of_match_table = of_palmas_match_tbl,
+		.owner = THIS_MODULE,
+	},
+	.probe = palmas_leds_probe,
+	.remove = palmas_leds_remove,
+};
+
+static int  palmas_leds_init(void)

drivers/leds/leds-palmas.c line 568

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+{
+	return platform_driver_register(&palmas_leds_driver);
+}
+module_init(palmas_leds_init);
+
+static void palmas_leds_exit(void)
+{
+	platform_driver_unregister(&palmas_leds_driver);
+}
+module_exit(palmas_leds_exit);

Please use module_platform_driver() macro which makes the code smaller and simpler.

+module_platform_driver(palmas_leds_driver);

Quoted 98 lines
drivers/leds/leds-palmas.c new file +6 -0
+
+MODULE_AUTHOR("Graeme Gregory <[email protected]>");
+MODULE_DESCRIPTION("Palmas LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:palmas-leds");
+MODULE_DEVICE_TABLE(of, of_palmas_match_tbl);
include/linux/mfd/palmas.h +60 -1
index 0b86845..856f54e 100644
@@ -232,6 +232,16 @@ struct palmas_clk_platform_data {
 	int clk32kgaudio_mode_sleep;
 };
 
+struct palmas_leds_platform_data {
+	int led1_current;
+	int led2_current;
+	int led3_current;
+	int led4_current;
+
+	int chrg_led_mode;
+	int chrg_led_vbat_low;
+};
+
 struct palmas_platform_data {
 	int gpio_base;
 

@@ -1855,6 +1865,12 @@ enum usb_irq_events {
 #define PALMAS_LED_CTRL						0x1
 #define PALMAS_PWM_CTRL1					0x2
 #define PALMAS_PWM_CTRL2					0x3
+#define PALMAS_LED_PERIOD2_CTRL					0x4
+#define PALMAS_LED_CTRL2					0x5
+#define PALMAS_LED_CURRENT_CTRL1				0x6
+#define PALMAS_LED_CURRENT_CTRL2				0x7
+#define PALMAS_CHRG_LED_CTRL					0x8
+#define PALMAS_LED_EN						0x9
 
 /* Bit definitions for LED_PERIOD_CTRL */
 #define PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_MASK		0x38

@@ -1882,6 +1898,50 @@ enum usb_irq_events {
 #define PALMAS_PWM_CTRL2_PWM_DUTY_SEL_MASK			0xff
 #define PALMAS_PWM_CTRL2_PWM_DUTY_SEL_SHIFT			0
 
+/* Bit definitions for LED_PERIOD2_CTRL */
+#define PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_MASK		0x38
+#define PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT		3
+#define PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_MASK		0x07
+#define PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT		0
+
+/* Bit definitions for LED_CTRL2 */
+#define PALMAS_LED_CTRL2_CHRG_LED_SEQ				0x20
+#define PALMAS_LED_CTRL2_CHRG_LED_SEQ_SHIFT			5
+#define PALMAS_LED_CTRL2_LED_3_SEQ				0x10
+#define PALMAS_LED_CTRL2_LED_3_SEQ_SHIFT			4
+#define PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_MASK			0x0c
+#define PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT			2
+#define PALMAS_LED_CTRL2_LED_3_ON_TIME_MASK			0x03
+#define PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT			0
+
+/* Bit definitions for LED_CURRENT_CTRL1 */
+#define PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_MASK		0x38
+#define PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_SHIFT		3
+#define PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_MASK		0x07
+#define PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_SHIFT		0
+
+/* Bit definitions for LED_CURRENT_CTRL2 */
+#define PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK		0x07
+#define PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_SHIFT		0
+
+/* Bit definitions for CHRG_LED_CTRL */
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_MASK		0x38
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_SHIFT		3
+#define PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS		0x02
+#define PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS_SHIFT		1
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE			0x01
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE_SHIFT		0
+
+/* Bit definitions for LED_EN */
+#define PALMAS_LED_EN_CHRG_LED_EN				0x08
+#define PALMAS_LED_EN_CHRG_LED_EN_SHIFT				3
+#define PALMAS_LED_EN_LED_3_EN					0x04
+#define PALMAS_LED_EN_LED_3_EN_SHIFT				2
+#define PALMAS_LED_EN_LED_2_EN					0x02
+#define PALMAS_LED_EN_LED_2_EN_SHIFT				1
+#define PALMAS_LED_EN_LED_1_EN					0x01
+#define PALMAS_LED_EN_LED_1_EN_SHIFT				0
+
 /* Registers for function INTERRUPT */
 #define PALMAS_INT1_STATUS					0x0
 #define PALMAS_INT1_MASK					0x1
--

1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw
Quoted 9 lines · Jingoo Han
Quoted 4 lines · Ian Lartey

The Palmas familly of chips has LED support. This is not always muxed to output pins so depending on the setting of the mux this driver will create the appropriate LED class devices.

Hi Ian Lartey,

I added some minor comments :)

Hello Jingoo Han,

Thanks for your comments.

Quoted 11 lines

Good luck.

Quoted 9 lines
Signed-off-by Graeme Gregory <[email protected]>
Signed-off-by Ian Lartey <[email protected]>
---
  drivers/leds/leds-palmas.c |  574 ++++++++++++++++++++++++++++++++++++++++++++
  include/linux/mfd/palmas.h |   60 +++++
  2 files changed, 634 insertions(+), 0 deletions(-)

create mode 100644 drivers/leds/leds-palmas.c

Quoted 12 lines

drivers/leds/leds-palmas.c line 4

Quoted 10 lines
drivers/leds/leds-palmas.c new file +4 -0
new file mode 100644
index 0000000..4cb73aa
@@ -0,0 +1,574 @@
+/*
+ * Driver for LED part of Palmas PMIC Chips
+ *
+ * Copyright 2011 Texas Instruments Inc.

2013 ?

Code _was_ started a while ago - I'll update.

Quoted 16 lines
Quoted 16 lines
drivers/leds/leds-palmas.c new file +15 -0
+ *
+ * Author: Graeme Gregory <[email protected]>
+ * Author: Ian Lartey <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under  the terms of the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
Quoted 8 lines

drivers/leds/leds-palmas.c line 24

Quoted 5 lines
drivers/leds/leds-palmas.c new file +5 -0
+#include <linux/leds.h>
+#include <linux/err.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>

Would you order header files according to alphabetical ordering? It would be better for the readability.

It could cause dependency issues so I'd rather leave them in the order they are in.

I'll implement all the other changes you've noted.

Thanks

Ian

Quoted 680 lines
Quoted 230 lines
drivers/leds/leds-palmas.c new file +229 -0
+
+struct palmas_leds_data;
+
+struct palmas_led {
+	struct led_classdev cdev;
+	struct palmas_leds_data *leds_data;
+	int led_no;
+	struct work_struct work;
+	struct mutex mutex;
+
+	spinlock_t value_lock;
+
+	int blink;
+	int on_time;
+	int period;
+	enum led_brightness brightness;
+};
+
+struct palmas_leds_data {
+	struct device *dev;
+	struct led_classdev cdev;
+	struct palmas *palmas;
+
+	struct palmas_led *palmas_led;
+	int no_leds;
+};
+
+#define to_palmas_led(led_cdev) \
+	container_of(led_cdev, struct palmas_led, cdev)
+
+#define LED_ON_62_5MS		0x00
+#define LED_ON_125MS		0x01
+#define LED_ON_250MS		0x02
+#define LED_ON_500MS		0x03
+
+#define LED_PERIOD_OFF		0x00
+#define LED_PERIOD_125MS	0x01
+#define LED_PERIOD_250MS	0x02
+#define LED_PERIOD_500MS	0x03
+#define LED_PERIOD_1000MS	0x04
+#define LED_PERIOD_2000MS	0x05
+#define LED_PERIOD_4000MS	0x06
+#define LED_PERIOD_8000MS	0x07
+
+
+static char *palmas_led_names[] = {
+	"palmas:led1",
+	"palmas:led2",
+	"palmas:led3",
+	"palmas:led4",
+};
+
+static int palmas_led_read(struct palmas_leds_data *leds, unsigned int reg,
+		unsigned int *dest)
+{
+	unsigned int addr;
+
+	addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+	return palmas_read(leds->palmas, PALMAS_LED_BASE, addr, dest);
+}
+
+static int palmas_led_write(struct palmas_leds_data *leds, unsigned int reg,
+		unsigned int value)
+{
+	unsigned int addr;
+
+	addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+	return palmas_write(leds->palmas, PALMAS_LED_BASE, addr, value);
+}
+
+static int palmas_led_update_bits(struct palmas_leds_data *leds,
+		unsigned int reg, unsigned int mask, unsigned int value)
+{
+	unsigned int addr;
+
+	addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+	return palmas_update_bits(leds->palmas, PALMAS_LED_BASE,
+			addr, mask, value);
+}
+
+static void palmas_leds_work(struct work_struct *work)
+{
+	struct palmas_led *led = container_of(work, struct palmas_led, work);
+	unsigned int period, ctrl;
+	unsigned long flags;
+
+	mutex_lock(&led->mutex);
+
+	palmas_led_read(led->leds_data, PALMAS_LED_CTRL, &ctrl);
+	palmas_led_read(led->leds_data, PALMAS_LED_PERIOD_CTRL, &period);
+
+	spin_lock_irqsave(&led->value_lock, flags);
+
+	switch (led->led_no) {
+	case 1:
+		ctrl &= ~PALMAS_LED_CTRL_LED_1_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL_LED_1_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL_LED_1_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_SHIFT;
+			}
+		}
+	case 2:
+		ctrl &= ~PALMAS_LED_CTRL_LED_2_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL_LED_2_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL_LED_2_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_SHIFT;
+			}
+		}
+	case 3:
+		ctrl &= ~PALMAS_LED_CTRL2_LED_3_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT;
+			}
+		}
+		break;
+	case 4:
+		ctrl &= ~PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_MASK;
+		period &= ~PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_MASK;
+
+		if (led->blink) {
+			ctrl |= led->on_time <<
+				PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT;
+			period |= led->period <<
+				PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT;
+		} else {
+			/*
+			 * for off value is zero which we already set by
+			 * masking earlier.
+			 */
+			if (led->brightness) {
+				ctrl |= LED_ON_500MS <<
+					PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT;
+				period |= LED_PERIOD_500MS <<
+				PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT;
+			}
+		}
+		break;
+	}
+	spin_unlock_irqrestore(&led->value_lock, flags);
+
+	if (led->led_no < 3) {
+		palmas_led_write(led->leds_data, PALMAS_LED_CTRL, ctrl);
+		palmas_led_write(led->leds_data, PALMAS_LED_PERIOD_CTRL,
+				period);
+	} else {
+		palmas_led_write(led->leds_data, PALMAS_LED_CTRL2, ctrl);
+		palmas_led_write(led->leds_data, PALMAS_LED_PERIOD2_CTRL,
+				period);
+	}
+
+	if (is_palmas_charger(led->leds_data->palmas->product_id)) {
+		if (led->brightness || led->blink)
+			palmas_led_update_bits(led->leds_data, PALMAS_LED_EN,
+					1 << (led->led_no - 1), 0xFF);
+		else
+			palmas_led_update_bits(led->leds_data, PALMAS_LED_EN,
+					1 << (led->led_no - 1), 0x00);
+	}
+	mutex_unlock(&led->mutex);
+}
+
+static void palmas_leds_set(struct led_classdev *led_cdev,
+			   enum led_brightness value)
+{
+	struct palmas_led *led = to_palmas_led(led_cdev);
+	unsigned long flags;
+
+	spin_lock_irqsave(&led->value_lock, flags);
+	led->brightness = value;
+	led->blink = 0;
+	schedule_work(&led->work);
+	spin_unlock_irqrestore(&led->value_lock, flags);
+}
+
+static int palmas_leds_blink_set(struct led_classdev *led_cdev,
+				   unsigned long *delay_on,
+				   unsigned long *delay_off)
+{
+	struct palmas_led *led = to_palmas_led(led_cdev);
+	unsigned long flags;
+	int ret = 0;
+	int period;
+

drivers/leds/leds-palmas.c line 263

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+	/* Pick some defaults if we've not been given times */
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = 250;
+		*delay_off = 250;
+	}
+
+	spin_lock_irqsave(&led->value_lock, flags);
+
+	/* We only have a limited selection of settings, see if we can
+	 * support the configuration we're being given */

According to the Coding Style, the preferred style for multi-line comments is:

+ /*
+ * We only have a limited selection of settings, see if we can
+ * support the configuration we're being given
+ */

Quoted 51 lines
drivers/leds/leds-palmas.c new file +51 -0
+	switch (*delay_on) {
+	case 500:
+		led->on_time = LED_ON_500MS;
+		break;
+	case 250:
+		led->on_time = LED_ON_250MS;
+		break;
+	case 125:
+		led->on_time = LED_ON_125MS;
+		break;
+	case 62:
+	case 63:
+		/* Actually 62.5ms */
+		led->on_time = LED_ON_62_5MS;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	period = *delay_on + *delay_off;
+
+	if (ret == 0) {
+		switch (period) {
+		case 124:
+		case 125:
+		case 126:
+			/* All possible variations of 62.5 + 62.5 */
+			led->period = LED_PERIOD_125MS;
+			break;
+		case 250:
+			led->period = LED_PERIOD_250MS;
+			break;
+		case 500:
+			led->period = LED_PERIOD_500MS;
+			break;
+		case 1000:
+			led->period = LED_PERIOD_1000MS;
+			break;
+		case 2000:
+			led->period = LED_PERIOD_2000MS;
+			break;
+		case 4000:
+			led->period = LED_PERIOD_4000MS;
+			break;
+		case 8000:
+			led->period = LED_PERIOD_8000MS;
+			break;
+		default:
+			ret = -EINVAL;
+			break;

drivers/leds/leds-palmas.c line 324

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+		}
+	}
+
+	if (ret == 0)
+		led->blink = 1;
+	else
+		led->blink = 0;
+
+	/* Always update; if we fail turn off blinking since we expect
+	 * a software fallback. */

Same as above.

Quoted 83 lines
drivers/leds/leds-palmas.c new file +83 -0
+	schedule_work(&led->work);
+
+	spin_unlock_irqrestore(&led->value_lock, flags);
+
+	return ret;
+}
+
+static void palmas_init_led(struct palmas_leds_data *leds_data, int offset,
+		int led_no)
+{
+	mutex_init(&leds_data->palmas_led[offset].mutex);
+	INIT_WORK(&leds_data->palmas_led[offset].work, palmas_leds_work);
+	spin_lock_init(&leds_data->palmas_led[offset].value_lock);
+
+	leds_data->palmas_led[offset].leds_data = leds_data;
+	leds_data->palmas_led[offset].led_no = led_no;
+	leds_data->palmas_led[offset].cdev.name = palmas_led_names[led_no - 1];
+	leds_data->palmas_led[offset].cdev.default_trigger = NULL;
+	leds_data->palmas_led[offset].cdev.brightness_set = palmas_leds_set;
+	leds_data->palmas_led[offset].cdev.blink_set = palmas_leds_blink_set;
+}
+
+static int  palmas_dt_to_pdata(struct device *dev,
+		struct device_node *node,
+		struct palmas_leds_platform_data *pdata)
+{
+	struct device_node *child_node;
+	int ret;
+	u32 prop;
+
+	child_node = of_get_child_by_name(node, "palmas_leds");
+	if (!child_node) {
+		dev_err(dev, "child node 'palmas_leds' not found\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32(child_node, "ti,led1_current", &prop);
+	if (!ret)
+		pdata->led1_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,led2_current", &prop);
+	if (!ret)
+		pdata->led2_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,led3_current", &prop);
+	if (!ret)
+		pdata->led3_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,led4_current", &prop);
+	if (!ret)
+		pdata->led4_current = prop;
+
+	ret = of_property_read_u32(child_node, "ti,chrg_led_mode", &prop);
+	if (!ret)
+		pdata->chrg_led_mode = prop;
+
+	ret = of_property_read_u32(child_node, "ti,chrg_led_vbat_low", &prop);
+	if (!ret)
+		pdata->chrg_led_vbat_low = prop;
+
+	return 0;
+}
+
+static int palmas_leds_probe(struct platform_device *pdev)
+{
+	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+	struct palmas_leds_platform_data *pdata = pdev->dev.platform_data;
+	struct palmas_leds_data *leds_data;
+	struct device_node *node = pdev->dev.of_node;
+	int ret, i;
+	int offset = 0;
+
+	if (!palmas->led_muxed && !is_palmas_charger(palmas->product_id)) {
+		dev_err(&pdev->dev, "there are no LEDs muxed\n");
+		return -EINVAL;
+	}
+
+	/* Palmas charger requires platform data */
+	if (is_palmas_charger(palmas->product_id) && node && !pdata) {
+		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+
+		if (!pdata)
+			return -ENOMEM;

drivers/leds/leds-palmas.c line 417

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+		ret = palmas_dt_to_pdata(&pdev->dev, node, pdata);
+		if (ret)
+			return ret;
+	}
+
+	if (is_palmas_charger(palmas->product_id) && !pdata)
+		return -EINVAL;
+
+	leds_data = kzalloc(sizeof(*leds_data), GFP_KERNEL);

How about using devm_kzalloc()?
Memory allocated with this function is automatically freed on driver detach. So it makes error path more simple.

Quoted 18 lines
drivers/leds/leds-palmas.c new file +18 -0
+	if (!leds_data)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, leds_data);
+
+	leds_data->palmas = palmas;
+
+	switch (palmas->led_muxed) {
+	case PALMAS_LED1_MUXED | PALMAS_LED2_MUXED:
+		leds_data->no_leds = 2;
+		break;
+	case PALMAS_LED1_MUXED:
+	case PALMAS_LED2_MUXED:
+		leds_data->no_leds = 1;
+		break;
+	default:
+		leds_data->no_leds = 0;
+		break;
+	}

drivers/leds/leds-palmas.c line 445

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+	if (is_palmas_charger(palmas->product_id)) {
+		if (pdata->chrg_led_mode)
+			leds_data->no_leds += 2;
+		else
+			leds_data->no_leds++;
+	}
+
+	leds_data->palmas_led = kcalloc(leds_data->no_leds,
+			sizeof(struct palmas_led), GFP_KERNEL);

How about using devm_kzalloc()?

Quoted 70 lines
drivers/leds/leds-palmas.c new file +70 -0
+
+	/* Initialise LED1 */
+	if (palmas->led_muxed & PALMAS_LED1_MUXED) {
+		palmas_init_led(leds_data, offset, 1);
+		offset++;
+	}
+
+	/* Initialise LED2 */
+	if (palmas->led_muxed & PALMAS_LED2_MUXED) {
+		palmas_init_led(leds_data, offset, 2);
+		offset++;
+	}
+
+	if (is_palmas_charger(palmas->product_id)) {
+		palmas_init_led(leds_data, offset, 3);
+		offset++;
+
+		if (pdata->chrg_led_mode) {
+			palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,
+					PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE,
+					PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE);
+
+			palmas_init_led(leds_data, offset, 4);
+		}
+	}
+
+	for (i = 0; i < leds_data->no_leds; i++) {
+		ret = led_classdev_register(leds_data->dev,
+				&leds_data->palmas_led[i].cdev);
+		if (ret < 0) {
+			dev_err(&pdev->dev,
+				"Failed to register LED no: %d err: %d\n",
+				i, ret);
+			goto err_led;
+		}
+	}
+
+	if (!is_palmas_charger(palmas->product_id))
+		return 0;
+
+	/* Set the LED current registers if set in platform data */
+	if (pdata->led1_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL1,
+				PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_MASK,
+				pdata->led1_current);
+
+	if (pdata->led2_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL1,
+				PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_MASK,
+				pdata->led2_current <<
+				PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_SHIFT);
+
+	if (pdata->led3_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL2,
+				PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK,
+				pdata->led3_current);
+
+	if (pdata->led3_current)
+		palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL2,
+				PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK,
+				pdata->led3_current);
+
+	if (pdata->led4_current)
+		palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,
+				PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_MASK,
+				pdata->led4_current <<
+				PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_SHIFT);
+
+	if (pdata->chrg_led_vbat_low)
+		palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,

drivers/leds/leds-palmas.c line 525

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+				PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS,
+				PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS);
+
+	return 0;
+
+err_led:
+	for (i = 0; i < leds_data->no_leds; i++)
+		led_classdev_unregister(&leds_data->palmas_led[i].cdev);
+	kfree(leds_data->palmas_led);
+	kfree(leds_data);

If you use devm_kzalloc() above, you don't need to use kfree().

drivers/leds/leds-palmas.c line 538

Quoted 13 lines
drivers/leds/leds-palmas.c new file +13 -0
+	return ret;
+}
+
+static int palmas_leds_remove(struct platform_device *pdev)
+{
+	struct palmas_leds_data *leds_data = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < leds_data->no_leds; i++)
+		led_classdev_unregister(&leds_data->palmas_led[i].cdev);
+	if (i)
+		kfree(leds_data->palmas_led);
+	kfree(leds_data);

If you use devm_kzalloc() above, you don't need to use kfree().

Quoted 20 lines
drivers/leds/leds-palmas.c new file +20 -0
+
+	return 0;
+}
+
+static struct of_device_id of_palmas_match_tbl[] = {
+	{ .compatible = "ti,palmas-leds", },
+	{ /* end */ }
+};
+
+static struct platform_driver palmas_leds_driver = {
+	.driver = {
+		.name = "palmas-leds",
+		.of_match_table = of_palmas_match_tbl,
+		.owner = THIS_MODULE,
+	},
+	.probe = palmas_leds_probe,
+	.remove = palmas_leds_remove,
+};
+
+static int  palmas_leds_init(void)

drivers/leds/leds-palmas.c line 568

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+{
+	return platform_driver_register(&palmas_leds_driver);
+}
+module_init(palmas_leds_init);
+
+static void palmas_leds_exit(void)
+{
+	platform_driver_unregister(&palmas_leds_driver);
+}
+module_exit(palmas_leds_exit);

Please use module_platform_driver() macro which makes the code smaller and simpler.

+module_platform_driver(palmas_leds_driver);

Quoted 94 lines
drivers/leds/leds-palmas.c new file +6 -0
+
+MODULE_AUTHOR("Graeme Gregory <[email protected]>");
+MODULE_DESCRIPTION("Palmas LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:palmas-leds");
+MODULE_DEVICE_TABLE(of, of_palmas_match_tbl);
include/linux/mfd/palmas.h +60 -1
index 0b86845..856f54e 100644
@@ -232,6 +232,16 @@ struct palmas_clk_platform_data {
  	int clk32kgaudio_mode_sleep;
  };
 
+struct palmas_leds_platform_data {
+	int led1_current;
+	int led2_current;
+	int led3_current;
+	int led4_current;
+
+	int chrg_led_mode;
+	int chrg_led_vbat_low;
+};
+
  struct palmas_platform_data {
  	int gpio_base;
 

@@ -1855,6 +1865,12 @@ enum usb_irq_events {
  #define PALMAS_LED_CTRL						0x1
  #define PALMAS_PWM_CTRL1					0x2
  #define PALMAS_PWM_CTRL2					0x3
+#define PALMAS_LED_PERIOD2_CTRL					0x4
+#define PALMAS_LED_CTRL2					0x5
+#define PALMAS_LED_CURRENT_CTRL1				0x6
+#define PALMAS_LED_CURRENT_CTRL2				0x7
+#define PALMAS_CHRG_LED_CTRL					0x8
+#define PALMAS_LED_EN						0x9
 
  /* Bit definitions for LED_PERIOD_CTRL */
  #define PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_MASK		0x38

@@ -1882,6 +1898,50 @@ enum usb_irq_events {
  #define PALMAS_PWM_CTRL2_PWM_DUTY_SEL_MASK			0xff
  #define PALMAS_PWM_CTRL2_PWM_DUTY_SEL_SHIFT			0
 
+/* Bit definitions for LED_PERIOD2_CTRL */
+#define PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_MASK		0x38
+#define PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT		3
+#define PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_MASK		0x07
+#define PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT		0
+
+/* Bit definitions for LED_CTRL2 */
+#define PALMAS_LED_CTRL2_CHRG_LED_SEQ				0x20
+#define PALMAS_LED_CTRL2_CHRG_LED_SEQ_SHIFT			5
+#define PALMAS_LED_CTRL2_LED_3_SEQ				0x10
+#define PALMAS_LED_CTRL2_LED_3_SEQ_SHIFT			4
+#define PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_MASK			0x0c
+#define PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT			2
+#define PALMAS_LED_CTRL2_LED_3_ON_TIME_MASK			0x03
+#define PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT			0
+
+/* Bit definitions for LED_CURRENT_CTRL1 */
+#define PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_MASK		0x38
+#define PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_SHIFT		3
+#define PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_MASK		0x07
+#define PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_SHIFT		0
+
+/* Bit definitions for LED_CURRENT_CTRL2 */
+#define PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK		0x07
+#define PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_SHIFT		0
+
+/* Bit definitions for CHRG_LED_CTRL */
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_MASK		0x38
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_SHIFT		3
+#define PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS		0x02
+#define PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS_SHIFT		1
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE			0x01
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE_SHIFT		0
+
+/* Bit definitions for LED_EN */
+#define PALMAS_LED_EN_CHRG_LED_EN				0x08
+#define PALMAS_LED_EN_CHRG_LED_EN_SHIFT				3
+#define PALMAS_LED_EN_LED_3_EN					0x04
+#define PALMAS_LED_EN_LED_3_EN_SHIFT				2
+#define PALMAS_LED_EN_LED_2_EN					0x02
+#define PALMAS_LED_EN_LED_2_EN_SHIFT				1
+#define PALMAS_LED_EN_LED_1_EN					0x01
+#define PALMAS_LED_EN_LED_1_EN_SHIFT				0
+
  /* Registers for function INTERRUPT */
  #define PALMAS_INT1_STATUS					0x0
  #define PALMAS_INT1_MASK					0x1
--

1.7.0.4

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴쪐{콗喩zX㎍썳變}찠꼿쟺�&j:+v돣�쳭喩zZ+�+zf＂톒쉱�~넮녬i�鎬z�췿ⅱ�?솳鈺�&�)刪f뷌^j푹y쬶끷@A첺뛴�� 0띠h��i�

#2 lore raw
Quoted 7 lines · Ian Lartey
Quoted 6 lines · Jingoo Han
Quoted 4 lines · Ian Lartey

The Palmas familly of chips has LED support. This is not always muxed to output pins so depending on the setting of the mux this driver will create the appropriate LED class devices.

Quoted 10 lines
Quoted 3 lines

Hi Ian Lartey,

I added some minor comments :)

Hello Jingoo Han,

Thanks for your comments.

Quoted 2 lines

Good luck.

Hello Jingo Han

Quick update - I will do the #include file reordering

Ian

Quoted 758 lines
Quoted 10 lines
Quoted 10 lines
Signed-off-by Graeme Gregory <[email protected]>
Signed-off-by Ian Lartey <[email protected]>
---
  drivers/leds/leds-palmas.c |  574
++++++++++++++++++++++++++++++++++++++++++++
  include/linux/mfd/palmas.h |   60 +++++
  2 files changed, 634 insertions(+), 0 deletions(-)

create mode 100644 drivers/leds/leds-palmas.c

Quoted 12 lines

drivers/leds/leds-palmas.c line 4

Quoted 10 lines
drivers/leds/leds-palmas.c new file +4 -0
new file mode 100644
index 0000000..4cb73aa
@@ -0,0 +1,574 @@
+/*
+ * Driver for LED part of Palmas PMIC Chips
+ *
+ * Copyright 2011 Texas Instruments Inc.

2013 ?

Code _was_ started a while ago - I'll update.

Quoted 19 lines
Quoted 19 lines
drivers/leds/leds-palmas.c new file +5 -0
+ *
+ * Author: Graeme Gregory <[email protected]>
+ * Author: Ian Lartey <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or

modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */

drivers/leds/leds-palmas.c new file +5 -0
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
Quoted 8 lines

drivers/leds/leds-palmas.c

Quoted 5 lines
drivers/leds/leds-palmas.c new file +5 -0
+#include <linux/leds.h>
+#include <linux/err.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>

Would you order header files according to alphabetical ordering? It would be better for the readability.

It could cause dependency issues so I'd rather leave them in the order they are in.

I'll implement all the other changes you've noted.

Thanks

Ian

Quoted 683 lines
Quoted 233 lines
drivers/leds/leds-palmas.c new file +53 -0
+
+struct palmas_leds_data;
+
+struct palmas_led {
+    struct led_classdev cdev;
+    struct palmas_leds_data *leds_data;
+    int led_no;
+    struct work_struct work;
+    struct mutex mutex;
+
+    spinlock_t value_lock;
+
+    int blink;
+    int on_time;
+    int period;
+    enum led_brightness brightness;
+};
+
+struct palmas_leds_data {
+    struct device *dev;
+    struct led_classdev cdev;
+    struct palmas *palmas;
+
+    struct palmas_led *palmas_led;
+    int no_leds;
+};
+
+#define to_palmas_led(led_cdev) \
+    container_of(led_cdev, struct palmas_led, cdev)
+
+#define LED_ON_62_5MS        0x00
+#define LED_ON_125MS        0x01
+#define LED_ON_250MS        0x02
+#define LED_ON_500MS        0x03
+
+#define LED_PERIOD_OFF        0x00
+#define LED_PERIOD_125MS    0x01
+#define LED_PERIOD_250MS    0x02
+#define LED_PERIOD_500MS    0x03
+#define LED_PERIOD_1000MS    0x04
+#define LED_PERIOD_2000MS    0x05
+#define LED_PERIOD_4000MS    0x06
+#define LED_PERIOD_8000MS    0x07
+
+
+static char *palmas_led_names[] = {
+    "palmas:led1",
+    "palmas:led2",
+    "palmas:led3",
+    "palmas:led4",
+};
+
+static int palmas_led_read(struct palmas_leds_data *leds, unsigned

int reg,

drivers/leds/leds-palmas.c new file +10 -0
+        unsigned int *dest)
+{
+    unsigned int addr;
+
+    addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+    return palmas_read(leds->palmas, PALMAS_LED_BASE, addr, dest);
+}
+
+static int palmas_led_write(struct palmas_leds_data *leds, unsigned

int reg,

drivers/leds/leds-palmas.c new file +23 -0
+        unsigned int value)
+{
+    unsigned int addr;
+
+    addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+    return palmas_write(leds->palmas, PALMAS_LED_BASE, addr, value);
+}
+
+static int palmas_led_update_bits(struct palmas_leds_data *leds,
+        unsigned int reg, unsigned int mask, unsigned int value)
+{
+    unsigned int addr;
+
+    addr = PALMAS_BASE_TO_REG(PALMAS_LED_BASE, reg);
+
+    return palmas_update_bits(leds->palmas, PALMAS_LED_BASE,
+            addr, mask, value);
+}
+
+static void palmas_leds_work(struct work_struct *work)
+{
+    struct palmas_led *led = container_of(work, struct palmas_led,

work);

drivers/leds/leds-palmas.c new file +143 -0
+    unsigned int period, ctrl;
+    unsigned long flags;
+
+    mutex_lock(&led->mutex);
+
+    palmas_led_read(led->leds_data, PALMAS_LED_CTRL, &ctrl);
+    palmas_led_read(led->leds_data, PALMAS_LED_PERIOD_CTRL, &period);
+
+    spin_lock_irqsave(&led->value_lock, flags);
+
+    switch (led->led_no) {
+    case 1:
+        ctrl &= ~PALMAS_LED_CTRL_LED_1_ON_TIME_MASK;
+        period &= ~PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_MASK;
+
+        if (led->blink) {
+            ctrl |= led->on_time <<
+                PALMAS_LED_CTRL_LED_1_ON_TIME_SHIFT;
+            period |= led->period <<
+                PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_SHIFT;
+        } else {
+            /*
+             * for off value is zero which we already set by
+             * masking earlier.
+             */
+            if (led->brightness) {
+                ctrl |= LED_ON_500MS <<
+                    PALMAS_LED_CTRL_LED_1_ON_TIME_SHIFT;
+                period |= LED_PERIOD_500MS <<
+                PALMAS_LED_PERIOD_CTRL_LED_1_PERIOD_SHIFT;
+            }
+        }
+    case 2:
+        ctrl &= ~PALMAS_LED_CTRL_LED_2_ON_TIME_MASK;
+        period &= ~PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_MASK;
+
+        if (led->blink) {
+            ctrl |= led->on_time <<
+                PALMAS_LED_CTRL_LED_2_ON_TIME_SHIFT;
+            period |= led->period <<
+                PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_SHIFT;
+        } else {
+            /*
+             * for off value is zero which we already set by
+             * masking earlier.
+             */
+            if (led->brightness) {
+                ctrl |= LED_ON_500MS <<
+                    PALMAS_LED_CTRL_LED_2_ON_TIME_SHIFT;
+                period |= LED_PERIOD_500MS <<
+                PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_SHIFT;
+            }
+        }
+    case 3:
+        ctrl &= ~PALMAS_LED_CTRL2_LED_3_ON_TIME_MASK;
+        period &= ~PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_MASK;
+
+        if (led->blink) {
+            ctrl |= led->on_time <<
+                PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT;
+            period |= led->period <<
+                PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT;
+        } else {
+            /*
+             * for off value is zero which we already set by
+             * masking earlier.
+             */
+            if (led->brightness) {
+                ctrl |= LED_ON_500MS <<
+                    PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT;
+                period |= LED_PERIOD_500MS <<
+                PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT;
+            }
+        }
+        break;
+    case 4:
+        ctrl &= ~PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_MASK;
+        period &= ~PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_MASK;
+
+        if (led->blink) {
+            ctrl |= led->on_time <<
+                PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT;
+            period |= led->period <<
+                PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT;
+        } else {
+            /*
+             * for off value is zero which we already set by
+             * masking earlier.
+             */
+            if (led->brightness) {
+                ctrl |= LED_ON_500MS <<
+                    PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT;
+                period |= LED_PERIOD_500MS <<
+                PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT;
+            }
+        }
+        break;
+    }
+    spin_unlock_irqrestore(&led->value_lock, flags);
+
+    if (led->led_no < 3) {
+        palmas_led_write(led->leds_data, PALMAS_LED_CTRL, ctrl);
+        palmas_led_write(led->leds_data, PALMAS_LED_PERIOD_CTRL,
+                period);
+    } else {
+        palmas_led_write(led->leds_data, PALMAS_LED_CTRL2, ctrl);
+        palmas_led_write(led->leds_data, PALMAS_LED_PERIOD2_CTRL,
+                period);
+    }
+
+    if (is_palmas_charger(led->leds_data->palmas->product_id)) {
+        if (led->brightness || led->blink)
+            palmas_led_update_bits(led->leds_data, PALMAS_LED_EN,
+                    1 << (led->led_no - 1), 0xFF);
+        else
+            palmas_led_update_bits(led->leds_data, PALMAS_LED_EN,
+                    1 << (led->led_no - 1), 0x00);
+    }
+    mutex_unlock(&led->mutex);
+}
+
+static void palmas_leds_set(struct led_classdev *led_cdev,
+               enum led_brightness value)
+{
+    struct palmas_led *led = to_palmas_led(led_cdev);
+    unsigned long flags;
+
+    spin_lock_irqsave(&led->value_lock, flags);
+    led->brightness = value;
+    led->blink = 0;
+    schedule_work(&led->work);
+    spin_unlock_irqrestore(&led->value_lock, flags);
+}
+
+static int palmas_leds_blink_set(struct led_classdev *led_cdev,
+                   unsigned long *delay_on,
+                   unsigned long *delay_off)
+{
+    struct palmas_led *led = to_palmas_led(led_cdev);
+    unsigned long flags;
+    int ret = 0;
+    int period;
+

drivers/leds/leds-palmas.c

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+    /* Pick some defaults if we've not been given times */
+    if (*delay_on == 0 && *delay_off == 0) {
+        *delay_on = 250;
+        *delay_off = 250;
+    }
+
+    spin_lock_irqsave(&led->value_lock, flags);
+
+    /* We only have a limited selection of settings, see if we can
+     * support the configuration we're being given */

According to the Coding Style, the preferred style for multi-line comments is:

+ /*

  +     * We only have a limited selection of settings, see if we can
  +     * support the configuration we're being given
  +     */
Quoted 51 lines
drivers/leds/leds-palmas.c new file +51 -0
+    switch (*delay_on) {
+    case 500:
+        led->on_time = LED_ON_500MS;
+        break;
+    case 250:
+        led->on_time = LED_ON_250MS;
+        break;
+    case 125:
+        led->on_time = LED_ON_125MS;
+        break;
+    case 62:
+    case 63:
+        /* Actually 62.5ms */
+        led->on_time = LED_ON_62_5MS;
+        break;
+    default:
+        ret = -EINVAL;
+        break;
+    }
+
+    period = *delay_on + *delay_off;
+
+    if (ret == 0) {
+        switch (period) {
+        case 124:
+        case 125:
+        case 126:
+            /* All possible variations of 62.5 + 62.5 */
+            led->period = LED_PERIOD_125MS;
+            break;
+        case 250:
+            led->period = LED_PERIOD_250MS;
+            break;
+        case 500:
+            led->period = LED_PERIOD_500MS;
+            break;
+        case 1000:
+            led->period = LED_PERIOD_1000MS;
+            break;
+        case 2000:
+            led->period = LED_PERIOD_2000MS;
+            break;
+        case 4000:
+            led->period = LED_PERIOD_4000MS;
+            break;
+        case 8000:
+            led->period = LED_PERIOD_8000MS;
+            break;
+        default:
+            ret = -EINVAL;
+            break;

drivers/leds/leds-palmas.c

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+        }
+    }
+
+    if (ret == 0)
+        led->blink = 1;
+    else
+        led->blink = 0;
+
+    /* Always update; if we fail turn off blinking since we expect
+     * a software fallback. */

Same as above.

Quoted 88 lines
drivers/leds/leds-palmas.c new file +8 -0
+    schedule_work(&led->work);
+
+    spin_unlock_irqrestore(&led->value_lock, flags);
+
+    return ret;
+}
+
+static void palmas_init_led(struct palmas_leds_data *leds_data, int

offset,

drivers/leds/leds-palmas.c new file +9 -0
+        int led_no)
+{
+    mutex_init(&leds_data->palmas_led[offset].mutex);
+    INIT_WORK(&leds_data->palmas_led[offset].work, palmas_leds_work);
+    spin_lock_init(&leds_data->palmas_led[offset].value_lock);
+
+    leds_data->palmas_led[offset].leds_data = leds_data;
+    leds_data->palmas_led[offset].led_no = led_no;
+    leds_data->palmas_led[offset].cdev.name =

palmas_led_names[led_no - 1];

drivers/leds/leds-palmas.c new file +2 -0
+    leds_data->palmas_led[offset].cdev.default_trigger = NULL;
+    leds_data->palmas_led[offset].cdev.brightness_set =

palmas_leds_set;
+ leds_data->palmas_led[offset].cdev.blink_set =
palmas_leds_blink_set;

drivers/leds/leds-palmas.c new file +37 -0
+}
+
+static int  palmas_dt_to_pdata(struct device *dev,
+        struct device_node *node,
+        struct palmas_leds_platform_data *pdata)
+{
+    struct device_node *child_node;
+    int ret;
+    u32 prop;
+
+    child_node = of_get_child_by_name(node, "palmas_leds");
+    if (!child_node) {
+        dev_err(dev, "child node 'palmas_leds' not found\n");
+        return -EINVAL;
+    }
+
+    ret = of_property_read_u32(child_node, "ti,led1_current", &prop);
+    if (!ret)
+        pdata->led1_current = prop;
+
+    ret = of_property_read_u32(child_node, "ti,led2_current", &prop);
+    if (!ret)
+        pdata->led2_current = prop;
+
+    ret = of_property_read_u32(child_node, "ti,led3_current", &prop);
+    if (!ret)
+        pdata->led3_current = prop;
+
+    ret = of_property_read_u32(child_node, "ti,led4_current", &prop);
+    if (!ret)
+        pdata->led4_current = prop;
+
+    ret = of_property_read_u32(child_node, "ti,chrg_led_mode", &prop);
+    if (!ret)
+        pdata->chrg_led_mode = prop;
+
+    ret = of_property_read_u32(child_node, "ti,chrg_led_vbat_low",

&prop);

drivers/leds/leds-palmas.c new file +26 -0
+    if (!ret)
+        pdata->chrg_led_vbat_low = prop;
+
+    return 0;
+}
+
+static int palmas_leds_probe(struct platform_device *pdev)
+{
+    struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+    struct palmas_leds_platform_data *pdata = pdev->dev.platform_data;
+    struct palmas_leds_data *leds_data;
+    struct device_node *node = pdev->dev.of_node;
+    int ret, i;
+    int offset = 0;
+
+    if (!palmas->led_muxed && !is_palmas_charger(palmas->product_id)) {
+        dev_err(&pdev->dev, "there are no LEDs muxed\n");
+        return -EINVAL;
+    }
+
+    /* Palmas charger requires platform data */
+    if (is_palmas_charger(palmas->product_id) && node && !pdata) {
+        pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+
+        if (!pdata)
+            return -ENOMEM;

drivers/leds/leds-palmas.c

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+        ret = palmas_dt_to_pdata(&pdev->dev, node, pdata);
+        if (ret)
+            return ret;
+    }
+
+    if (is_palmas_charger(palmas->product_id) && !pdata)
+        return -EINVAL;
+
+    leds_data = kzalloc(sizeof(*leds_data), GFP_KERNEL);

How about using devm_kzalloc()?
Memory allocated with this function is automatically freed on driver detach. So it makes error path more simple.

Quoted 18 lines
drivers/leds/leds-palmas.c new file +18 -0
+    if (!leds_data)
+        return -ENOMEM;
+    platform_set_drvdata(pdev, leds_data);
+
+    leds_data->palmas = palmas;
+
+    switch (palmas->led_muxed) {
+    case PALMAS_LED1_MUXED | PALMAS_LED2_MUXED:
+        leds_data->no_leds = 2;
+        break;
+    case PALMAS_LED1_MUXED:
+    case PALMAS_LED2_MUXED:
+        leds_data->no_leds = 1;
+        break;
+    default:
+        leds_data->no_leds = 0;
+        break;
+    }

drivers/leds/leds-palmas.c

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+
+    if (is_palmas_charger(palmas->product_id)) {
+        if (pdata->chrg_led_mode)
+            leds_data->no_leds += 2;
+        else
+            leds_data->no_leds++;
+    }
+
+    leds_data->palmas_led = kcalloc(leds_data->no_leds,
+            sizeof(struct palmas_led), GFP_KERNEL);

How about using devm_kzalloc()?

Quoted 70 lines
drivers/leds/leds-palmas.c new file +70 -0
+
+    /* Initialise LED1 */
+    if (palmas->led_muxed & PALMAS_LED1_MUXED) {
+        palmas_init_led(leds_data, offset, 1);
+        offset++;
+    }
+
+    /* Initialise LED2 */
+    if (palmas->led_muxed & PALMAS_LED2_MUXED) {
+        palmas_init_led(leds_data, offset, 2);
+        offset++;
+    }
+
+    if (is_palmas_charger(palmas->product_id)) {
+        palmas_init_led(leds_data, offset, 3);
+        offset++;
+
+        if (pdata->chrg_led_mode) {
+            palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,
+                    PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE,
+                    PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE);
+
+            palmas_init_led(leds_data, offset, 4);
+        }
+    }
+
+    for (i = 0; i < leds_data->no_leds; i++) {
+        ret = led_classdev_register(leds_data->dev,
+                &leds_data->palmas_led[i].cdev);
+        if (ret < 0) {
+            dev_err(&pdev->dev,
+                "Failed to register LED no: %d err: %d\n",
+                i, ret);
+            goto err_led;
+        }
+    }
+
+    if (!is_palmas_charger(palmas->product_id))
+        return 0;
+
+    /* Set the LED current registers if set in platform data */
+    if (pdata->led1_current)
+        palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL1,
+                PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_MASK,
+                pdata->led1_current);
+
+    if (pdata->led2_current)
+        palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL1,
+                PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_MASK,
+                pdata->led2_current <<
+                PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_SHIFT);
+
+    if (pdata->led3_current)
+        palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL2,
+                PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK,
+                pdata->led3_current);
+
+    if (pdata->led3_current)
+        palmas_led_update_bits(leds_data, PALMAS_LED_CURRENT_CTRL2,
+                PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK,
+                pdata->led3_current);
+
+    if (pdata->led4_current)
+        palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,
+                PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_MASK,
+                pdata->led4_current <<
+                PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_SHIFT);
+
+    if (pdata->chrg_led_vbat_low)
+        palmas_led_update_bits(leds_data, PALMAS_CHRG_LED_CTRL,

drivers/leds/leds-palmas.c

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+                PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS,
+                PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS);
+
+    return 0;
+
+err_led:
+    for (i = 0; i < leds_data->no_leds; i++)
+        led_classdev_unregister(&leds_data->palmas_led[i].cdev);
+    kfree(leds_data->palmas_led);
+    kfree(leds_data);

If you use devm_kzalloc() above, you don't need to use kfree().

drivers/leds/leds-palmas.c

Quoted 13 lines
drivers/leds/leds-palmas.c new file +13 -0
+    return ret;
+}
+
+static int palmas_leds_remove(struct platform_device *pdev)
+{
+    struct palmas_leds_data *leds_data = platform_get_drvdata(pdev);
+    int i;
+
+    for (i = 0; i < leds_data->no_leds; i++)
+        led_classdev_unregister(&leds_data->palmas_led[i].cdev);
+    if (i)
+        kfree(leds_data->palmas_led);
+    kfree(leds_data);

If you use devm_kzalloc() above, you don't need to use kfree().

Quoted 20 lines
drivers/leds/leds-palmas.c new file +20 -0
+
+    return 0;
+}
+
+static struct of_device_id of_palmas_match_tbl[] = {
+    { .compatible = "ti,palmas-leds", },
+    { /* end */ }
+};
+
+static struct platform_driver palmas_leds_driver = {
+    .driver = {
+        .name = "palmas-leds",
+        .of_match_table = of_palmas_match_tbl,
+        .owner = THIS_MODULE,
+    },
+    .probe = palmas_leds_probe,
+    .remove = palmas_leds_remove,
+};
+
+static int  palmas_leds_init(void)

drivers/leds/leds-palmas.c

Quoted 10 lines
drivers/leds/leds-palmas.c new file +10 -0
+{
+    return platform_driver_register(&palmas_leds_driver);
+}
+module_init(palmas_leds_init);
+
+static void palmas_leds_exit(void)
+{
+    platform_driver_unregister(&palmas_leds_driver);
+}
+module_exit(palmas_leds_exit);

Please use module_platform_driver() macro which makes the code smaller and simpler.

+module_platform_driver(palmas_leds_driver);

Quoted 94 lines
drivers/leds/leds-palmas.c new file +6 -0
+
+MODULE_AUTHOR("Graeme Gregory <[email protected]>");
+MODULE_DESCRIPTION("Palmas LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:palmas-leds");
+MODULE_DEVICE_TABLE(of, of_palmas_match_tbl);
include/linux/mfd/palmas.h +60 -1
index 0b86845..856f54e 100644
@@ -232,6 +232,16 @@ struct palmas_clk_platform_data {
      int clk32kgaudio_mode_sleep;
  };
 
+struct palmas_leds_platform_data {
+    int led1_current;
+    int led2_current;
+    int led3_current;
+    int led4_current;
+
+    int chrg_led_mode;
+    int chrg_led_vbat_low;
+};
+
  struct palmas_platform_data {
      int gpio_base;
 

@@ -1855,6 +1865,12 @@ enum usb_irq_events {
  #define PALMAS_LED_CTRL                        0x1
  #define PALMAS_PWM_CTRL1                    0x2
  #define PALMAS_PWM_CTRL2                    0x3
+#define PALMAS_LED_PERIOD2_CTRL                    0x4
+#define PALMAS_LED_CTRL2                    0x5
+#define PALMAS_LED_CURRENT_CTRL1                0x6
+#define PALMAS_LED_CURRENT_CTRL2                0x7
+#define PALMAS_CHRG_LED_CTRL                    0x8
+#define PALMAS_LED_EN                        0x9
 
  /* Bit definitions for LED_PERIOD_CTRL */
  #define PALMAS_LED_PERIOD_CTRL_LED_2_PERIOD_MASK        0x38

@@ -1882,6 +1898,50 @@ enum usb_irq_events {
  #define PALMAS_PWM_CTRL2_PWM_DUTY_SEL_MASK            0xff
  #define PALMAS_PWM_CTRL2_PWM_DUTY_SEL_SHIFT            0
 
+/* Bit definitions for LED_PERIOD2_CTRL */
+#define PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_MASK        0x38
+#define PALMAS_LED_PERIOD2_CTRL_CHRG_LED_PERIOD_SHIFT        3
+#define PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_MASK        0x07
+#define PALMAS_LED_PERIOD2_CTRL_LED_3_PERIOD_SHIFT        0
+
+/* Bit definitions for LED_CTRL2 */
+#define PALMAS_LED_CTRL2_CHRG_LED_SEQ                0x20
+#define PALMAS_LED_CTRL2_CHRG_LED_SEQ_SHIFT            5
+#define PALMAS_LED_CTRL2_LED_3_SEQ                0x10
+#define PALMAS_LED_CTRL2_LED_3_SEQ_SHIFT            4
+#define PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_MASK            0x0c
+#define PALMAS_LED_CTRL2_CHRG_LED_ON_TIME_SHIFT            2
+#define PALMAS_LED_CTRL2_LED_3_ON_TIME_MASK            0x03
+#define PALMAS_LED_CTRL2_LED_3_ON_TIME_SHIFT            0
+
+/* Bit definitions for LED_CURRENT_CTRL1 */
+#define PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_MASK        0x38
+#define PALMAS_LED_CURRENT_CTRL1_LED_2_CURRENT_SHIFT        3
+#define PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_MASK        0x07
+#define PALMAS_LED_CURRENT_CTRL1_LED_1_CURRENT_SHIFT        0
+
+/* Bit definitions for LED_CURRENT_CTRL2 */
+#define PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_MASK        0x07
+#define PALMAS_LED_CURRENT_CTRL2_LED_3_CURRENT_SHIFT        0
+
+/* Bit definitions for CHRG_LED_CTRL */
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_MASK        0x38
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_CURRENT_SHIFT        3
+#define PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS        0x02
+#define PALMAS_CHRG_LED_CTRL_CHRG_LOWBAT_BLK_DIS_SHIFT        1
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE            0x01
+#define PALMAS_CHRG_LED_CTRL_CHRG_LED_MODE_SHIFT        0
+
+/* Bit definitions for LED_EN */
+#define PALMAS_LED_EN_CHRG_LED_EN                0x08
+#define PALMAS_LED_EN_CHRG_LED_EN_SHIFT                3
+#define PALMAS_LED_EN_LED_3_EN                    0x04
+#define PALMAS_LED_EN_LED_3_EN_SHIFT                2
+#define PALMAS_LED_EN_LED_2_EN                    0x02
+#define PALMAS_LED_EN_LED_2_EN_SHIFT                1
+#define PALMAS_LED_EN_LED_1_EN                    0x01
+#define PALMAS_LED_EN_LED_1_EN_SHIFT                0
+
  /* Registers for function INTERRUPT */
  #define PALMAS_INT1_STATUS                    0x0
  #define PALMAS_INT1_MASK                    0x1
--

1.7.0.4

Quoted 8 lines
Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

N떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴쪐{콗喩zX㎍썳變}찠꼿쟺 �&j:+v돣�쳭喩zZ+�+zf＂톒쉱�~넮녬i�鎬z�췿ⅱ�?솳鈺�&�)刪f뷌^j푹y쬶 끷@A첺뛴�� 0띠h��i�

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/