[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
