lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 28 Feb 2013 01:12:55 +0000 (GMT)
From:	Jingoo Han <jg1.han@...sung.com>
To:	Ian Lartey <ian@...mlogic.co.uk>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"ldewangan@...dia.com" <ldewangan@...dia.com>,
	"j-keerthy@...com" <j-keerthy@...com>,
	"gg@...mlogic.co.uk" <gg@...mlogic.co.uk>,
	"cooloney@...il.com" <cooloney@...il.com>,
	"rpurdie@...ys.net" <rpurdie@...ys.net>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	Jingoo Han <jg1.han@...sung.com>
Subject: Re: [PATCH 1/2] leds: Add support for Palmas LEDs

On Wednesday, February 27, 2013 10:13 PM, Ian Lartey wrote:
> 
> 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.

> 
> Signed-off-by: Graeme Gregory <gg@...mlogic.co.uk>
> Signed-off-by: Ian Lartey <ian@...mlogic.co.uk>
> ---
>  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
> 
> diff --git a/drivers/leds/leds-palmas.c b/drivers/leds/leds-palmas.c
> new file mode 100644
> index 0000000..4cb73aa
> --- /dev/null
> +++ b/drivers/leds/leds-palmas.c
> @@ -0,0 +1,574 @@
> +/*
> + * Driver for LED part of Palmas PMIC Chips
> + *
> + * Copyright 2011 Texas Instruments Inc.

2013 ?

> + *
> + * Author: Graeme Gregory <gg@...mlogic.co.uk>
> + * Author: Ian Lartey <ian@...mlogic.co.uk>
> + *
> + *  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>
> +#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.

> +
> +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;
> +
> +	/* 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
 +	 */


> +	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;
> +		}
> +	}
> +
> +	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.

> +	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;
> +
> +		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.


> +	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;
> +	}
> +
> +	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()?


> +
> +	/* 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,
> +				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().

> +	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().

> +
> +	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)
> +{
> +	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);


> +
> +MODULE_AUTHOR("Graeme Gregory <gg@...mlogic.co.uk>");
> +MODULE_DESCRIPTION("Palmas LED driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:palmas-leds");
> +MODULE_DEVICE_TABLE(of, of_palmas_match_tbl);
> diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> index 0b86845..856f54e 100644
> --- a/include/linux/mfd/palmas.h
> +++ b/include/linux/mfd/palmas.h
> @@ -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 majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ