On Tue, Mar 17, 2009 at 08:38, <[email protected]> wrote:
> --- trunk/drivers/video/backlight/Kconfig 2009-03-17 12:24:38 UTC (rev
> 6184)
> +++ trunk/drivers/video/backlight/Kconfig 2009-03-17 12:38:22 UTC (rev
> 6185)
> @@ -214,3 +214,10 @@
> +config BACKLIGHT_ADP5520
> + tristate "Backlight Driver for ADP5520/ADP5501 using WLED"
> + depends on BACKLIGHT_CLASS_DEVICE && PMIC_ADP5520
> + help
> + If you have a LCD backlight connected to the BST/BL_SNK output of
> + ADP5520 or ADP5501, say Y here to enable this driver.
could you add info as to what the module will be called if selected as such ?
> --- trunk/drivers/video/backlight/adp5520_bl.c
> +++ trunk/drivers/video/backlight/adp5520_bl.c
@@ -0,0 +1,368 @@
> + * Copyright 2009 Analog Devices Inc.
> + *
> + *
only need one empty comment line here ...
> +static int adp5520_bl_set(struct backlight_device *bl, int brightness)
> +{
> + ...
> + } else {
> + ret |= adp5520_write(master, DAYLIGHT_MAX, brightness);
> + }
dont need these braces
> + if (data->current_brightness && brightness == 0)
> + ret |= adp5520_set_bits(master,
> + MODE_STATUS, DIM_EN);
> +
> + if (data->current_brightness == 0 && brightness)
else if ?
> + if (ret)
> + return ret;
> +
> + data->current_brightness = brightness;
> + return 0;
maybe better structured as
if (!ret)
data->current_brightness = brightness;
return ret;
> +static int adp5520_bl_setup(struct backlight_device *bl)
only called by _probe, so this should be __devinit
> +static int adp5520_bl_probe(struct platform_device *pdev)
__devinit
> + if (data->pdata->en_ambl_sens)
> + ret = sysfs_create_group(&bl->dev.kobj,
> + &adp5520_bl_attr_group);
> +
> + platform_set_drvdata(pdev, bl);
> + ret |= adp5520_bl_setup(bl);
> + backlight_update_status(bl);
> +
> + return ret;
can sysfs_create_group() fail ? if it does, do we care ?
> +static int adp5520_bl_remove(struct platform_device *pdev)
__devexit
> +{
> + struct backlight_device *bl = platform_get_drvdata(pdev);
> + struct adp5520_bl *data = bl_get_data(bl);
> + int ret;
> +
> + ret = adp5520_clr_bits(data->master, MODE_STATUS, BL_EN);
> +
> + if (data->pdata->en_ambl_sens)
> + sysfs_remove_group(&bl->dev.kobj,
> + &adp5520_bl_attr_group);
> +
> + backlight_device_unregister(bl);
> + kfree(data);
> +
> + return ret;
> +}
if the ret is non-zero, i'm guessing the means the instance wont be
removed and so the remove function will be called again. if that
happens, the freeing here will happen doubly and thus crash right ?
-mike
_______________________________________________
Linux-kernel-commits mailing list
[email protected]
http://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
