[PATCH 1/3] leds: leds-mc13783: Prepare driver to support MC13892 LEDs
Bryan Wu
cooloney at gmail.com
Fri Jun 7 18:18:49 EDT 2013
On Mon, Jun 3, 2013 at 8:38 AM, Alexander Shiyan <shc_work at mail.ru> wrote:
> This patch prepares driver code to support MC13892 LEDs.
> Some stuff was optimized for support another PMIC, some
> dependencies was moved into devdata calls.
>
> Signed-off-by: Alexander Shiyan <shc_work at mail.ru>
> ---
> drivers/leds/leds-mc13783.c | 201 +++++++++++++++++++++++++-------------------
> include/linux/mfd/mc13xxx.h | 1 -
> 2 files changed, 113 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/leds/leds-mc13783.c b/drivers/leds/leds-mc13783.c
> index ea8fc5d..449940d 100644
> --- a/drivers/leds/leds-mc13783.c
> +++ b/drivers/leds/leds-mc13783.c
> @@ -24,7 +24,14 @@
> #include <linux/mfd/mc13xxx.h>
> #include <linux/slab.h>
>
> -struct mc13783_led {
> +struct mc13xxx_led_devtype {
> + int led_min;
> + int led_max;
> + int (*leds_startup)(struct platform_device *);
> + void (*leds_shutdown)(struct platform_device *);
> +};
> +
> +struct mc13xxx_led {
> struct led_classdev cdev;
> struct work_struct work;
> struct mc13xxx *master;
> @@ -32,7 +39,7 @@ struct mc13783_led {
> int id;
> };
>
> -#define MC13783_REG_LED_CONTROL_0 51
> +#define MC13XXX_REG_LED_CONTROL_0 51
> #define MC13783_LED_C0_ENABLE_BIT (1 << 0)
> #define MC13783_LED_C0_TRIODE_MD_BIT (1 << 7)
> #define MC13783_LED_C0_TRIODE_AD_BIT (1 << 8)
> @@ -43,10 +50,10 @@ struct mc13783_led {
> #define MC13783_LED_C0_ABREF_MASK 0x3
> #define MC13783_LED_C0_ABREF 14
>
> -#define MC13783_REG_LED_CONTROL_1 52
> +#define MC13XXX_REG_LED_CONTROL_1 52
> #define MC13783_LED_C1_TC1HALF_BIT (1 << 18)
>
> -#define MC13783_REG_LED_CONTROL_2 53
> +#define MC13XXX_REG_LED_CONTROL_2 53
> #define MC13783_LED_C2_BL_P_MASK 0xf
> #define MC13783_LED_C2_MD_P 9
> #define MC13783_LED_C2_AD_P 13
> @@ -56,7 +63,7 @@ struct mc13783_led {
> #define MC13783_LED_C2_AD_C 3
> #define MC13783_LED_C2_KP_C 6
>
> -#define MC13783_REG_LED_CONTROL_3 54
> +#define MC13XXX_REG_LED_CONTROL_3 54
> #define MC13783_LED_C3_TC_P 6
> #define MC13783_LED_C3_TC_P_MASK 0x1f
>
> @@ -69,29 +76,29 @@ struct mc13783_led {
> #define MC13783_LED_Cx_TRIODE_TC_BIT (1 << 23)
> #define MC13783_LED_Cx_TC_C_MASK 0x3
>
> -static void mc13783_led_work(struct work_struct *work)
> +static void mc13xxx_led_work(struct work_struct *work)
> {
> - struct mc13783_led *led = container_of(work, struct mc13783_led, work);
> - int reg = 0;
> - int mask = 0;
> - int value = 0;
> - int bank, off, shift;
> + struct mc13xxx_led *led = container_of(work, struct mc13xxx_led, work);
> + int reg, mask, value, bank, off, shift;
>
> switch (led->id) {
> case MC13783_LED_MD:
> - reg = MC13783_REG_LED_CONTROL_2;
> - mask = MC13783_LED_C2_BL_P_MASK << MC13783_LED_C2_MD_P;
> - value = (led->new_brightness >> 4) << MC13783_LED_C2_MD_P;
> + reg = MC13XXX_REG_LED_CONTROL_2;
> + mask = MC13783_LED_C2_BL_P_MASK;
> + shift = MC13783_LED_C2_MD_P;
> + value = led->new_brightness >> 4;
> break;
> case MC13783_LED_AD:
> - reg = MC13783_REG_LED_CONTROL_2;
> - mask = MC13783_LED_C2_BL_P_MASK << MC13783_LED_C2_AD_P;
> - value = (led->new_brightness >> 4) << MC13783_LED_C2_AD_P;
> + reg = MC13XXX_REG_LED_CONTROL_2;
> + mask = MC13783_LED_C2_BL_P_MASK;
> + shift = MC13783_LED_C2_AD_P;
> + value = led->new_brightness >> 4;
> break;
> case MC13783_LED_KP:
> - reg = MC13783_REG_LED_CONTROL_2;
> - mask = MC13783_LED_C2_BL_P_MASK << MC13783_LED_C2_KP_P;
> - value = (led->new_brightness >> 4) << MC13783_LED_C2_KP_P;
> + reg = MC13XXX_REG_LED_CONTROL_2;
> + mask = MC13783_LED_C2_BL_P_MASK;
> + shift = MC13783_LED_C2_KP_P;
> + value = led->new_brightness >> 4;
> break;
> case MC13783_LED_R1:
> case MC13783_LED_G1:
> @@ -103,57 +110,51 @@ static void mc13783_led_work(struct work_struct *work)
> case MC13783_LED_G3:
> case MC13783_LED_B3:
> off = led->id - MC13783_LED_R1;
> - bank = off/3;
> - reg = MC13783_REG_LED_CONTROL_3 + off/3;
> - shift = (off - bank * 3) * 5 + MC13783_LED_C3_TC_P;
> - value = (led->new_brightness >> 3) << shift;
> - mask = MC13783_LED_C3_TC_P_MASK << shift;
> + bank = off / 3;
> + reg = MC13XXX_REG_LED_CONTROL_3 + bank;
> + shift = (off - bank * 3) * 5 + MC13783_LED_C3_TC_P;
> + value = led->new_brightness >> 3;
> + mask = MC13783_LED_C3_TC_P_MASK;
> break;
> + default:
> + BUG();
> }
>
> mc13xxx_lock(led->master);
> -
> - mc13xxx_reg_rmw(led->master, reg, mask, value);
> -
> + mc13xxx_reg_rmw(led->master, reg, mask << shift,
> + value << shift);
> mc13xxx_unlock(led->master);
> }
>
> -static void mc13783_led_set(struct led_classdev *led_cdev,
> - enum led_brightness value)
> +static void mc13xxx_led_set(struct led_classdev *led_cdev,
> + enum led_brightness value)
> {
> - struct mc13783_led *led;
> + struct mc13xxx_led *led =
> + container_of(led_cdev, struct mc13xxx_led, cdev);
>
> - led = container_of(led_cdev, struct mc13783_led, cdev);
> led->new_brightness = value;
> schedule_work(&led->work);
> }
>
> -static int mc13783_led_setup(struct mc13783_led *led, int max_current)
> +static int mc13xxx_led_setup(struct mc13xxx_led *led, int max_current)
> {
> - int shift = 0;
> - int mask = 0;
> - int value = 0;
> - int reg = 0;
> - int ret, bank;
> + int shift, mask, reg, ret, bank;
>
> switch (led->id) {
> case MC13783_LED_MD:
> shift = MC13783_LED_C2_MD_C;
> mask = MC13783_LED_C2_BL_C_MASK;
> - value = max_current & MC13783_LED_C2_BL_C_MASK;
> - reg = MC13783_REG_LED_CONTROL_2;
> + reg = MC13XXX_REG_LED_CONTROL_2;
> break;
> case MC13783_LED_AD:
> shift = MC13783_LED_C2_AD_C;
> mask = MC13783_LED_C2_BL_C_MASK;
> - value = max_current & MC13783_LED_C2_BL_C_MASK;
> - reg = MC13783_REG_LED_CONTROL_2;
> + reg = MC13XXX_REG_LED_CONTROL_2;
> break;
> case MC13783_LED_KP:
> shift = MC13783_LED_C2_KP_C;
> mask = MC13783_LED_C2_BL_C_MASK;
> - value = max_current & MC13783_LED_C2_BL_C_MASK;
> - reg = MC13783_REG_LED_CONTROL_2;
> + reg = MC13XXX_REG_LED_CONTROL_2;
> break;
> case MC13783_LED_R1:
> case MC13783_LED_G1:
> @@ -164,29 +165,28 @@ static int mc13783_led_setup(struct mc13783_led *led, int max_current)
> case MC13783_LED_R3:
> case MC13783_LED_G3:
> case MC13783_LED_B3:
> - bank = (led->id - MC13783_LED_R1)/3;
> - reg = MC13783_REG_LED_CONTROL_3 + bank;
> + bank = (led->id - MC13783_LED_R1) / 3;
> + reg = MC13XXX_REG_LED_CONTROL_3 + bank;
> shift = ((led->id - MC13783_LED_R1) - bank * 3) * 2;
> mask = MC13783_LED_Cx_TC_C_MASK;
> - value = max_current & MC13783_LED_Cx_TC_C_MASK;
> break;
> + default:
> + BUG();
> }
>
> mc13xxx_lock(led->master);
> -
> ret = mc13xxx_reg_rmw(led->master, reg, mask << shift,
> - value << shift);
> -
> + (max_current & mask) << shift);
> mc13xxx_unlock(led->master);
> +
> return ret;
> }
>
> -static int mc13783_leds_prepare(struct platform_device *pdev)
> +static int mc13783_leds_startup(struct platform_device *pdev)
> {
> struct mc13xxx_leds_platform_data *pdata = dev_get_platdata(&pdev->dev);
> struct mc13xxx *dev = dev_get_drvdata(pdev->dev.parent);
> - int ret = 0;
> - int reg = 0;
> + int ret, reg = 0;
>
> mc13xxx_lock(dev);
>
> @@ -196,7 +196,7 @@ static int mc13783_leds_prepare(struct platform_device *pdev)
> if (pdata->flags & MC13783_LED_SLEWLIMTC)
> reg |= MC13783_LED_Cx_SLEWLIM_BIT;
>
> - ret = mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_1, reg);
> + ret = mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_1, reg);
> if (ret)
> goto out;
>
> @@ -206,7 +206,7 @@ static int mc13783_leds_prepare(struct platform_device *pdev)
> if (pdata->flags & MC13783_LED_SLEWLIMBL)
> reg |= MC13783_LED_Cx_SLEWLIM_BIT;
>
> - ret = mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_2, reg);
> + ret = mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_2, reg);
> if (ret)
> goto out;
>
> @@ -216,7 +216,7 @@ static int mc13783_leds_prepare(struct platform_device *pdev)
> if (pdata->flags & MC13783_LED_TRIODE_TC1)
> reg |= MC13783_LED_Cx_TRIODE_TC_BIT;
>
> - ret = mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_3, reg);
> + ret = mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_3, reg);
> if (ret)
> goto out;
>
> @@ -255,27 +255,47 @@ static int mc13783_leds_prepare(struct platform_device *pdev)
> reg |= (pdata->abref & MC13783_LED_C0_ABREF_MASK) <<
> MC13783_LED_C0_ABREF;
>
> - ret = mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_0, reg);
> + ret = mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_0, reg);
>
> out:
> mc13xxx_unlock(dev);
> +
> return ret;
> }
>
> -static int mc13783_led_probe(struct platform_device *pdev)
> +static void mc13783_leds_shutdown(struct platform_device *pdev)
> +{
> + struct mc13xxx *dev = dev_get_drvdata(pdev->dev.parent);
> +
> + mc13xxx_lock(dev);
> +
> + mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_0, 0);
> + mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_1, 0);
> + mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_2, 0);
> + mc13xxx_reg_write(dev, MC13XXX_REG_LED_CONTROL_3, 0);
> + mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_4, 0);
> + mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_5, 0);
> +
> + mc13xxx_unlock(dev);
> +}
> +
> +static int mc13xxx_led_probe(struct platform_device *pdev)
> {
> struct mc13xxx_leds_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + struct mc13xxx_led_devtype *devtype =
> + (struct mc13xxx_led_devtype *)pdev->id_entry->driver_data;
You don't need to do pointer type casting here. I'm just wandering can
we merge this struct mc13xxx_led_devtype into struct
mc13xxx_leds_platform_data?
> struct mc13xxx_led_platform_data *led_cur;
> - struct mc13783_led *led, *led_dat;
> + struct mc13xxx_led *led, *led_dat;
> + unsigned int init_led = 0;
> int ret, i;
> - int init_led = 0;
>
> if (pdata == NULL) {
> dev_err(&pdev->dev, "missing platform data\n");
> return -ENODEV;
> }
>
> - if (pdata->num_leds < 1 || pdata->num_leds > (MC13783_LED_MAX + 1)) {
> + if ((pdata->num_leds < 1) ||
> + (pdata->num_leds > (devtype->led_max - devtype->led_min + 1))) {
> dev_err(&pdev->dev, "Invalid led count %d\n", pdata->num_leds);
> return -EINVAL;
> }
> @@ -287,7 +307,7 @@ static int mc13783_led_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - ret = mc13783_leds_prepare(pdev);
> + ret = devtype->leds_startup(pdev);
> if (ret) {
> dev_err(&pdev->dev, "unable to init led driver\n");
> return ret;
> @@ -297,7 +317,8 @@ static int mc13783_led_probe(struct platform_device *pdev)
> led_dat = &led[i];
> led_cur = &pdata->led[i];
>
> - if (led_cur->id > MC13783_LED_MAX || led_cur->id < 0) {
> + if ((led_cur->id > devtype->led_max) ||
> + (led_cur->id < devtype->led_min)) {
> dev_err(&pdev->dev, "invalid id %d\n", led_cur->id);
> ret = -EINVAL;
> goto err_register;
> @@ -313,12 +334,12 @@ static int mc13783_led_probe(struct platform_device *pdev)
> init_led |= 1 << led_cur->id;
> led_dat->cdev.name = led_cur->name;
> led_dat->cdev.default_trigger = led_cur->default_trigger;
> - led_dat->cdev.brightness_set = mc13783_led_set;
> + led_dat->cdev.brightness_set = mc13xxx_led_set;
> led_dat->cdev.brightness = LED_OFF;
> led_dat->id = led_cur->id;
> led_dat->master = dev_get_drvdata(pdev->dev.parent);
>
> - INIT_WORK(&led_dat->work, mc13783_led_work);
> + INIT_WORK(&led_dat->work, mc13xxx_led_work);
>
> ret = led_classdev_register(pdev->dev.parent, &led_dat->cdev);
> if (ret) {
> @@ -327,7 +348,7 @@ static int mc13783_led_probe(struct platform_device *pdev)
> goto err_register;
> }
>
> - ret = mc13783_led_setup(led_dat, led_cur->max_current);
> + ret = mc13xxx_led_setup(led_dat, led_cur->max_current);
> if (ret) {
> dev_err(&pdev->dev, "unable to init led %d\n",
> led_dat->id);
> @@ -341,51 +362,55 @@ static int mc13783_led_probe(struct platform_device *pdev)
>
> err_register:
> for (i = i - 1; i >= 0; i--) {
> - led_classdev_unregister(&led[i].cdev);
> cancel_work_sync(&led[i].work);
> + led_classdev_unregister(&led[i].cdev);
> }
>
> return ret;
> }
>
> -static int mc13783_led_remove(struct platform_device *pdev)
> +static int mc13xxx_led_remove(struct platform_device *pdev)
> {
> struct mc13xxx_leds_platform_data *pdata = dev_get_platdata(&pdev->dev);
> - struct mc13783_led *led = platform_get_drvdata(pdev);
> - struct mc13xxx *dev = dev_get_drvdata(pdev->dev.parent);
> + struct mc13xxx_led_devtype *devtype =
> + (struct mc13xxx_led_devtype *)pdev->id_entry->driver_data;
> + struct mc13xxx_led *led = platform_get_drvdata(pdev);
> int i;
>
> for (i = 0; i < pdata->num_leds; i++) {
> - led_classdev_unregister(&led[i].cdev);
> cancel_work_sync(&led[i].work);
> + led_classdev_unregister(&led[i].cdev);
Yes, we normally unregister it firstly then cancel the workqueue.
> }
>
> - mc13xxx_lock(dev);
> -
> - mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_0, 0);
> - mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_1, 0);
> - mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_2, 0);
> - mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_3, 0);
> - mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_4, 0);
> - mc13xxx_reg_write(dev, MC13783_REG_LED_CONTROL_5, 0);
> -
> - mc13xxx_unlock(dev);
> + devtype->leds_shutdown(pdev);
>
> return 0;
> }
>
> -static struct platform_driver mc13783_led_driver = {
> +static const struct mc13xxx_led_devtype mc13783_led_devtype = {
> + .led_min = MC13783_LED_MD,
> + .led_max = MC13783_LED_B3,
> + .leds_startup = mc13783_leds_startup,
> + .leds_shutdown = mc13783_leds_shutdown,
> +};
> +
> +static const struct platform_device_id mc13xxx_led_id_table[] = {
> + { "mc13783-led", (kernel_ulong_t)&mc13783_led_devtype, },
> + { }
> +};
> +MODULE_DEVICE_TABLE(platform, mc13xxx_led_id_table);
> +
> +static struct platform_driver mc13xxx_led_driver = {
> .driver = {
> - .name = "mc13783-led",
> + .name = "mc13xxx-led",
> .owner = THIS_MODULE,
> },
> - .probe = mc13783_led_probe,
> - .remove = mc13783_led_remove,
> + .probe = mc13xxx_led_probe,
> + .remove = mc13xxx_led_remove,
> + .id_table = mc13xxx_led_id_table,
> };
> -
> -module_platform_driver(mc13783_led_driver);
> +module_platform_driver(mc13xxx_led_driver);
>
> MODULE_DESCRIPTION("LEDs driver for Freescale MC13783 PMIC");
> MODULE_AUTHOR("Philippe Retornaz <philippe.retornaz at epfl.ch>");
> MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:mc13783-led");
Do you need like this?
MODULE_ALIAS("platform:mc13xxx-led");
> diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
> index bf07075..fe0e003 100644
> --- a/include/linux/mfd/mc13xxx.h
> +++ b/include/linux/mfd/mc13xxx.h
> @@ -91,7 +91,6 @@ struct mc13xxx_led_platform_data {
> #define MC13783_LED_R3 9
> #define MC13783_LED_G3 10
> #define MC13783_LED_B3 11
> -#define MC13783_LED_MAX MC13783_LED_B3
> int id;
> const char *name;
> const char *default_trigger;
> --
> 1.8.1.5
>
More information about the linux-arm-kernel
mailing list