[PATCH] mfd: Add PWM1 and PWM2 support to twl6030-pwm
From: Hemanth V Date: Fri, 26 Aug 2011 10:49:29 +0530 Subject: [PATCH] Add PWM1 and PWM2 support to twl6030-pwm driver This patch adds support for PWM1/PWM2. TWL6030 PWM driver also supports Indicator LED PWM. Function pointers are defined for for init, enable, disable and configuration for both Indicator LED PWM (led_pwm) and PWM1/PWM2 (std_pwm) Tested-by: Tomi Valkeinen Signed-off-by: Hemanth V --- drivers/mfd/twl6030-pwm.c | 324 ++-- 1 files changed, 309 insertions(+), 15 deletions(-) diff --git a/drivers/mfd/twl6030-pwm.c b/drivers/mfd/twl6030-pwm.c index e8fee14..8d9c3f5 100644 --- a/drivers/mfd/twl6030-pwm.c +++ b/drivers/mfd/twl6030-pwm.c @@ -5,6 +5,9 @@ * Copyright (C) 2010 Texas Instruments * Author: Hemanth V * + * Added support for PWM1, PWM2 + * Hemanth V + * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 as published by * the Free Software Foundation. @@ -36,7 +39,9 @@ #define PWM_CTRL2_CURR_02 (2 << 4) /* LED supply source */ +#define PWM_CTRL2_SRC_VBUS (0 << 2) #define PWM_CTRL2_SRC_VAC (1 << 2) +#define PWM_CTRL2_SRC_EXT (2 << 2) /* LED modes */ #define PWM_CTRL2_MODE_HW (0 << 0) @@ -45,12 +50,53 @@ #define PWM_CTRL2_MODE_MASK0x3 +/* PWMs supported by driver */ +#define PWM_ID_LED 1 +#define PWM_ID_PWM12 +#define PWM_ID_PWM23 + +#define LED_PWM1ON 0x00 +#define LED_PWM1OFF0x01 +#define LED_PWM2ON 0x03 +#define LED_PWM2OFF0x04 +#define TWL6030_TOGGLE30x92 +#define PWMSTATUS2 0x94 + +/* Defines for TOGGLE3 register */ +#define PWM2EN (1 << 5) +#define PWM2S (1 << 4) +#define PWM2R (1 << 3) +#define PWM1EN (1 << 2) +#define PWM1S (1 << 1) +#define PWM1R (1 << 0) + +/* Defines for PWMSTATUS2 register */ +#define PWM1_CLK_EN(1 << 1) +#define PWM2_CLK_EN(1 << 3) +#defineTRUE1 +#defineFALSE 0 + +static DEFINE_MUTEX(pwm_lock); +static LIST_HEAD(pwm_list); + +struct pwm_device; + +struct pwm_ops { + int (*config)(struct pwm_device *, int, int); + int (*enable)(struct pwm_device *); + void(*disable)(struct pwm_device *); + int (*init)(struct pwm_device *); +}; + struct pwm_device { - const char *label; - unsigned int pwm_id; + struct list_headnode; + const char *label; + unsigned intpwm_id; + unsigned intuse_count; + struct pwm_ops *ops; }; -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) +int led_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) { u8 duty_cycle; int ret; @@ -69,9 +115,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) } return 0; } -EXPORT_SYMBOL(pwm_config); -int pwm_enable(struct pwm_device *pwm) +int led_pwm_enable(struct pwm_device *pwm) { u8 val; int ret; @@ -95,9 +140,8 @@ int pwm_enable(struct pwm_device *pwm) twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2); return 0; } -EXPORT_SYMBOL(pwm_enable); -void pwm_disable(struct pwm_device *pwm) +void led_pwm_disable(struct pwm_device *pwm) { u8 val; int ret; @@ -120,37 +164,284 @@ void pwm_disable(struct pwm_device *pwm) } return; } -EXPORT_SYMBOL(pwm_disable); -struct pwm_device *pwm_request(int pwm_id, const char *label) +int led_pwm_init(struct pwm_device *pwm) { u8 val; int ret; + + val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VBUS | + PWM_CTRL2_MODE_HW; + + ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2); + + return ret; +} + +static struct pwm_ops pwm_led = { + .config = led_pwm_config, + .enable = led_pwm_enable, + .disable = led_pwm_disable, + .init = led_pwm_init, +}; + +int std_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) +{ + int ret = 0, level, pwm_id, reg; + + level = (duty_ns * PWM_CTRL1_MAX) / period_ns; + pwm_id = pwm->pwm_id; + + if (pwm_id == PWM_ID_PWM1) + reg = LED_PWM1ON; + else + reg = LED_PWM2ON; + + if (level > 1) { + if (level == 255) + level = 0x7F; + else + level = (~(level/2)) & 0x7F; + + ret = twl_i2c_write_u8(TWL_MODULE_PWM, level, reg); + } + + return ret; +} + +void std_pwm_disable(struct pwm_device *pwm) +{ + + int ret, pwm_id; + bool pwm1_enabled = FALSE; + bool pwm2_enabled = FALSE; + u8 reg_val, reset_val, dis_val = 0; + +
Re: [PATCH] mfd: Add PWM1 and PWM2 support to twl6030-pwm
> From: Hemanth V > Date: Fri, 26 Aug 2011 10:49:29 +0530 > Subject: [PATCH] Add PWM1 and PWM2 support to twl6030-pwm driver Samuel, any comments on this patch. Thanks Hemanth > > This patch adds support for PWM1/PWM2. TWL6030 PWM driver also > supports Indicator LED PWM. Function pointers are defined for > for init, enable, disable and configuration for both Indicator LED > PWM (led_pwm) and PWM1/PWM2 (std_pwm) > > Tested-by: Tomi Valkeinen > Signed-off-by: Hemanth V > --- > drivers/mfd/twl6030-pwm.c | 324 ++-- > 1 files changed, 309 insertions(+), 15 deletions(-) > > diff --git a/drivers/mfd/twl6030-pwm.c b/drivers/mfd/twl6030-pwm.c > index e8fee14..8d9c3f5 100644 > --- a/drivers/mfd/twl6030-pwm.c > +++ b/drivers/mfd/twl6030-pwm.c > @@ -5,6 +5,9 @@ > * Copyright (C) 2010 Texas Instruments > * Author: Hemanth V > * > + * Added support for PWM1, PWM2 > + * Hemanth V > + * > * This program is free software; you can redistribute it and/or modify it > * under the terms of the GNU General Public License version 2 as published > by > * the Free Software Foundation. > @@ -36,7 +39,9 @@ > #define PWM_CTRL2_CURR_02(2 << 4) > > /* LED supply source */ > +#define PWM_CTRL2_SRC_VBUS (0 << 2) > #define PWM_CTRL2_SRC_VAC(1 << 2) > +#define PWM_CTRL2_SRC_EXT(2 << 2) > > /* LED modes */ > #define PWM_CTRL2_MODE_HW(0 << 0) > @@ -45,12 +50,53 @@ > > #define PWM_CTRL2_MODE_MASK 0x3 > > +/* PWMs supported by driver */ > +#define PWM_ID_LED 1 > +#define PWM_ID_PWM1 2 > +#define PWM_ID_PWM2 3 > + > +#define LED_PWM1ON 0x00 > +#define LED_PWM1OFF 0x01 > +#define LED_PWM2ON 0x03 > +#define LED_PWM2OFF 0x04 > +#define TWL6030_TOGGLE3 0x92 > +#define PWMSTATUS2 0x94 > + > +/* Defines for TOGGLE3 register */ > +#define PWM2EN (1 << 5) > +#define PWM2S(1 << 4) > +#define PWM2R(1 << 3) > +#define PWM1EN (1 << 2) > +#define PWM1S(1 << 1) > +#define PWM1R(1 << 0) > + > +/* Defines for PWMSTATUS2 register */ > +#define PWM1_CLK_EN (1 << 1) > +#define PWM2_CLK_EN (1 << 3) > +#define TRUE1 > +#define FALSE 0 > + > +static DEFINE_MUTEX(pwm_lock); > +static LIST_HEAD(pwm_list); > + > +struct pwm_device; > + > +struct pwm_ops { > + int (*config)(struct pwm_device *, int, int); > + int (*enable)(struct pwm_device *); > + void(*disable)(struct pwm_device *); > + int (*init)(struct pwm_device *); > +}; > + > struct pwm_device { > - const char *label; > - unsigned int pwm_id; > + struct list_headnode; > + const char *label; > + unsigned intpwm_id; > + unsigned intuse_count; > + struct pwm_ops *ops; > }; > > -int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) > +int led_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) > { > u8 duty_cycle; > int ret; > @@ -69,9 +115,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int > period_ns) > } > return 0; > } > -EXPORT_SYMBOL(pwm_config); > > -int pwm_enable(struct pwm_device *pwm) > +int led_pwm_enable(struct pwm_device *pwm) > { > u8 val; > int ret; > @@ -95,9 +140,8 @@ int pwm_enable(struct pwm_device *pwm) > twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2); > return 0; > } > -EXPORT_SYMBOL(pwm_enable); > > -void pwm_disable(struct pwm_device *pwm) > +void led_pwm_disable(struct pwm_device *pwm) > { > u8 val; > int ret; > @@ -120,37 +164,284 @@ void pwm_disable(struct pwm_device *pwm) > } > return; > } > -EXPORT_SYMBOL(pwm_disable); > > -struct pwm_device *pwm_request(int pwm_id, const char *label) > +int led_pwm_init(struct pwm_device *pwm) > { > u8 val; > int ret; > + > + val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VBUS | > + PWM_CTRL2_MODE_HW; > + > + ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2); > + > + return ret; > +} > + > +static struct pwm_ops pwm_led = { > + .config = led_pwm_config, > + .enable = led_pwm_enable, > + .disable = led_pwm_disable, > + .init = led_pwm_init, > +}; > + > +int std_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) > +{ > + int ret = 0, level, pwm_id, reg; > + > + level = (duty_ns * PWM_CTRL1_MAX) / period_ns; > + pwm_id = pwm->pwm_id; > + > + if (pwm_id == PWM_ID_PWM1) > + reg = LED_PWM1ON; > + else > + reg = LED_PWM2ON; > + > + if (level > 1) { > + if (level == 255) > + level = 0x7F; > + else > + level = (~(level/2)) & 0x7F; > + > + re
Re: [PATCH] mfd: Add PWM1 and PWM2 support to twl6030-pwm
Hi Hemanth, On Tue, Aug 30, 2011 at 12:27:17PM +0530, Hemanth V wrote: > From: Hemanth V > Date: Fri, 26 Aug 2011 10:49:29 +0530 > Subject: [PATCH] Add PWM1 and PWM2 support to twl6030-pwm driver > > This patch adds support for PWM1/PWM2. TWL6030 PWM driver also > supports Indicator LED PWM. Function pointers are defined for > for init, enable, disable and configuration for both Indicator LED > PWM (led_pwm) and PWM1/PWM2 (std_pwm) Some comments on this code: > +/* PWMs supported by driver */ > +#define PWM_ID_LED 1 > +#define PWM_ID_PWM1 2 > +#define PWM_ID_PWM2 3 I wish we could use enums here, but that's not what the PWM API is expecting. > +int led_pwm_enable(struct pwm_device *pwm) All your pwm_ops should be static now. > { > u8 val; > int ret; > @@ -95,9 +140,8 @@ int pwm_enable(struct pwm_device *pwm) > twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, LED_PWM_CTRL2); > return 0; > } > -EXPORT_SYMBOL(pwm_enable); > > -void pwm_disable(struct pwm_device *pwm) > +void led_pwm_disable(struct pwm_device *pwm) > { > u8 val; > int ret; > @@ -120,37 +164,284 @@ void pwm_disable(struct pwm_device *pwm) > } > return; > } > -EXPORT_SYMBOL(pwm_disable); > > -struct pwm_device *pwm_request(int pwm_id, const char *label) > +int led_pwm_init(struct pwm_device *pwm) > { > u8 val; > int ret; > + > + val = PWM_CTRL2_DIS_PD | PWM_CTRL2_CURR_02 | PWM_CTRL2_SRC_VBUS | > + PWM_CTRL2_MODE_HW; > + > + ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, LED_PWM_CTRL2); > + > + return ret; > +} > + > +static struct pwm_ops pwm_led = { > + .config = led_pwm_config, > + .enable = led_pwm_enable, > + .disable = led_pwm_disable, > + .init = led_pwm_init, > +}; > + > +int std_pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) > +{ > + int ret = 0, level, pwm_id, reg; > + > + level = (duty_ns * PWM_CTRL1_MAX) / period_ns; > + pwm_id = pwm->pwm_id; > + > + if (pwm_id == PWM_ID_PWM1) > + reg = LED_PWM1ON; > + else > + reg = LED_PWM2ON; This is not consistent with your: if (PWM1) else if (PWM2) else error logic below. Moreover, I'd rather use switch() here but that's more of a personal taste than anything else. > +struct pwm_device *pwm_request(int pwm_id, const char *label) > +{ > + int ret, found = 0; > struct pwm_device *pwm; > > + mutex_lock(&pwm_lock); > + > + list_for_each_entry(pwm, &pwm_list, node) { > + if (pwm->pwm_id == pwm_id) { > + found = 1; > + break; > + } > + } > + > + if (found) { > + if (pwm->use_count == 0) { > + pwm->use_count++; > + pwm->label = label; > + } else { > + pwm = ERR_PTR(-EBUSY); > + } I failed to understand the logic here. How can you have found == TRUE, and use_count being 0 ? Also, don't you want to track the pwm users and disable it when user_count is reaching 0 ? You're not doing that from pwm_free(). > + goto out; You're leaving with the pwm_lock locked. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html