Messages in this thread | | | Date | Thu, 28 Feb 2013 11:14:47 +0530 | From | Laxman Dewangan <> | Subject | Re: [PATCH 1/2] leds: Add support for Palmas LEDs |
| |
On Wednesday 27 February 2013 06:43 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. > > Signed-off-by: Graeme Gregory <gg@slimlogic.co.uk> > Signed-off-by: Ian Lartey <ian@slimlogic.co.uk> > ---
Patch 1 and 2 can be merged together as this is makefile, Kconfig and driver addition.
> + > +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); This is not require. Infact doing this will not work.
palmas_read already take care of proper offset. unsigned int addr = PALMAS_BASE_TO_REG(base, reg); int slave_id = PALMAS_BASE_TO_SLAVE(base);
return regmap_read(palmas->regmap[slave_id], addr, val);
same for other places also.
> > +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; > +
case 1,2,3 and 4 looks same, jus shift/mask are changed. I think this can be rewritten here to reduce code size.
> +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);
Use devm_kzalloc().
> + > +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);
use Module_platform_driver().
| |