* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver [not found] <Pine.LNX.4.64.1301081846020.1794@axis700.grange> @ 2013-01-25 5:20 ` Jingoo Han 2013-01-25 7:48 ` Guennadi Liakhovetski 0 siblings, 1 reply; 4+ messages in thread From: Jingoo Han @ 2013-01-25 5:20 UTC (permalink / raw) To: 'Guennadi Liakhovetski' Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh, 'Magnus Damm', 'Richard Purdie', linux-kernel, 'Jingoo Han' On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote > > This is an initial commit of a backlight driver, using step-up DCDC power > supplies on AS3711 PMIC. Only one mode has actually been tested, several > further modes have been implemented "dry," but disabled to avoid accidental > hardware damage. Anyone wishing to use any of those modes will have to > modify the driver. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> CC'ed Andrew Morton. Hi Guennadi Liakhovetski, I have reviewed this patch with AS3711 datasheet. I cannot find any problems. It looks good. However, some hardcoded numbers need to be changed to the bit definitions. Acked-by: Jingoo Han <jg1.han@samsung.com> Best regards, Jingoo Han > --- > > Tested on sh73a0-based kzm9g board. As the commit message says, only one > mode has been tested and is enabled. That mode copies the sample code from > the manufacturer. Deviations from that code proved to be fatal for the > hardware... > > drivers/video/backlight/Kconfig | 7 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/as3711_bl.c | 379 +++++++++++++++++++++++++++++++++++ > 3 files changed, 387 insertions(+), 0 deletions(-) > create mode 100644 drivers/video/backlight/as3711_bl.c > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index 765a945..6ef0ede 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217 > If you have a Texas Instruments TPS65217 say Y to enable the > backlight driver. > > +config BACKLIGHT_AS3711 > + tristate "AS3711 Backlight" > + depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711 > + help > + If you have an Austrian Microsystems AS3711 say Y to enable the > + backlight driver. > + > endif # BACKLIGHT_CLASS_DEVICE > > endif # BACKLIGHT_LCD_SUPPORT > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > index e7ce729..d3e188f 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o > obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o > obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c > new file mode 100644 > index 0000000..c6bc65d > --- /dev/null > +++ b/drivers/video/backlight/as3711_bl.c > @@ -0,0 +1,379 @@ > +/* > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters > + * > + * Copyright (C) 2012 Renesas Electronics Corporation > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the version 2 of the GNU General Public License as > + * published by the Free Software Foundation > + */ > + > +#include <linux/backlight.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/fb.h> > +#include <linux/kernel.h> > +#include <linux/mfd/as3711.h> > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +enum as3711_bl_type { > + AS3711_BL_SU1, > + AS3711_BL_SU2, > +}; > + > +struct as3711_bl_data { > + bool powered; > + const char *fb_name; > + struct device *fb_dev; > + enum as3711_bl_type type; > + int brightness; > + struct backlight_device *bl; > +}; > + > +struct as3711_bl_supply { > + struct as3711_bl_data su1; > + struct as3711_bl_data su2; > + const struct as3711_bl_pdata *pdata; > + struct as3711 *as3711; > +}; > + > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su) > +{ > + switch (su->type) { > + case AS3711_BL_SU1: > + return container_of(su, struct as3711_bl_supply, su1); > + case AS3711_BL_SU2: > + return container_of(su, struct as3711_bl_supply, su2); > + } > + return NULL; > +} > + > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data, > + unsigned int brightness) > +{ > + struct as3711_bl_supply *supply = to_supply(data); > + struct as3711 *as3711 = supply->as3711; > + const struct as3711_bl_pdata *pdata = supply->pdata; > + int ret = 0; > + > + /* Only all equal current values are supported */ > + if (pdata->su2_auto_curr1) > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > + brightness); > + if (!ret && pdata->su2_auto_curr2) > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > + brightness); > + if (!ret && pdata->su2_auto_curr3) > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > + brightness); > + > + return ret; > +} > + > +static int as3711_set_brightness_v(struct as3711 *as3711, > + unsigned int brightness, > + unsigned int reg) > +{ > + if (brightness > 31) > + return -EINVAL; > + > + return regmap_update_bits(as3711->regmap, reg, 0xf0, > + brightness << 4); > +} > + > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply) > +{ > + struct as3711 *as3711 = supply->as3711; > + int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5, > + 3, supply->pdata->su2_fbprot); > + if (!ret) > + ret = regmap_update_bits(as3711->regmap, > + AS3711_STEPUP_CONTROL_2, 1, 0); > + if (!ret) > + ret = regmap_update_bits(as3711->regmap, > + AS3711_STEPUP_CONTROL_2, 1, 1); > + return ret; > +} > + > +/* > + * Someone with less fragile or less expensive hardware could try to simplify > + * the brightness adjustment procedure. > + */ > +static int as3711_bl_update_status(struct backlight_device *bl) > +{ > + struct as3711_bl_data *data = bl_get_data(bl); > + struct as3711_bl_supply *supply = to_supply(data); > + struct as3711 *as3711 = supply->as3711; > + int brightness = bl->props.brightness; > + int ret = 0; > + > + dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n", > + __func__, bl->props.brightness, bl->props.power, > + bl->props.fb_blank, bl->props.state); > + > + if (bl->props.power != FB_BLANK_UNBLANK || > + bl->props.fb_blank != FB_BLANK_UNBLANK || > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > + brightness = 0; > + > + if (data->type == AS3711_BL_SU1) { > + ret = as3711_set_brightness_v(as3711, brightness, > + AS3711_STEPUP_CONTROL_1); > + } else { > + const struct as3711_bl_pdata *pdata = supply->pdata; > + > + switch (pdata->su2_feedback) { > + case AS3711_SU2_VOLTAGE: > + ret = as3711_set_brightness_v(as3711, brightness, > + AS3711_STEPUP_CONTROL_2); > + break; > + case AS3711_SU2_CURR_AUTO: > + ret = as3711_set_brightness_auto_i(data, brightness / 4); > + if (ret < 0) > + return ret; > + if (brightness) { > + ret = as3711_bl_su2_reset(supply); > + if (ret < 0) > + return ret; > + udelay(500); > + ret = as3711_set_brightness_auto_i(data, brightness); > + } else { > + ret = regmap_update_bits(as3711->regmap, > + AS3711_STEPUP_CONTROL_2, 1, 0); > + } > + break; > + /* Manual one current feedback pin below */ > + case AS3711_SU2_CURR1: > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > + brightness); > + break; > + case AS3711_SU2_CURR2: > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > + brightness); > + break; > + case AS3711_SU2_CURR3: > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > + brightness); > + break; > + default: > + ret = -EINVAL; > + } > + } > + if (!ret) > + data->brightness = brightness; > + > + return ret; > +} > + > +static int as3711_bl_get_brightness(struct backlight_device *bl) > +{ > + struct as3711_bl_data *data = bl_get_data(bl); > + > + return data->brightness; > +} > + > +static const struct backlight_ops as3711_bl_ops = { > + .update_status = as3711_bl_update_status, > + .get_brightness = as3711_bl_get_brightness, > +}; > + > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply) > +{ > + struct as3711 *as3711 = supply->as3711; > + const struct as3711_bl_pdata *pdata = supply->pdata; > + u8 ctl = 0; > + int ret; > + > + dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback); > + > + /* Turn SU2 off */ > + ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0); > + if (ret < 0) > + return ret; > + > + switch (pdata->su2_feedback) { > + case AS3711_SU2_VOLTAGE: > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0); > + break; > + case AS3711_SU2_CURR1: > + ctl = 1; > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1); > + break; > + case AS3711_SU2_CURR2: > + ctl = 4; > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2); > + break; > + case AS3711_SU2_CURR3: > + ctl = 0x10; > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3); > + break; > + case AS3711_SU2_CURR_AUTO: > + if (pdata->su2_auto_curr1) > + ctl = 2; > + if (pdata->su2_auto_curr2) > + ctl |= 8; > + if (pdata->su2_auto_curr3) > + ctl |= 0x20; > + ret = 0; > + break; > + default: > + return -EINVAL; > + } > + > + if (!ret) > + ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl); > + > + return ret; > +} > + > +static int as3711_bl_register(struct platform_device *pdev, > + unsigned int max_brightness, struct as3711_bl_data *su) > +{ > + struct backlight_properties props = {.type = BACKLIGHT_RAW,}; > + struct backlight_device *bl; > + > + /* max tuning I = 31uA for voltage- and 38250uA for current-feedback */ > + props.max_brightness = max_brightness; > + > + bl = backlight_device_register(su->type == AS3711_BL_SU1 ? > + "as3711-su1" : "as3711-su2", > + &pdev->dev, su, > + &as3711_bl_ops, &props); > + if (IS_ERR(bl)) { > + dev_err(&pdev->dev, "failed to register backlight\n"); > + return PTR_ERR(bl); > + } > + > + bl->props.brightness = props.max_brightness; > + > + backlight_update_status(bl); > + > + su->bl = bl; > + > + return 0; > +} > + > +static int as3711_backlight_probe(struct platform_device *pdev) > +{ > + struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev); > + struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent); > + struct as3711_bl_supply *supply; > + struct as3711_bl_data *su; > + unsigned int max_brightness; > + int ret; > + > + if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) { > + dev_err(&pdev->dev, "No platform data, exiting...\n"); > + return -ENODEV; > + } > + > + /* > + * Due to possible hardware damage I chose to block all modes, > + * unsupported on my hardware. Anyone, wishing to use any of those modes > + * will have to first review the code, then activate and test it. > + */ > + if (pdata->su1_fb || > + pdata->su2_fbprot != AS3711_SU2_GPIO4 || > + pdata->su2_feedback != AS3711_SU2_CURR_AUTO) { > + dev_warn(&pdev->dev, > + "Attention! An untested mode has been chosen!\n" > + "Please, review the code, enable, test, and report success:-)\n"); > + return -EINVAL; > + } > + > + supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL); > + if (!supply) > + return -ENOMEM; > + > + supply->as3711 = as3711; > + supply->pdata = pdata; > + > + if (pdata->su1_fb) { > + su = &supply->su1; > + su->fb_name = pdata->su1_fb; > + su->type = AS3711_BL_SU1; > + > + max_brightness = min(pdata->su1_max_uA, 31); > + ret = as3711_bl_register(pdev, max_brightness, su); > + if (ret < 0) > + return ret; > + } > + > + if (pdata->su2_fb) { > + su = &supply->su2; > + su->fb_name = pdata->su2_fb; > + su->type = AS3711_BL_SU2; > + > + switch (pdata->su2_fbprot) { > + case AS3711_SU2_GPIO2: > + case AS3711_SU2_GPIO3: > + case AS3711_SU2_GPIO4: > + case AS3711_SU2_LX_SD4: > + break; > + default: > + ret = -EINVAL; > + goto esu2; > + } > + > + switch (pdata->su2_feedback) { > + case AS3711_SU2_VOLTAGE: > + max_brightness = min(pdata->su2_max_uA, 31); > + break; > + case AS3711_SU2_CURR1: > + case AS3711_SU2_CURR2: > + case AS3711_SU2_CURR3: > + case AS3711_SU2_CURR_AUTO: > + max_brightness = min(pdata->su2_max_uA / 150, 255); > + break; > + default: > + ret = -EINVAL; > + goto esu2; > + } > + > + ret = as3711_bl_init_su2(supply); > + if (ret < 0) > + return ret; > + > + ret = as3711_bl_register(pdev, max_brightness, su); > + if (ret < 0) > + goto esu2; > + } > + > + platform_set_drvdata(pdev, supply); > + > + return 0; > + > +esu2: > + backlight_device_unregister(supply->su1.bl); > + return ret; > +} > + > +static int as3711_backlight_remove(struct platform_device *pdev) > +{ > + struct as3711_bl_supply *supply = platform_get_drvdata(pdev); > + > + backlight_device_unregister(supply->su1.bl); > + backlight_device_unregister(supply->su2.bl); > + > + return 0; > +} > + > +static struct platform_driver as3711_backlight_driver = { > + .driver = { > + .name = "as3711-backlight", > + .owner = THIS_MODULE, > + }, > + .probe = as3711_backlight_probe, > + .remove = as3711_backlight_remove, > +}; > + > +module_platform_driver(as3711_backlight_driver); > + > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs"); > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:as3711-backlight"); > -- > 1.7.2.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver 2013-01-25 5:20 ` [PATCH] backlight: add an AS3711 PMIC backlight driver Jingoo Han @ 2013-01-25 7:48 ` Guennadi Liakhovetski 2013-01-28 0:53 ` Jingoo Han 0 siblings, 1 reply; 4+ messages in thread From: Guennadi Liakhovetski @ 2013-01-25 7:48 UTC (permalink / raw) To: Jingoo Han Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh, 'Magnus Damm', 'Richard Purdie', linux-kernel Hi Jingoo Han On Fri, 25 Jan 2013, Jingoo Han wrote: > On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote > > > > This is an initial commit of a backlight driver, using step-up DCDC power > > supplies on AS3711 PMIC. Only one mode has actually been tested, several > > further modes have been implemented "dry," but disabled to avoid accidental > > hardware damage. Anyone wishing to use any of those modes will have to > > modify the driver. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > CC'ed Andrew Morton. > > Hi Guennadi Liakhovetski, > > I have reviewed this patch with AS3711 datasheet. > I cannot find any problems. It looks good. Thanks for the review. > However, some hardcoded numbers need to be changed > to the bit definitions. Which specific hardcoded values do you mean? A while ago in a discussion on one of kernel mailing lists a conclusion has been made, that macros do not always improve code readability. E.g. in a statement like this + case AS3711_SU2_CURR_AUTO: + if (pdata->su2_auto_curr1) + ctl = 2; + if (pdata->su2_auto_curr2) + ctl |= 8; + if (pdata->su2_auto_curr3) + ctl |= 0x20; making it + case AS3711_SU2_CURR_AUTO: + if (pdata->su2_auto_curr1) + ctl = SU2_AUTO_CURR1; would hardly make it more readable. IMHO it is already pretty clear, that bit 1 enables the current-1 sink. As long as these fields are only used at one location in the driver (and they are), using a macro and defining it elsewhere only makes it harder to see actual values. Speaking of this, a comment like /* * Select, which current sinks shall be used for automatic * feedback selection */ would help much more than any macro names. But as it stands, I think the current version is also possible to understand :-) If desired, however, comments can be added in an incremental patch. Thanks Guennadi > Acked-by: Jingoo Han <jg1.han@samsung.com> > > > Best regards, > Jingoo Han > > > --- > > > > Tested on sh73a0-based kzm9g board. As the commit message says, only one > > mode has been tested and is enabled. That mode copies the sample code from > > the manufacturer. Deviations from that code proved to be fatal for the > > hardware... > > > > drivers/video/backlight/Kconfig | 7 + > > drivers/video/backlight/Makefile | 1 + > > drivers/video/backlight/as3711_bl.c | 379 +++++++++++++++++++++++++++++++++++ > > 3 files changed, 387 insertions(+), 0 deletions(-) > > create mode 100644 drivers/video/backlight/as3711_bl.c > > > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > > index 765a945..6ef0ede 100644 > > --- a/drivers/video/backlight/Kconfig > > +++ b/drivers/video/backlight/Kconfig > > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217 > > If you have a Texas Instruments TPS65217 say Y to enable the > > backlight driver. > > > > +config BACKLIGHT_AS3711 > > + tristate "AS3711 Backlight" > > + depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711 > > + help > > + If you have an Austrian Microsystems AS3711 say Y to enable the > > + backlight driver. > > + > > endif # BACKLIGHT_CLASS_DEVICE > > > > endif # BACKLIGHT_LCD_SUPPORT > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > > index e7ce729..d3e188f 100644 > > --- a/drivers/video/backlight/Makefile > > +++ b/drivers/video/backlight/Makefile > > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o > > obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o > > obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o > > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o > > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o > > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c > > new file mode 100644 > > index 0000000..c6bc65d > > --- /dev/null > > +++ b/drivers/video/backlight/as3711_bl.c > > @@ -0,0 +1,379 @@ > > +/* > > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters > > + * > > + * Copyright (C) 2012 Renesas Electronics Corporation > > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the version 2 of the GNU General Public License as > > + * published by the Free Software Foundation > > + */ > > + > > +#include <linux/backlight.h> > > +#include <linux/delay.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/fb.h> > > +#include <linux/kernel.h> > > +#include <linux/mfd/as3711.h> > > +#include <linux/module.h> > > +#include <linux/regmap.h> > > +#include <linux/slab.h> > > + > > +enum as3711_bl_type { > > + AS3711_BL_SU1, > > + AS3711_BL_SU2, > > +}; > > + > > +struct as3711_bl_data { > > + bool powered; > > + const char *fb_name; > > + struct device *fb_dev; > > + enum as3711_bl_type type; > > + int brightness; > > + struct backlight_device *bl; > > +}; > > + > > +struct as3711_bl_supply { > > + struct as3711_bl_data su1; > > + struct as3711_bl_data su2; > > + const struct as3711_bl_pdata *pdata; > > + struct as3711 *as3711; > > +}; > > + > > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su) > > +{ > > + switch (su->type) { > > + case AS3711_BL_SU1: > > + return container_of(su, struct as3711_bl_supply, su1); > > + case AS3711_BL_SU2: > > + return container_of(su, struct as3711_bl_supply, su2); > > + } > > + return NULL; > > +} > > + > > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data, > > + unsigned int brightness) > > +{ > > + struct as3711_bl_supply *supply = to_supply(data); > > + struct as3711 *as3711 = supply->as3711; > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > + int ret = 0; > > + > > + /* Only all equal current values are supported */ > > + if (pdata->su2_auto_curr1) > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > > + brightness); > > + if (!ret && pdata->su2_auto_curr2) > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > > + brightness); > > + if (!ret && pdata->su2_auto_curr3) > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > > + brightness); > > + > > + return ret; > > +} > > + > > +static int as3711_set_brightness_v(struct as3711 *as3711, > > + unsigned int brightness, > > + unsigned int reg) > > +{ > > + if (brightness > 31) > > + return -EINVAL; > > + > > + return regmap_update_bits(as3711->regmap, reg, 0xf0, > > + brightness << 4); > > +} > > + > > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply) > > +{ > > + struct as3711 *as3711 = supply->as3711; > > + int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5, > > + 3, supply->pdata->su2_fbprot); > > + if (!ret) > > + ret = regmap_update_bits(as3711->regmap, > > + AS3711_STEPUP_CONTROL_2, 1, 0); > > + if (!ret) > > + ret = regmap_update_bits(as3711->regmap, > > + AS3711_STEPUP_CONTROL_2, 1, 1); > > + return ret; > > +} > > + > > +/* > > + * Someone with less fragile or less expensive hardware could try to simplify > > + * the brightness adjustment procedure. > > + */ > > +static int as3711_bl_update_status(struct backlight_device *bl) > > +{ > > + struct as3711_bl_data *data = bl_get_data(bl); > > + struct as3711_bl_supply *supply = to_supply(data); > > + struct as3711 *as3711 = supply->as3711; > > + int brightness = bl->props.brightness; > > + int ret = 0; > > + > > + dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n", > > + __func__, bl->props.brightness, bl->props.power, > > + bl->props.fb_blank, bl->props.state); > > + > > + if (bl->props.power != FB_BLANK_UNBLANK || > > + bl->props.fb_blank != FB_BLANK_UNBLANK || > > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > > + brightness = 0; > > + > > + if (data->type == AS3711_BL_SU1) { > > + ret = as3711_set_brightness_v(as3711, brightness, > > + AS3711_STEPUP_CONTROL_1); > > + } else { > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > + > > + switch (pdata->su2_feedback) { > > + case AS3711_SU2_VOLTAGE: > > + ret = as3711_set_brightness_v(as3711, brightness, > > + AS3711_STEPUP_CONTROL_2); > > + break; > > + case AS3711_SU2_CURR_AUTO: > > + ret = as3711_set_brightness_auto_i(data, brightness / 4); > > + if (ret < 0) > > + return ret; > > + if (brightness) { > > + ret = as3711_bl_su2_reset(supply); > > + if (ret < 0) > > + return ret; > > + udelay(500); > > + ret = as3711_set_brightness_auto_i(data, brightness); > > + } else { > > + ret = regmap_update_bits(as3711->regmap, > > + AS3711_STEPUP_CONTROL_2, 1, 0); > > + } > > + break; > > + /* Manual one current feedback pin below */ > > + case AS3711_SU2_CURR1: > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > > + brightness); > > + break; > > + case AS3711_SU2_CURR2: > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > > + brightness); > > + break; > > + case AS3711_SU2_CURR3: > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > > + brightness); > > + break; > > + default: > > + ret = -EINVAL; > > + } > > + } > > + if (!ret) > > + data->brightness = brightness; > > + > > + return ret; > > +} > > + > > +static int as3711_bl_get_brightness(struct backlight_device *bl) > > +{ > > + struct as3711_bl_data *data = bl_get_data(bl); > > + > > + return data->brightness; > > +} > > + > > +static const struct backlight_ops as3711_bl_ops = { > > + .update_status = as3711_bl_update_status, > > + .get_brightness = as3711_bl_get_brightness, > > +}; > > + > > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply) > > +{ > > + struct as3711 *as3711 = supply->as3711; > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > + u8 ctl = 0; > > + int ret; > > + > > + dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback); > > + > > + /* Turn SU2 off */ > > + ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0); > > + if (ret < 0) > > + return ret; > > + > > + switch (pdata->su2_feedback) { > > + case AS3711_SU2_VOLTAGE: > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0); > > + break; > > + case AS3711_SU2_CURR1: > > + ctl = 1; > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1); > > + break; > > + case AS3711_SU2_CURR2: > > + ctl = 4; > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2); > > + break; > > + case AS3711_SU2_CURR3: > > + ctl = 0x10; > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3); > > + break; > > + case AS3711_SU2_CURR_AUTO: > > + if (pdata->su2_auto_curr1) > > + ctl = 2; > > + if (pdata->su2_auto_curr2) > > + ctl |= 8; > > + if (pdata->su2_auto_curr3) > > + ctl |= 0x20; > > + ret = 0; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + if (!ret) > > + ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl); > > + > > + return ret; > > +} > > + > > +static int as3711_bl_register(struct platform_device *pdev, > > + unsigned int max_brightness, struct as3711_bl_data *su) > > +{ > > + struct backlight_properties props = {.type = BACKLIGHT_RAW,}; > > + struct backlight_device *bl; > > + > > + /* max tuning I = 31uA for voltage- and 38250uA for current-feedback */ > > + props.max_brightness = max_brightness; > > + > > + bl = backlight_device_register(su->type == AS3711_BL_SU1 ? > > + "as3711-su1" : "as3711-su2", > > + &pdev->dev, su, > > + &as3711_bl_ops, &props); > > + if (IS_ERR(bl)) { > > + dev_err(&pdev->dev, "failed to register backlight\n"); > > + return PTR_ERR(bl); > > + } > > + > > + bl->props.brightness = props.max_brightness; > > + > > + backlight_update_status(bl); > > + > > + su->bl = bl; > > + > > + return 0; > > +} > > + > > +static int as3711_backlight_probe(struct platform_device *pdev) > > +{ > > + struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev); > > + struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent); > > + struct as3711_bl_supply *supply; > > + struct as3711_bl_data *su; > > + unsigned int max_brightness; > > + int ret; > > + > > + if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) { > > + dev_err(&pdev->dev, "No platform data, exiting...\n"); > > + return -ENODEV; > > + } > > + > > + /* > > + * Due to possible hardware damage I chose to block all modes, > > + * unsupported on my hardware. Anyone, wishing to use any of those modes > > + * will have to first review the code, then activate and test it. > > + */ > > + if (pdata->su1_fb || > > + pdata->su2_fbprot != AS3711_SU2_GPIO4 || > > + pdata->su2_feedback != AS3711_SU2_CURR_AUTO) { > > + dev_warn(&pdev->dev, > > + "Attention! An untested mode has been chosen!\n" > > + "Please, review the code, enable, test, and report success:-)\n"); > > + return -EINVAL; > > + } > > + > > + supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL); > > + if (!supply) > > + return -ENOMEM; > > + > > + supply->as3711 = as3711; > > + supply->pdata = pdata; > > + > > + if (pdata->su1_fb) { > > + su = &supply->su1; > > + su->fb_name = pdata->su1_fb; > > + su->type = AS3711_BL_SU1; > > + > > + max_brightness = min(pdata->su1_max_uA, 31); > > + ret = as3711_bl_register(pdev, max_brightness, su); > > + if (ret < 0) > > + return ret; > > + } > > + > > + if (pdata->su2_fb) { > > + su = &supply->su2; > > + su->fb_name = pdata->su2_fb; > > + su->type = AS3711_BL_SU2; > > + > > + switch (pdata->su2_fbprot) { > > + case AS3711_SU2_GPIO2: > > + case AS3711_SU2_GPIO3: > > + case AS3711_SU2_GPIO4: > > + case AS3711_SU2_LX_SD4: > > + break; > > + default: > > + ret = -EINVAL; > > + goto esu2; > > + } > > + > > + switch (pdata->su2_feedback) { > > + case AS3711_SU2_VOLTAGE: > > + max_brightness = min(pdata->su2_max_uA, 31); > > + break; > > + case AS3711_SU2_CURR1: > > + case AS3711_SU2_CURR2: > > + case AS3711_SU2_CURR3: > > + case AS3711_SU2_CURR_AUTO: > > + max_brightness = min(pdata->su2_max_uA / 150, 255); > > + break; > > + default: > > + ret = -EINVAL; > > + goto esu2; > > + } > > + > > + ret = as3711_bl_init_su2(supply); > > + if (ret < 0) > > + return ret; > > + > > + ret = as3711_bl_register(pdev, max_brightness, su); > > + if (ret < 0) > > + goto esu2; > > + } > > + > > + platform_set_drvdata(pdev, supply); > > + > > + return 0; > > + > > +esu2: > > + backlight_device_unregister(supply->su1.bl); > > + return ret; > > +} > > + > > +static int as3711_backlight_remove(struct platform_device *pdev) > > +{ > > + struct as3711_bl_supply *supply = platform_get_drvdata(pdev); > > + > > + backlight_device_unregister(supply->su1.bl); > > + backlight_device_unregister(supply->su2.bl); > > + > > + return 0; > > +} > > + > > +static struct platform_driver as3711_backlight_driver = { > > + .driver = { > > + .name = "as3711-backlight", > > + .owner = THIS_MODULE, > > + }, > > + .probe = as3711_backlight_probe, > > + .remove = as3711_backlight_remove, > > +}; > > + > > +module_platform_driver(as3711_backlight_driver); > > + > > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs"); > > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de"); > > +MODULE_LICENSE("GPL"); > > +MODULE_ALIAS("platform:as3711-backlight"); > > -- > > 1.7.2.5 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver 2013-01-25 7:48 ` Guennadi Liakhovetski @ 2013-01-28 0:53 ` Jingoo Han 2013-02-05 9:08 ` Guennadi Liakhovetski 0 siblings, 1 reply; 4+ messages in thread From: Jingoo Han @ 2013-01-28 0:53 UTC (permalink / raw) To: 'Guennadi Liakhovetski' Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh, 'Magnus Damm', 'Richard Purdie', linux-kernel, 'Jingoo Han' On Friday, January 25, 2013 4:49 PM, Guennadi Liakhovetski wrote > > Hi Jingoo Han > > On Fri, 25 Jan 2013, Jingoo Han wrote: > > > On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote > > > > > > This is an initial commit of a backlight driver, using step-up DCDC power > > > supplies on AS3711 PMIC. Only one mode has actually been tested, several > > > further modes have been implemented "dry," but disabled to avoid accidental > > > hardware damage. Anyone wishing to use any of those modes will have to > > > modify the driver. > > > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > > > CC'ed Andrew Morton. > > > > Hi Guennadi Liakhovetski, > > > > I have reviewed this patch with AS3711 datasheet. > > I cannot find any problems. It looks good. > > Thanks for the review. > > > However, some hardcoded numbers need to be changed > > to the bit definitions. > > Which specific hardcoded values do you mean? A while ago in a discussion > on one of kernel mailing lists a conclusion has been made, that macros do > not always improve code readability. E.g. in a statement like this > > + case AS3711_SU2_CURR_AUTO: > + if (pdata->su2_auto_curr1) > + ctl = 2; > + if (pdata->su2_auto_curr2) > + ctl |= 8; > + if (pdata->su2_auto_curr3) > + ctl |= 0x20; > > making it > > + case AS3711_SU2_CURR_AUTO: > + if (pdata->su2_auto_curr1) > + ctl = SU2_AUTO_CURR1; > > would hardly make it more readable. IMHO it is already pretty clear, that > bit 1 enables the current-1 sink. As long as these fields are only used at > one location in the driver (and they are), using a macro and defining it > elsewhere only makes it harder to see actual values. Speaking of this, a > comment like I don't think so. Some people feel that it is not clear a bit. Of course, I know what you mean. Also, your comment is reasonable. However, personally, I prefer the latter. Because, I think that the meaning of bits is more important than actual bits. In the latter case, we can notice the meaning of bits more easily. Best regards, Jingoo Han > > /* > * Select, which current sinks shall be used for automatic > * feedback selection > */ > > would help much more than any macro names. But as it stands, I think the > current version is also possible to understand :-) If desired, however, > comments can be added in an incremental patch > > Thanks > Guennadi > > > Acked-by: Jingoo Han <jg1.han@samsung.com> > > > > > > Best regards, > > Jingoo Han > > > > > --- > > > > > > Tested on sh73a0-based kzm9g board. As the commit message says, only one > > > mode has been tested and is enabled. That mode copies the sample code from > > > the manufacturer. Deviations from that code proved to be fatal for the > > > hardware... > > > > > > drivers/video/backlight/Kconfig | 7 + > > > drivers/video/backlight/Makefile | 1 + > > > drivers/video/backlight/as3711_bl.c | 379 +++++++++++++++++++++++++++++++++++ > > > 3 files changed, 387 insertions(+), 0 deletions(-) > > > create mode 100644 drivers/video/backlight/as3711_bl.c > > > > > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > > > index 765a945..6ef0ede 100644 > > > --- a/drivers/video/backlight/Kconfig > > > +++ b/drivers/video/backlight/Kconfig > > > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217 > > > If you have a Texas Instruments TPS65217 say Y to enable the > > > backlight driver. > > > > > > +config BACKLIGHT_AS3711 > > > + tristate "AS3711 Backlight" > > > + depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711 > > > + help > > > + If you have an Austrian Microsystems AS3711 say Y to enable the > > > + backlight driver. > > > + > > > endif # BACKLIGHT_CLASS_DEVICE > > > > > > endif # BACKLIGHT_LCD_SUPPORT > > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > > > index e7ce729..d3e188f 100644 > > > --- a/drivers/video/backlight/Makefile > > > +++ b/drivers/video/backlight/Makefile > > > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o > > > obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o > > > obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o > > > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o > > > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o > > > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c > > > new file mode 100644 > > > index 0000000..c6bc65d > > > --- /dev/null > > > +++ b/drivers/video/backlight/as3711_bl.c > > > @@ -0,0 +1,379 @@ > > > +/* > > > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters > > > + * > > > + * Copyright (C) 2012 Renesas Electronics Corporation > > > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de> > > > + * > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the version 2 of the GNU General Public License as > > > + * published by the Free Software Foundation > > > + */ > > > + > > > +#include <linux/backlight.h> > > > +#include <linux/delay.h> > > > +#include <linux/device.h> > > > +#include <linux/err.h> > > > +#include <linux/fb.h> > > > +#include <linux/kernel.h> > > > +#include <linux/mfd/as3711.h> > > > +#include <linux/module.h> > > > +#include <linux/regmap.h> > > > +#include <linux/slab.h> > > > + > > > +enum as3711_bl_type { > > > + AS3711_BL_SU1, > > > + AS3711_BL_SU2, > > > +}; > > > + > > > +struct as3711_bl_data { > > > + bool powered; > > > + const char *fb_name; > > > + struct device *fb_dev; > > > + enum as3711_bl_type type; > > > + int brightness; > > > + struct backlight_device *bl; > > > +}; > > > + > > > +struct as3711_bl_supply { > > > + struct as3711_bl_data su1; > > > + struct as3711_bl_data su2; > > > + const struct as3711_bl_pdata *pdata; > > > + struct as3711 *as3711; > > > +}; > > > + > > > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su) > > > +{ > > > + switch (su->type) { > > > + case AS3711_BL_SU1: > > > + return container_of(su, struct as3711_bl_supply, su1); > > > + case AS3711_BL_SU2: > > > + return container_of(su, struct as3711_bl_supply, su2); > > > + } > > > + return NULL; > > > +} > > > + > > > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data, > > > + unsigned int brightness) > > > +{ > > > + struct as3711_bl_supply *supply = to_supply(data); > > > + struct as3711 *as3711 = supply->as3711; > > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > > + int ret = 0; > > > + > > > + /* Only all equal current values are supported */ > > > + if (pdata->su2_auto_curr1) > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > > > + brightness); > > > + if (!ret && pdata->su2_auto_curr2) > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > > > + brightness); > > > + if (!ret && pdata->su2_auto_curr3) > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > > > + brightness); > > > + > > > + return ret; > > > +} > > > + > > > +static int as3711_set_brightness_v(struct as3711 *as3711, > > > + unsigned int brightness, > > > + unsigned int reg) > > > +{ > > > + if (brightness > 31) > > > + return -EINVAL; > > > + > > > + return regmap_update_bits(as3711->regmap, reg, 0xf0, > > > + brightness << 4); > > > +} > > > + > > > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply) > > > +{ > > > + struct as3711 *as3711 = supply->as3711; > > > + int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5, > > > + 3, supply->pdata->su2_fbprot); > > > + if (!ret) > > > + ret = regmap_update_bits(as3711->regmap, > > > + AS3711_STEPUP_CONTROL_2, 1, 0); > > > + if (!ret) > > > + ret = regmap_update_bits(as3711->regmap, > > > + AS3711_STEPUP_CONTROL_2, 1, 1); > > > + return ret; > > > +} > > > + > > > +/* > > > + * Someone with less fragile or less expensive hardware could try to simplify > > > + * the brightness adjustment procedure. > > > + */ > > > +static int as3711_bl_update_status(struct backlight_device *bl) > > > +{ > > > + struct as3711_bl_data *data = bl_get_data(bl); > > > + struct as3711_bl_supply *supply = to_supply(data); > > > + struct as3711 *as3711 = supply->as3711; > > > + int brightness = bl->props.brightness; > > > + int ret = 0; > > > + > > > + dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n", > > > + __func__, bl->props.brightness, bl->props.power, > > > + bl->props.fb_blank, bl->props.state); > > > + > > > + if (bl->props.power != FB_BLANK_UNBLANK || > > > + bl->props.fb_blank != FB_BLANK_UNBLANK || > > > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > > > + brightness = 0; > > > + > > > + if (data->type == AS3711_BL_SU1) { > > > + ret = as3711_set_brightness_v(as3711, brightness, > > > + AS3711_STEPUP_CONTROL_1); > > > + } else { > > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > > + > > > + switch (pdata->su2_feedback) { > > > + case AS3711_SU2_VOLTAGE: > > > + ret = as3711_set_brightness_v(as3711, brightness, > > > + AS3711_STEPUP_CONTROL_2); > > > + break; > > > + case AS3711_SU2_CURR_AUTO: > > > + ret = as3711_set_brightness_auto_i(data, brightness / 4); > > > + if (ret < 0) > > > + return ret; > > > + if (brightness) { > > > + ret = as3711_bl_su2_reset(supply); > > > + if (ret < 0) > > > + return ret; > > > + udelay(500); > > > + ret = as3711_set_brightness_auto_i(data, brightness); > > > + } else { > > > + ret = regmap_update_bits(as3711->regmap, > > > + AS3711_STEPUP_CONTROL_2, 1, 0); > > > + } > > > + break; > > > + /* Manual one current feedback pin below */ > > > + case AS3711_SU2_CURR1: > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > > > + brightness); > > > + break; > > > + case AS3711_SU2_CURR2: > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > > > + brightness); > > > + break; > > > + case AS3711_SU2_CURR3: > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > > > + brightness); > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + } > > > + } > > > + if (!ret) > > > + data->brightness = brightness; > > > + > > > + return ret; > > > +} > > > + > > > +static int as3711_bl_get_brightness(struct backlight_device *bl) > > > +{ > > > + struct as3711_bl_data *data = bl_get_data(bl); > > > + > > > + return data->brightness; > > > +} > > > + > > > +static const struct backlight_ops as3711_bl_ops = { > > > + .update_status = as3711_bl_update_status, > > > + .get_brightness = as3711_bl_get_brightness, > > > +}; > > > + > > > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply) > > > +{ > > > + struct as3711 *as3711 = supply->as3711; > > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > > + u8 ctl = 0; > > > + int ret; > > > + > > > + dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback); > > > + > > > + /* Turn SU2 off */ > > > + ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0); > > > + if (ret < 0) > > > + return ret; > > > + > > > + switch (pdata->su2_feedback) { > > > + case AS3711_SU2_VOLTAGE: > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0); > > > + break; > > > + case AS3711_SU2_CURR1: > > > + ctl = 1; > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1); > > > + break; > > > + case AS3711_SU2_CURR2: > > > + ctl = 4; > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2); > > > + break; > > > + case AS3711_SU2_CURR3: > > > + ctl = 0x10; > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3); > > > + break; > > > + case AS3711_SU2_CURR_AUTO: > > > + if (pdata->su2_auto_curr1) > > > + ctl = 2; > > > + if (pdata->su2_auto_curr2) > > > + ctl |= 8; > > > + if (pdata->su2_auto_curr3) > > > + ctl |= 0x20; > > > + ret = 0; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + if (!ret) > > > + ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl); > > > + > > > + return ret; > > > +} > > > + > > > +static int as3711_bl_register(struct platform_device *pdev, > > > + unsigned int max_brightness, struct as3711_bl_data *su) > > > +{ > > > + struct backlight_properties props = {.type = BACKLIGHT_RAW,}; > > > + struct backlight_device *bl; > > > + > > > + /* max tuning I = 31uA for voltage- and 38250uA for current-feedback */ > > > + props.max_brightness = max_brightness; > > > + > > > + bl = backlight_device_register(su->type == AS3711_BL_SU1 ? > > > + "as3711-su1" : "as3711-su2", > > > + &pdev->dev, su, > > > + &as3711_bl_ops, &props); > > > + if (IS_ERR(bl)) { > > > + dev_err(&pdev->dev, "failed to register backlight\n"); > > > + return PTR_ERR(bl); > > > + } > > > + > > > + bl->props.brightness = props.max_brightness; > > > + > > > + backlight_update_status(bl); > > > + > > > + su->bl = bl; > > > + > > > + return 0; > > > +} > > > + > > > +static int as3711_backlight_probe(struct platform_device *pdev) > > > +{ > > > + struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev); > > > + struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent); > > > + struct as3711_bl_supply *supply; > > > + struct as3711_bl_data *su; > > > + unsigned int max_brightness; > > > + int ret; > > > + > > > + if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) { > > > + dev_err(&pdev->dev, "No platform data, exiting...\n"); > > > + return -ENODEV; > > > + } > > > + > > > + /* > > > + * Due to possible hardware damage I chose to block all modes, > > > + * unsupported on my hardware. Anyone, wishing to use any of those modes > > > + * will have to first review the code, then activate and test it. > > > + */ > > > + if (pdata->su1_fb || > > > + pdata->su2_fbprot != AS3711_SU2_GPIO4 || > > > + pdata->su2_feedback != AS3711_SU2_CURR_AUTO) { > > > + dev_warn(&pdev->dev, > > > + "Attention! An untested mode has been chosen!\n" > > > + "Please, review the code, enable, test, and report success:-)\n"); > > > + return -EINVAL; > > > + } > > > + > > > + supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL); > > > + if (!supply) > > > + return -ENOMEM; > > > + > > > + supply->as3711 = as3711; > > > + supply->pdata = pdata; > > > + > > > + if (pdata->su1_fb) { > > > + su = &supply->su1; > > > + su->fb_name = pdata->su1_fb; > > > + su->type = AS3711_BL_SU1; > > > + > > > + max_brightness = min(pdata->su1_max_uA, 31); > > > + ret = as3711_bl_register(pdev, max_brightness, su); > > > + if (ret < 0) > > > + return ret; > > > + } > > > + > > > + if (pdata->su2_fb) { > > > + su = &supply->su2; > > > + su->fb_name = pdata->su2_fb; > > > + su->type = AS3711_BL_SU2; > > > + > > > + switch (pdata->su2_fbprot) { > > > + case AS3711_SU2_GPIO2: > > > + case AS3711_SU2_GPIO3: > > > + case AS3711_SU2_GPIO4: > > > + case AS3711_SU2_LX_SD4: > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + goto esu2; > > > + } > > > + > > > + switch (pdata->su2_feedback) { > > > + case AS3711_SU2_VOLTAGE: > > > + max_brightness = min(pdata->su2_max_uA, 31); > > > + break; > > > + case AS3711_SU2_CURR1: > > > + case AS3711_SU2_CURR2: > > > + case AS3711_SU2_CURR3: > > > + case AS3711_SU2_CURR_AUTO: > > > + max_brightness = min(pdata->su2_max_uA / 150, 255); > > > + break; > > > + default: > > > + ret = -EINVAL; > > > + goto esu2; > > > + } > > > + > > > + ret = as3711_bl_init_su2(supply); > > > + if (ret < 0) > > > + return ret; > > > + > > > + ret = as3711_bl_register(pdev, max_brightness, su); > > > + if (ret < 0) > > > + goto esu2; > > > + } > > > + > > > + platform_set_drvdata(pdev, supply); > > > + > > > + return 0; > > > + > > > +esu2: > > > + backlight_device_unregister(supply->su1.bl); > > > + return ret; > > > +} > > > + > > > +static int as3711_backlight_remove(struct platform_device *pdev) > > > +{ > > > + struct as3711_bl_supply *supply = platform_get_drvdata(pdev); > > > + > > > + backlight_device_unregister(supply->su1.bl); > > > + backlight_device_unregister(supply->su2.bl); > > > + > > > + return 0; > > > +} > > > + > > > +static struct platform_driver as3711_backlight_driver = { > > > + .driver = { > > > + .name = "as3711-backlight", > > > + .owner = THIS_MODULE, > > > + }, > > > + .probe = as3711_backlight_probe, > > > + .remove = as3711_backlight_remove, > > > +}; > > > + > > > +module_platform_driver(as3711_backlight_driver); > > > + > > > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs"); > > > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de"); > > > +MODULE_LICENSE("GPL"); > > > +MODULE_ALIAS("platform:as3711-backlight"); > > > -- > > > 1.7.2.5 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > --- > Guennadi Liakhovetski, Ph.D. > Freelance Open-Source Software Developer > http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver 2013-01-28 0:53 ` Jingoo Han @ 2013-02-05 9:08 ` Guennadi Liakhovetski 0 siblings, 0 replies; 4+ messages in thread From: Guennadi Liakhovetski @ 2013-02-05 9:08 UTC (permalink / raw) To: Jingoo Han Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh, 'Magnus Damm', 'Richard Purdie', linux-kernel Hi Richard Could you tell us your opinion on this: On Mon, 28 Jan 2013, Jingoo Han wrote: > On Friday, January 25, 2013 4:49 PM, Guennadi Liakhovetski wrote > > > > Hi Jingoo Han > > > > On Fri, 25 Jan 2013, Jingoo Han wrote: > > > > > On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote > > > > > > > > This is an initial commit of a backlight driver, using step-up DCDC power > > > > supplies on AS3711 PMIC. Only one mode has actually been tested, several > > > > further modes have been implemented "dry," but disabled to avoid accidental > > > > hardware damage. Anyone wishing to use any of those modes will have to > > > > modify the driver. > > > > > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > > > > > > CC'ed Andrew Morton. > > > > > > Hi Guennadi Liakhovetski, > > > > > > I have reviewed this patch with AS3711 datasheet. > > > I cannot find any problems. It looks good. > > > > Thanks for the review. > > > > > However, some hardcoded numbers need to be changed > > > to the bit definitions. > > > > Which specific hardcoded values do you mean? A while ago in a discussion > > on one of kernel mailing lists a conclusion has been made, that macros do > > not always improve code readability. E.g. in a statement like this > > > > + case AS3711_SU2_CURR_AUTO: > > + if (pdata->su2_auto_curr1) > > + ctl = 2; > > + if (pdata->su2_auto_curr2) > > + ctl |= 8; > > + if (pdata->su2_auto_curr3) > > + ctl |= 0x20; > > > > making it > > > > + case AS3711_SU2_CURR_AUTO: > > + if (pdata->su2_auto_curr1) > > + ctl = SU2_AUTO_CURR1; > > > > would hardly make it more readable. IMHO it is already pretty clear, that > > bit 1 enables the current-1 sink. As long as these fields are only used at > > one location in the driver (and they are), using a macro and defining it > > elsewhere only makes it harder to see actual values. Speaking of this, a > > comment like > > I don't think so. Some people feel that it is not clear a bit. > Of course, I know what you mean. > Also, your comment is reasonable. > However, personally, I prefer the latter. > Because, I think that the meaning of bits is more important than > actual bits. In the latter case, we can notice the meaning of bits > more easily. Do you also find preferable to use symbolic names for every single bit, occurring in a driver, or you agree, that excessive use of such macros can only needlessly clutter the code? Thanks Guennadi > Best regards, > Jingoo Han > > > > > /* > > * Select, which current sinks shall be used for automatic > > * feedback selection > > */ > > > > would help much more than any macro names. But as it stands, I think the > > current version is also possible to understand :-) If desired, however, > > comments can be added in an incremental patch > > > > > Thanks > > Guennadi > > > > > Acked-by: Jingoo Han <jg1.han@samsung.com> > > > > > > > > > Best regards, > > > Jingoo Han > > > > > > > --- > > > > > > > > Tested on sh73a0-based kzm9g board. As the commit message says, only one > > > > mode has been tested and is enabled. That mode copies the sample code from > > > > the manufacturer. Deviations from that code proved to be fatal for the > > > > hardware... > > > > > > > > drivers/video/backlight/Kconfig | 7 + > > > > drivers/video/backlight/Makefile | 1 + > > > > drivers/video/backlight/as3711_bl.c | 379 +++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 387 insertions(+), 0 deletions(-) > > > > create mode 100644 drivers/video/backlight/as3711_bl.c > > > > > > > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > > > > index 765a945..6ef0ede 100644 > > > > --- a/drivers/video/backlight/Kconfig > > > > +++ b/drivers/video/backlight/Kconfig > > > > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217 > > > > If you have a Texas Instruments TPS65217 say Y to enable the > > > > backlight driver. > > > > > > > > +config BACKLIGHT_AS3711 > > > > + tristate "AS3711 Backlight" > > > > + depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711 > > > > + help > > > > + If you have an Austrian Microsystems AS3711 say Y to enable the > > > > + backlight driver. > > > > + > > > > endif # BACKLIGHT_CLASS_DEVICE > > > > > > > > endif # BACKLIGHT_LCD_SUPPORT > > > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > > > > index e7ce729..d3e188f 100644 > > > > --- a/drivers/video/backlight/Makefile > > > > +++ b/drivers/video/backlight/Makefile > > > > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o > > > > obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o > > > > obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o > > > > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o > > > > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o > > > > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c > > > > new file mode 100644 > > > > index 0000000..c6bc65d > > > > --- /dev/null > > > > +++ b/drivers/video/backlight/as3711_bl.c > > > > @@ -0,0 +1,379 @@ > > > > +/* > > > > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters > > > > + * > > > > + * Copyright (C) 2012 Renesas Electronics Corporation > > > > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de> > > > > + * > > > > + * This program is free software; you can redistribute it and/or modify > > > > + * it under the terms of the version 2 of the GNU General Public License as > > > > + * published by the Free Software Foundation > > > > + */ > > > > + > > > > +#include <linux/backlight.h> > > > > +#include <linux/delay.h> > > > > +#include <linux/device.h> > > > > +#include <linux/err.h> > > > > +#include <linux/fb.h> > > > > +#include <linux/kernel.h> > > > > +#include <linux/mfd/as3711.h> > > > > +#include <linux/module.h> > > > > +#include <linux/regmap.h> > > > > +#include <linux/slab.h> > > > > + > > > > +enum as3711_bl_type { > > > > + AS3711_BL_SU1, > > > > + AS3711_BL_SU2, > > > > +}; > > > > + > > > > +struct as3711_bl_data { > > > > + bool powered; > > > > + const char *fb_name; > > > > + struct device *fb_dev; > > > > + enum as3711_bl_type type; > > > > + int brightness; > > > > + struct backlight_device *bl; > > > > +}; > > > > + > > > > +struct as3711_bl_supply { > > > > + struct as3711_bl_data su1; > > > > + struct as3711_bl_data su2; > > > > + const struct as3711_bl_pdata *pdata; > > > > + struct as3711 *as3711; > > > > +}; > > > > + > > > > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su) > > > > +{ > > > > + switch (su->type) { > > > > + case AS3711_BL_SU1: > > > > + return container_of(su, struct as3711_bl_supply, su1); > > > > + case AS3711_BL_SU2: > > > > + return container_of(su, struct as3711_bl_supply, su2); > > > > + } > > > > + return NULL; > > > > +} > > > > + > > > > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data, > > > > + unsigned int brightness) > > > > +{ > > > > + struct as3711_bl_supply *supply = to_supply(data); > > > > + struct as3711 *as3711 = supply->as3711; > > > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > > > + int ret = 0; > > > > + > > > > + /* Only all equal current values are supported */ > > > > + if (pdata->su2_auto_curr1) > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > > > > + brightness); > > > > + if (!ret && pdata->su2_auto_curr2) > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > > > > + brightness); > > > > + if (!ret && pdata->su2_auto_curr3) > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > > > > + brightness); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int as3711_set_brightness_v(struct as3711 *as3711, > > > > + unsigned int brightness, > > > > + unsigned int reg) > > > > +{ > > > > + if (brightness > 31) > > > > + return -EINVAL; > > > > + > > > > + return regmap_update_bits(as3711->regmap, reg, 0xf0, > > > > + brightness << 4); > > > > +} > > > > + > > > > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply) > > > > +{ > > > > + struct as3711 *as3711 = supply->as3711; > > > > + int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5, > > > > + 3, supply->pdata->su2_fbprot); > > > > + if (!ret) > > > > + ret = regmap_update_bits(as3711->regmap, > > > > + AS3711_STEPUP_CONTROL_2, 1, 0); > > > > + if (!ret) > > > > + ret = regmap_update_bits(as3711->regmap, > > > > + AS3711_STEPUP_CONTROL_2, 1, 1); > > > > + return ret; > > > > +} > > > > + > > > > +/* > > > > + * Someone with less fragile or less expensive hardware could try to simplify > > > > + * the brightness adjustment procedure. > > > > + */ > > > > +static int as3711_bl_update_status(struct backlight_device *bl) > > > > +{ > > > > + struct as3711_bl_data *data = bl_get_data(bl); > > > > + struct as3711_bl_supply *supply = to_supply(data); > > > > + struct as3711 *as3711 = supply->as3711; > > > > + int brightness = bl->props.brightness; > > > > + int ret = 0; > > > > + > > > > + dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n", > > > > + __func__, bl->props.brightness, bl->props.power, > > > > + bl->props.fb_blank, bl->props.state); > > > > + > > > > + if (bl->props.power != FB_BLANK_UNBLANK || > > > > + bl->props.fb_blank != FB_BLANK_UNBLANK || > > > > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) > > > > + brightness = 0; > > > > + > > > > + if (data->type == AS3711_BL_SU1) { > > > > + ret = as3711_set_brightness_v(as3711, brightness, > > > > + AS3711_STEPUP_CONTROL_1); > > > > + } else { > > > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > > > + > > > > + switch (pdata->su2_feedback) { > > > > + case AS3711_SU2_VOLTAGE: > > > > + ret = as3711_set_brightness_v(as3711, brightness, > > > > + AS3711_STEPUP_CONTROL_2); > > > > + break; > > > > + case AS3711_SU2_CURR_AUTO: > > > > + ret = as3711_set_brightness_auto_i(data, brightness / 4); > > > > + if (ret < 0) > > > > + return ret; > > > > + if (brightness) { > > > > + ret = as3711_bl_su2_reset(supply); > > > > + if (ret < 0) > > > > + return ret; > > > > + udelay(500); > > > > + ret = as3711_set_brightness_auto_i(data, brightness); > > > > + } else { > > > > + ret = regmap_update_bits(as3711->regmap, > > > > + AS3711_STEPUP_CONTROL_2, 1, 0); > > > > + } > > > > + break; > > > > + /* Manual one current feedback pin below */ > > > > + case AS3711_SU2_CURR1: > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE, > > > > + brightness); > > > > + break; > > > > + case AS3711_SU2_CURR2: > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE, > > > > + brightness); > > > > + break; > > > > + case AS3711_SU2_CURR3: > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE, > > > > + brightness); > > > > + break; > > > > + default: > > > > + ret = -EINVAL; > > > > + } > > > > + } > > > > + if (!ret) > > > > + data->brightness = brightness; > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int as3711_bl_get_brightness(struct backlight_device *bl) > > > > +{ > > > > + struct as3711_bl_data *data = bl_get_data(bl); > > > > + > > > > + return data->brightness; > > > > +} > > > > + > > > > +static const struct backlight_ops as3711_bl_ops = { > > > > + .update_status = as3711_bl_update_status, > > > > + .get_brightness = as3711_bl_get_brightness, > > > > +}; > > > > + > > > > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply) > > > > +{ > > > > + struct as3711 *as3711 = supply->as3711; > > > > + const struct as3711_bl_pdata *pdata = supply->pdata; > > > > + u8 ctl = 0; > > > > + int ret; > > > > + > > > > + dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback); > > > > + > > > > + /* Turn SU2 off */ > > > > + ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + switch (pdata->su2_feedback) { > > > > + case AS3711_SU2_VOLTAGE: > > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0); > > > > + break; > > > > + case AS3711_SU2_CURR1: > > > > + ctl = 1; > > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1); > > > > + break; > > > > + case AS3711_SU2_CURR2: > > > > + ctl = 4; > > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2); > > > > + break; > > > > + case AS3711_SU2_CURR3: > > > > + ctl = 0x10; > > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3); > > > > + break; > > > > + case AS3711_SU2_CURR_AUTO: > > > > + if (pdata->su2_auto_curr1) > > > > + ctl = 2; > > > > + if (pdata->su2_auto_curr2) > > > > + ctl |= 8; > > > > + if (pdata->su2_auto_curr3) > > > > + ctl |= 0x20; > > > > + ret = 0; > > > > + break; > > > > + default: > > > > + return -EINVAL; > > > > + } > > > > + > > > > + if (!ret) > > > > + ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int as3711_bl_register(struct platform_device *pdev, > > > > + unsigned int max_brightness, struct as3711_bl_data *su) > > > > +{ > > > > + struct backlight_properties props = {.type = BACKLIGHT_RAW,}; > > > > + struct backlight_device *bl; > > > > + > > > > + /* max tuning I = 31uA for voltage- and 38250uA for current-feedback */ > > > > + props.max_brightness = max_brightness; > > > > + > > > > + bl = backlight_device_register(su->type == AS3711_BL_SU1 ? > > > > + "as3711-su1" : "as3711-su2", > > > > + &pdev->dev, su, > > > > + &as3711_bl_ops, &props); > > > > + if (IS_ERR(bl)) { > > > > + dev_err(&pdev->dev, "failed to register backlight\n"); > > > > + return PTR_ERR(bl); > > > > + } > > > > + > > > > + bl->props.brightness = props.max_brightness; > > > > + > > > > + backlight_update_status(bl); > > > > + > > > > + su->bl = bl; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int as3711_backlight_probe(struct platform_device *pdev) > > > > +{ > > > > + struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev); > > > > + struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent); > > > > + struct as3711_bl_supply *supply; > > > > + struct as3711_bl_data *su; > > > > + unsigned int max_brightness; > > > > + int ret; > > > > + > > > > + if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) { > > > > + dev_err(&pdev->dev, "No platform data, exiting...\n"); > > > > + return -ENODEV; > > > > + } > > > > + > > > > + /* > > > > + * Due to possible hardware damage I chose to block all modes, > > > > + * unsupported on my hardware. Anyone, wishing to use any of those modes > > > > + * will have to first review the code, then activate and test it. > > > > + */ > > > > + if (pdata->su1_fb || > > > > + pdata->su2_fbprot != AS3711_SU2_GPIO4 || > > > > + pdata->su2_feedback != AS3711_SU2_CURR_AUTO) { > > > > + dev_warn(&pdev->dev, > > > > + "Attention! An untested mode has been chosen!\n" > > > > + "Please, review the code, enable, test, and report success:-)\n"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL); > > > > + if (!supply) > > > > + return -ENOMEM; > > > > + > > > > + supply->as3711 = as3711; > > > > + supply->pdata = pdata; > > > > + > > > > + if (pdata->su1_fb) { > > > > + su = &supply->su1; > > > > + su->fb_name = pdata->su1_fb; > > > > + su->type = AS3711_BL_SU1; > > > > + > > > > + max_brightness = min(pdata->su1_max_uA, 31); > > > > + ret = as3711_bl_register(pdev, max_brightness, su); > > > > + if (ret < 0) > > > > + return ret; > > > > + } > > > > + > > > > + if (pdata->su2_fb) { > > > > + su = &supply->su2; > > > > + su->fb_name = pdata->su2_fb; > > > > + su->type = AS3711_BL_SU2; > > > > + > > > > + switch (pdata->su2_fbprot) { > > > > + case AS3711_SU2_GPIO2: > > > > + case AS3711_SU2_GPIO3: > > > > + case AS3711_SU2_GPIO4: > > > > + case AS3711_SU2_LX_SD4: > > > > + break; > > > > + default: > > > > + ret = -EINVAL; > > > > + goto esu2; > > > > + } > > > > + > > > > + switch (pdata->su2_feedback) { > > > > + case AS3711_SU2_VOLTAGE: > > > > + max_brightness = min(pdata->su2_max_uA, 31); > > > > + break; > > > > + case AS3711_SU2_CURR1: > > > > + case AS3711_SU2_CURR2: > > > > + case AS3711_SU2_CURR3: > > > > + case AS3711_SU2_CURR_AUTO: > > > > + max_brightness = min(pdata->su2_max_uA / 150, 255); > > > > + break; > > > > + default: > > > > + ret = -EINVAL; > > > > + goto esu2; > > > > + } > > > > + > > > > + ret = as3711_bl_init_su2(supply); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + ret = as3711_bl_register(pdev, max_brightness, su); > > > > + if (ret < 0) > > > > + goto esu2; > > > > + } > > > > + > > > > + platform_set_drvdata(pdev, supply); > > > > + > > > > + return 0; > > > > + > > > > +esu2: > > > > + backlight_device_unregister(supply->su1.bl); > > > > + return ret; > > > > +} > > > > + > > > > +static int as3711_backlight_remove(struct platform_device *pdev) > > > > +{ > > > > + struct as3711_bl_supply *supply = platform_get_drvdata(pdev); > > > > + > > > > + backlight_device_unregister(supply->su1.bl); > > > > + backlight_device_unregister(supply->su2.bl); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static struct platform_driver as3711_backlight_driver = { > > > > + .driver = { > > > > + .name = "as3711-backlight", > > > > + .owner = THIS_MODULE, > > > > + }, > > > > + .probe = as3711_backlight_probe, > > > > + .remove = as3711_backlight_remove, > > > > +}; > > > > + > > > > +module_platform_driver(as3711_backlight_driver); > > > > + > > > > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs"); > > > > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de"); > > > > +MODULE_LICENSE("GPL"); > > > > +MODULE_ALIAS("platform:as3711-backlight"); > > > > -- > > > > 1.7.2.5 > > > > > > > > -- > > > > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > > > > the body of a message to majordomo@vger.kernel.org > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > > > --- > > Guennadi Liakhovetski, Ph.D. > > Freelance Open-Source Software Developer > > http://www.open-technology.de/ > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-02-05 9:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <Pine.LNX.4.64.1301081846020.1794@axis700.grange>
2013-01-25 5:20 ` [PATCH] backlight: add an AS3711 PMIC backlight driver Jingoo Han
2013-01-25 7:48 ` Guennadi Liakhovetski
2013-01-28 0:53 ` Jingoo Han
2013-02-05 9:08 ` Guennadi Liakhovetski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome