subject:"\[PATCH 2\/3\] pwm\: New driver to support PWMs on TWL4030\/6030 series of PMICs"

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-08 Thread Grazvydas Ignotas

On Thu, Nov 8, 2012 at 9:14 AM, Péter Ujfalusi  wrote:
> On 11/07/2012 06:50 PM, Grazvydas Ignotas wrote:
>>> +   if (pwm->hwpwm) {
>>> +   /* PWM 1 */
>>> +   mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
>>> +   bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
>>> +   } else {
>>> +   /* PWM 0 */
>>> +   mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
>>> +   bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
>>> +   }
>>> +
>>> +   /* Save the current MUX configuration for the PWM */
>>> +   twl->twl4030_pwm_mux &= ~mask;
>>> +   twl->twl4030_pwm_mux |= (val & mask);
>>
>> Do we really need this mask clearing here? After probe twl4030_pwm_mux
>> should be zero, and if twl4030_pwm_request is called twice you don't
>> clear the important bits before |=, I think 'twl4030_pwm_mux = val &
>> mask' would be better here.
>
> I'm storing both PWM's state in the same variable, but in different offsets:
> PWM0: bits 2-3
> PWM1: bits 4-5
> Probably it is over engineering to clear the relevant bits in the backup
> storage, but better to be safe IMHO.
> I would leave this part as it is.

Oh, it should be good then.


-- 
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-08 Thread Grazvydas Ignotas

On Thu, Nov 8, 2012 at 9:14 AM, Péter Ujfalusi peter.ujfal...@ti.com wrote:
 On 11/07/2012 06:50 PM, Grazvydas Ignotas wrote:
 +   if (pwm-hwpwm) {
 +   /* PWM 1 */
 +   mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
 +   bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
 +   } else {
 +   /* PWM 0 */
 +   mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
 +   bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
 +   }
 +
 +   /* Save the current MUX configuration for the PWM */
 +   twl-twl4030_pwm_mux = ~mask;
 +   twl-twl4030_pwm_mux |= (val  mask);

 Do we really need this mask clearing here? After probe twl4030_pwm_mux
 should be zero, and if twl4030_pwm_request is called twice you don't
 clear the important bits before |=, I think 'twl4030_pwm_mux = val 
 mask' would be better here.

 I'm storing both PWM's state in the same variable, but in different offsets:
 PWM0: bits 2-3
 PWM1: bits 4-5
 Probably it is over engineering to clear the relevant bits in the backup
 storage, but better to be safe IMHO.
 I would leave this part as it is.

Oh, it should be good then.


-- 
Gražvydas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-07 Thread Péter Ujfalusi

On 11/07/2012 06:50 PM, Grazvydas Ignotas wrote:
>> +static int twl4030_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>> +{
>> +   int ret;
>> +   u8 val;
>> +
>> +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_GPBR1_REG);
>> +   if (ret < 0) {
>> +   dev_err(chip->dev, "%s: Failed to read GPBR1\n", pwm->label);
>> +   return ret;
>> +   }
>> +
>> +   val |= TWL4030_PWM_TOGGLE(pwm->hwpwm, TWL4030_PWMX_BITS);
> 
> In my experience doing it like this doesn't work reliably, i.e.
> sometimes it just won't enable. I had to first set CLK_ENABLE bit, and
> then ENABLE bit with separate i2c write. Perhaps it needs a cycle or
> two of 32k clock or something (that doesn't seem to be documented
> though).

Thanks, I'll change to the reliable sequence. I do not have HW where I can
test the twl4030 PWMs.

> 
>> +
>> +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
>> +   if (ret < 0)
>> +   dev_err(chip->dev, "%s: Failed to enable PWM\n", pwm->label);
>> +
>> +   return ret;
>> +}
>> +
>> +static void twl4030_pwm_disable(struct pwm_chip *chip, struct pwm_device 
>> *pwm)
>> +{
>> +   int ret;
>> +   u8 val;
>> +
>> +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_GPBR1_REG);
>> +   if (ret < 0) {
>> +   dev_err(chip->dev, "%s: Failed to read GPBR1\n", pwm->label);
>> +   return;
>> +   }
>> +
>> +   val &= ~TWL4030_PWM_TOGGLE(pwm->hwpwm, TWL4030_PWMX_BITS);
> 
> Same problem here, I would sometimes get LED stuck at full brightness
> after this, first clearing ENABLE and then CLK_ENABLE fixed it (we
> have charger LED connected to PWM1 on pandora).

I would guessed that if we need special care we should have turned off CLK
followed by disabling the PWM.
I'll use the sequence you described in the next version.

> 
>> +
>> +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
>> +   if (ret < 0)
>> +   dev_err(chip->dev, "%s: Failed to disable PWM\n", 
>> pwm->label);
>> +}
>> +
>> +static int twl4030_pwm_request(struct pwm_chip *chip, struct pwm_device 
>> *pwm)
>> +{
>> +   struct twl_pwm_chip *twl = container_of(chip, struct twl_pwm_chip,
>> +   chip);
>> +   int ret;
>> +   u8 val, mask, bits;
>> +
>> +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_PMBR1_REG);
>> +   if (ret < 0) {
>> +   dev_err(chip->dev, "%s: Failed to read PMBR1\n", pwm->label);
>> +   return ret;
>> +   }
>> +
>> +   if (pwm->hwpwm) {
>> +   /* PWM 1 */
>> +   mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
>> +   bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
>> +   } else {
>> +   /* PWM 0 */
>> +   mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
>> +   bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
>> +   }
>> +
>> +   /* Save the current MUX configuration for the PWM */
>> +   twl->twl4030_pwm_mux &= ~mask;
>> +   twl->twl4030_pwm_mux |= (val & mask);
> 
> Do we really need this mask clearing here? After probe twl4030_pwm_mux
> should be zero, and if twl4030_pwm_request is called twice you don't
> clear the important bits before |=, I think 'twl4030_pwm_mux = val &
> mask' would be better here.

I'm storing both PWM's state in the same variable, but in different offsets:
PWM0: bits 2-3
PWM1: bits 4-5
Probably it is over engineering to clear the relevant bits in the backup
storage, but better to be safe IMHO.
I would leave this part as it is.

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-07 Thread Grazvydas Ignotas

On Wed, Nov 7, 2012 at 4:44 PM, Peter Ujfalusi  wrote:
> The driver supports the following PWM outputs:
> TWL4030 PWM0 and PWM1
> TWL6030 PWM1 and PWM2
>
> On TWL4030 the PWM signals are muxed. Upon requesting the PWM the driver
> will select the correct mux so the PWM can be used. When the PWM has been
> freed the original configuration going to be restored.
>
> Signed-off-by: Peter Ujfalusi 
> ---
>  drivers/pwm/Kconfig   |  10 ++
>  drivers/pwm/Makefile  |   1 +
>  drivers/pwm/pwm-twl.c | 304 
> ++
>  3 files changed, 315 insertions(+)
>  create mode 100644 drivers/pwm/pwm-twl.c
>



> --- /dev/null
> +++ b/drivers/pwm/pwm-twl.c



> +
> +static int twl4030_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +   int ret;
> +   u8 val;
> +
> +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_GPBR1_REG);
> +   if (ret < 0) {
> +   dev_err(chip->dev, "%s: Failed to read GPBR1\n", pwm->label);
> +   return ret;
> +   }
> +
> +   val |= TWL4030_PWM_TOGGLE(pwm->hwpwm, TWL4030_PWMX_BITS);

In my experience doing it like this doesn't work reliably, i.e.
sometimes it just won't enable. I had to first set CLK_ENABLE bit, and
then ENABLE bit with separate i2c write. Perhaps it needs a cycle or
two of 32k clock or something (that doesn't seem to be documented
though).

> +
> +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
> +   if (ret < 0)
> +   dev_err(chip->dev, "%s: Failed to enable PWM\n", pwm->label);
> +
> +   return ret;
> +}
> +
> +static void twl4030_pwm_disable(struct pwm_chip *chip, struct pwm_device 
> *pwm)
> +{
> +   int ret;
> +   u8 val;
> +
> +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_GPBR1_REG);
> +   if (ret < 0) {
> +   dev_err(chip->dev, "%s: Failed to read GPBR1\n", pwm->label);
> +   return;
> +   }
> +
> +   val &= ~TWL4030_PWM_TOGGLE(pwm->hwpwm, TWL4030_PWMX_BITS);

Same problem here, I would sometimes get LED stuck at full brightness
after this, first clearing ENABLE and then CLK_ENABLE fixed it (we
have charger LED connected to PWM1 on pandora).

> +
> +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
> +   if (ret < 0)
> +   dev_err(chip->dev, "%s: Failed to disable PWM\n", pwm->label);
> +}
> +
> +static int twl4030_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +   struct twl_pwm_chip *twl = container_of(chip, struct twl_pwm_chip,
> +   chip);
> +   int ret;
> +   u8 val, mask, bits;
> +
> +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_PMBR1_REG);
> +   if (ret < 0) {
> +   dev_err(chip->dev, "%s: Failed to read PMBR1\n", pwm->label);
> +   return ret;
> +   }
> +
> +   if (pwm->hwpwm) {
> +   /* PWM 1 */
> +   mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
> +   bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
> +   } else {
> +   /* PWM 0 */
> +   mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
> +   bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
> +   }
> +
> +   /* Save the current MUX configuration for the PWM */
> +   twl->twl4030_pwm_mux &= ~mask;
> +   twl->twl4030_pwm_mux |= (val & mask);

Do we really need this mask clearing here? After probe twl4030_pwm_mux
should be zero, and if twl4030_pwm_request is called twice you don't
clear the important bits before |=, I think 'twl4030_pwm_mux = val &
mask' would be better here.


-- 
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-07 Thread Peter Ujfalusi

The driver supports the following PWM outputs:
TWL4030 PWM0 and PWM1
TWL6030 PWM1 and PWM2

On TWL4030 the PWM signals are muxed. Upon requesting the PWM the driver
will select the correct mux so the PWM can be used. When the PWM has been
freed the original configuration going to be restored.

Signed-off-by: Peter Ujfalusi 
---
 drivers/pwm/Kconfig   |  10 ++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-twl.c | 304 ++
 3 files changed, 315 insertions(+)
 create mode 100644 drivers/pwm/pwm-twl.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e678005..c577db9 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -142,6 +142,16 @@ config  PWM_TIEHRPWM
  To compile this driver as a module, choose M here: the module
  will be called pwm-tiehrpwm.
 
+config PWM_TWL
+   tristate "TWL4030/6030 PWM support"
+   select HAVE_PWM
+   depends on TWL4030_CORE
+   help
+ Generic PWM framework driver for TWL4030/6030.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-twl.
+
 config PWM_VT8500
tristate "vt8500 pwm support"
depends on ARCH_VT8500
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 29cf57e..3324c06 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -11,4 +11,5 @@ obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
 obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
 obj-$(CONFIG_PWM_TIECAP)   += pwm-tiecap.o
 obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
+obj-$(CONFIG_PWM_TWL)  += pwm-twl.o
 obj-$(CONFIG_PWM_VT8500)   += pwm-vt8500.o
diff --git a/drivers/pwm/pwm-twl.c b/drivers/pwm/pwm-twl.c
new file mode 100644
index 000..0821ffe
--- /dev/null
+++ b/drivers/pwm/pwm-twl.c
@@ -0,0 +1,304 @@
+/*
+ * Driver for TWL4030/6030 Generic Pulse Width Modulator
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Peter Ujfalusi 
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define TWL_PWM_MAX0x7f
+
+/* Registers, bits and macro for TWL4030 */
+#define TWL4030_GPBR1_REG  0x0c
+#define TWL4030_PMBR1_REG  0x0d
+
+/* GPBR1 register bits */
+#define TWL4030_PWMXCLK_ENABLE (1 << 0)
+#define TWL4030_PWMX_ENABLE(1 << 2)
+#define TWL4030_PWMX_BITS  (TWL4030_PWMX_ENABLE | TWL4030_PWMXCLK_ENABLE)
+#define TWL4030_PWM_TOGGLE(pwm, x) ((x) << (pwm))
+
+/* PMBR1 register bits */
+#define TWL4030_GPIO6_PWM0_MUTE_MASK   (0x03 << 2)
+#define TWL4030_GPIO6_PWM0_MUTE_PWM0   (0x01 << 2)
+#define TWL4030_GPIO7_VIBRASYNC_PWM1_MASK  (0x03 << 4)
+#define TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1  (0x03 << 4)
+
+/* Register, bits and macro for TWL6030 */
+#define TWL6030_TOGGLE3_REG0x92
+
+#define TWL6030_PWMXR  (1 << 0)
+#define TWL6030_PWMXS  (1 << 1)
+#define TWL6030_PWMXEN (1 << 2)
+#define TWL6030_PWM_TOGGLE(pwm, x) ((x) << (pwm * 3))
+
+struct twl_pwm_chip {
+   struct pwm_chip chip;
+   u8 twl6030_toggle3;
+   u8 twl4030_pwm_mux;
+};
+
+static int twl_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+   int duty_cycle = (duty_ns * TWL_PWM_MAX) / period_ns;
+   u8 on_time;
+   u8 pwm_config[2];
+   int base, ret;
+
+   if (duty_cycle >= TWL_PWM_MAX)
+   on_time = TWL_PWM_MAX;
+   else if (!duty_cycle)
+   on_time = TWL_PWM_MAX - 1;
+   else
+   on_time = TWL_PWM_MAX - duty_cycle;
+
+   base = pwm->hwpwm * 3;
+
+   pwm_config[0] = on_time;
+   pwm_config[1] = TWL_PWM_MAX;
+
+   ret = twl_i2c_write(TWL_MODULE_PWM, pwm_config, base, 2);
+   if (ret < 0)
+   dev_err(chip->dev, "%s: Failed to configure PWM\n", pwm->label);
+
+   return ret;
+}
+
+static int twl4030_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+   int ret;
+   u8 val;
+
+   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, , TWL4030_GPBR1_REG);
+   if (ret < 0) {
+   dev_err(chip->dev, "%s: Failed to read GPBR1\n", pwm->label);
+   return ret;
+   }
+
+   val |= TWL4030_PWM_TOGGLE(pwm->hwpwm, TWL4030_PWMX_BITS);
+
+   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
+   if (ret < 0)
+

[PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-07 Thread Peter Ujfalusi

The driver supports the following PWM outputs:
TWL4030 PWM0 and PWM1
TWL6030 PWM1 and PWM2

On TWL4030 the PWM signals are muxed. Upon requesting the PWM the driver
will select the correct mux so the PWM can be used. When the PWM has been
freed the original configuration going to be restored.

Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
---
 drivers/pwm/Kconfig   |  10 ++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-twl.c | 304 ++
 3 files changed, 315 insertions(+)
 create mode 100644 drivers/pwm/pwm-twl.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e678005..c577db9 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -142,6 +142,16 @@ config  PWM_TIEHRPWM
  To compile this driver as a module, choose M here: the module
  will be called pwm-tiehrpwm.
 
+config PWM_TWL
+   tristate TWL4030/6030 PWM support
+   select HAVE_PWM
+   depends on TWL4030_CORE
+   help
+ Generic PWM framework driver for TWL4030/6030.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-twl.
+
 config PWM_VT8500
tristate vt8500 pwm support
depends on ARCH_VT8500
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 29cf57e..3324c06 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -11,4 +11,5 @@ obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
 obj-$(CONFIG_PWM_TEGRA)+= pwm-tegra.o
 obj-$(CONFIG_PWM_TIECAP)   += pwm-tiecap.o
 obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
+obj-$(CONFIG_PWM_TWL)  += pwm-twl.o
 obj-$(CONFIG_PWM_VT8500)   += pwm-vt8500.o
diff --git a/drivers/pwm/pwm-twl.c b/drivers/pwm/pwm-twl.c
new file mode 100644
index 000..0821ffe
--- /dev/null
+++ b/drivers/pwm/pwm-twl.c
@@ -0,0 +1,304 @@
+/*
+ * Driver for TWL4030/6030 Generic Pulse Width Modulator
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Peter Ujfalusi peter.ujfal...@ti.com
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/pwm.h
+#include linux/i2c/twl.h
+#include linux/slab.h
+
+#define TWL_PWM_MAX0x7f
+
+/* Registers, bits and macro for TWL4030 */
+#define TWL4030_GPBR1_REG  0x0c
+#define TWL4030_PMBR1_REG  0x0d
+
+/* GPBR1 register bits */
+#define TWL4030_PWMXCLK_ENABLE (1  0)
+#define TWL4030_PWMX_ENABLE(1  2)
+#define TWL4030_PWMX_BITS  (TWL4030_PWMX_ENABLE | TWL4030_PWMXCLK_ENABLE)
+#define TWL4030_PWM_TOGGLE(pwm, x) ((x)  (pwm))
+
+/* PMBR1 register bits */
+#define TWL4030_GPIO6_PWM0_MUTE_MASK   (0x03  2)
+#define TWL4030_GPIO6_PWM0_MUTE_PWM0   (0x01  2)
+#define TWL4030_GPIO7_VIBRASYNC_PWM1_MASK  (0x03  4)
+#define TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1  (0x03  4)
+
+/* Register, bits and macro for TWL6030 */
+#define TWL6030_TOGGLE3_REG0x92
+
+#define TWL6030_PWMXR  (1  0)
+#define TWL6030_PWMXS  (1  1)
+#define TWL6030_PWMXEN (1  2)
+#define TWL6030_PWM_TOGGLE(pwm, x) ((x)  (pwm * 3))
+
+struct twl_pwm_chip {
+   struct pwm_chip chip;
+   u8 twl6030_toggle3;
+   u8 twl4030_pwm_mux;
+};
+
+static int twl_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+   int duty_cycle = (duty_ns * TWL_PWM_MAX) / period_ns;
+   u8 on_time;
+   u8 pwm_config[2];
+   int base, ret;
+
+   if (duty_cycle = TWL_PWM_MAX)
+   on_time = TWL_PWM_MAX;
+   else if (!duty_cycle)
+   on_time = TWL_PWM_MAX - 1;
+   else
+   on_time = TWL_PWM_MAX - duty_cycle;
+
+   base = pwm-hwpwm * 3;
+
+   pwm_config[0] = on_time;
+   pwm_config[1] = TWL_PWM_MAX;
+
+   ret = twl_i2c_write(TWL_MODULE_PWM, pwm_config, base, 2);
+   if (ret  0)
+   dev_err(chip-dev, %s: Failed to configure PWM\n, pwm-label);
+
+   return ret;
+}
+
+static int twl4030_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+   int ret;
+   u8 val;
+
+   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
+   if (ret  0) {
+   dev_err(chip-dev, %s: Failed to read GPBR1\n, pwm-label);
+   return ret;
+   }
+
+   val |= TWL4030_PWM_TOGGLE(pwm-hwpwm, TWL4030_PWMX_BITS);
+
+   ret =

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-07 Thread Grazvydas Ignotas

On Wed, Nov 7, 2012 at 4:44 PM, Peter Ujfalusi peter.ujfal...@ti.com wrote:
 The driver supports the following PWM outputs:
 TWL4030 PWM0 and PWM1
 TWL6030 PWM1 and PWM2

 On TWL4030 the PWM signals are muxed. Upon requesting the PWM the driver
 will select the correct mux so the PWM can be used. When the PWM has been
 freed the original configuration going to be restored.

 Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
 ---
  drivers/pwm/Kconfig   |  10 ++
  drivers/pwm/Makefile  |   1 +
  drivers/pwm/pwm-twl.c | 304 
 ++
  3 files changed, 315 insertions(+)
  create mode 100644 drivers/pwm/pwm-twl.c


snip

 --- /dev/null
 +++ b/drivers/pwm/pwm-twl.c

snip

 +
 +static int twl4030_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 +{
 +   int ret;
 +   u8 val;
 +
 +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0) {
 +   dev_err(chip-dev, %s: Failed to read GPBR1\n, pwm-label);
 +   return ret;
 +   }
 +
 +   val |= TWL4030_PWM_TOGGLE(pwm-hwpwm, TWL4030_PWMX_BITS);

In my experience doing it like this doesn't work reliably, i.e.
sometimes it just won't enable. I had to first set CLK_ENABLE bit, and
then ENABLE bit with separate i2c write. Perhaps it needs a cycle or
two of 32k clock or something (that doesn't seem to be documented
though).

 +
 +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0)
 +   dev_err(chip-dev, %s: Failed to enable PWM\n, pwm-label);
 +
 +   return ret;
 +}
 +
 +static void twl4030_pwm_disable(struct pwm_chip *chip, struct pwm_device 
 *pwm)
 +{
 +   int ret;
 +   u8 val;
 +
 +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0) {
 +   dev_err(chip-dev, %s: Failed to read GPBR1\n, pwm-label);
 +   return;
 +   }
 +
 +   val = ~TWL4030_PWM_TOGGLE(pwm-hwpwm, TWL4030_PWMX_BITS);

Same problem here, I would sometimes get LED stuck at full brightness
after this, first clearing ENABLE and then CLK_ENABLE fixed it (we
have charger LED connected to PWM1 on pandora).

 +
 +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0)
 +   dev_err(chip-dev, %s: Failed to disable PWM\n, pwm-label);
 +}
 +
 +static int twl4030_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 +{
 +   struct twl_pwm_chip *twl = container_of(chip, struct twl_pwm_chip,
 +   chip);
 +   int ret;
 +   u8 val, mask, bits;
 +
 +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_PMBR1_REG);
 +   if (ret  0) {
 +   dev_err(chip-dev, %s: Failed to read PMBR1\n, pwm-label);
 +   return ret;
 +   }
 +
 +   if (pwm-hwpwm) {
 +   /* PWM 1 */
 +   mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
 +   bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
 +   } else {
 +   /* PWM 0 */
 +   mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
 +   bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
 +   }
 +
 +   /* Save the current MUX configuration for the PWM */
 +   twl-twl4030_pwm_mux = ~mask;
 +   twl-twl4030_pwm_mux |= (val  mask);

Do we really need this mask clearing here? After probe twl4030_pwm_mux
should be zero, and if twl4030_pwm_request is called twice you don't
clear the important bits before |=, I think 'twl4030_pwm_mux = val 
mask' would be better here.


-- 
Gražvydas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

2012-11-07 Thread Péter Ujfalusi

On 11/07/2012 06:50 PM, Grazvydas Ignotas wrote:
 +static int twl4030_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 +{
 +   int ret;
 +   u8 val;
 +
 +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0) {
 +   dev_err(chip-dev, %s: Failed to read GPBR1\n, pwm-label);
 +   return ret;
 +   }
 +
 +   val |= TWL4030_PWM_TOGGLE(pwm-hwpwm, TWL4030_PWMX_BITS);
 
 In my experience doing it like this doesn't work reliably, i.e.
 sometimes it just won't enable. I had to first set CLK_ENABLE bit, and
 then ENABLE bit with separate i2c write. Perhaps it needs a cycle or
 two of 32k clock or something (that doesn't seem to be documented
 though).

Thanks, I'll change to the reliable sequence. I do not have HW where I can
test the twl4030 PWMs.

 
 +
 +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0)
 +   dev_err(chip-dev, %s: Failed to enable PWM\n, pwm-label);
 +
 +   return ret;
 +}
 +
 +static void twl4030_pwm_disable(struct pwm_chip *chip, struct pwm_device 
 *pwm)
 +{
 +   int ret;
 +   u8 val;
 +
 +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0) {
 +   dev_err(chip-dev, %s: Failed to read GPBR1\n, pwm-label);
 +   return;
 +   }
 +
 +   val = ~TWL4030_PWM_TOGGLE(pwm-hwpwm, TWL4030_PWMX_BITS);
 
 Same problem here, I would sometimes get LED stuck at full brightness
 after this, first clearing ENABLE and then CLK_ENABLE fixed it (we
 have charger LED connected to PWM1 on pandora).

I would guessed that if we need special care we should have turned off CLK
followed by disabling the PWM.
I'll use the sequence you described in the next version.

 
 +
 +   ret = twl_i2c_write_u8(TWL4030_MODULE_INTBR, val, TWL4030_GPBR1_REG);
 +   if (ret  0)
 +   dev_err(chip-dev, %s: Failed to disable PWM\n, 
 pwm-label);
 +}
 +
 +static int twl4030_pwm_request(struct pwm_chip *chip, struct pwm_device 
 *pwm)
 +{
 +   struct twl_pwm_chip *twl = container_of(chip, struct twl_pwm_chip,
 +   chip);
 +   int ret;
 +   u8 val, mask, bits;
 +
 +   ret = twl_i2c_read_u8(TWL4030_MODULE_INTBR, val, TWL4030_PMBR1_REG);
 +   if (ret  0) {
 +   dev_err(chip-dev, %s: Failed to read PMBR1\n, pwm-label);
 +   return ret;
 +   }
 +
 +   if (pwm-hwpwm) {
 +   /* PWM 1 */
 +   mask = TWL4030_GPIO7_VIBRASYNC_PWM1_MASK;
 +   bits = TWL4030_GPIO7_VIBRASYNC_PWM1_PWM1;
 +   } else {
 +   /* PWM 0 */
 +   mask = TWL4030_GPIO6_PWM0_MUTE_MASK;
 +   bits = TWL4030_GPIO6_PWM0_MUTE_PWM0;
 +   }
 +
 +   /* Save the current MUX configuration for the PWM */
 +   twl-twl4030_pwm_mux = ~mask;
 +   twl-twl4030_pwm_mux |= (val  mask);
 
 Do we really need this mask clearing here? After probe twl4030_pwm_mux
 should be zero, and if twl4030_pwm_request is called twice you don't
 clear the important bits before |=, I think 'twl4030_pwm_mux = val 
 mask' would be better here.

I'm storing both PWM's state in the same variable, but in different offsets:
PWM0: bits 2-3
PWM1: bits 4-5
Probably it is over engineering to clear the relevant bits in the backup
storage, but better to be safe IMHO.
I would leave this part as it is.

-- 
Péter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

[PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

[PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

Re: [PATCH 2/3] pwm: New driver to support PWMs on TWL4030/6030 series of PMICs

8 matches

Site Navigation

Mail list logo

Footer information