[1/7] pwm: jz4740: Obtain regmap from parent node
diff mbox series

Message ID 20190809123031.24219-2-paul@crapouillou.net
State New
Headers show
Series
  • pwm: jz4740: Driver update
Related show

Commit Message

Paul Cercueil Aug. 9, 2019, 12:30 p.m. UTC
The TCU registers are shared between a handful of drivers, accessing
them through the same regmap.

While this driver is devicetree-compatible, it is never (as of now)
probed from devicetree, so this change does not introduce a ABI problem
with current devicetree files.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---
 drivers/pwm/Kconfig      |  1 +
 drivers/pwm/pwm-jz4740.c | 80 ++++++++++++++++++++++++++--------------
 2 files changed, 53 insertions(+), 28 deletions(-)

Comments

Uwe Kleine-König Aug. 9, 2019, 4:51 p.m. UTC | #1
On Fri, Aug 09, 2019 at 02:30:25PM +0200, Paul Cercueil wrote:
> The TCU registers are shared between a handful of drivers, accessing
> them through the same regmap.
> 
> While this driver is devicetree-compatible, it is never (as of now)
> probed from devicetree, so this change does not introduce a ABI problem
> with current devicetree files.

If you adapt the binding also update the binding doc please.

> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Mathieu Malaterre <malat@debian.org>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>

nitpick: put your S-o-b line after the other tags you added.

> ---
>  drivers/pwm/Kconfig      |  1 +
>  drivers/pwm/pwm-jz4740.c | 80 ++++++++++++++++++++++++++--------------
>  2 files changed, 53 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index a7e57516959e..cc4df0ec978a 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -225,6 +225,7 @@ config PWM_IMX_TPM
>  config PWM_JZ4740
>  	tristate "Ingenic JZ47xx PWM support"
>  	depends on MACH_INGENIC
> +	select MFD_SYSCON
>  	help
>  	  Generic PWM framework driver for Ingenic JZ47xx based
>  	  machines.
> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> index f901e8a0d33d..7aea5e0c6e18 100644
> --- a/drivers/pwm/pwm-jz4740.c
> +++ b/drivers/pwm/pwm-jz4740.c
> @@ -8,18 +8,20 @@
>  #include <linux/err.h>
>  #include <linux/gpio.h>
>  #include <linux/kernel.h>
> +#include <linux/mfd/ingenic-tcu.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/pwm.h>
> -
> -#include <asm/mach-jz4740/timer.h>
> +#include <linux/regmap.h>
>  
>  #define NUM_PWM 8
>  
>  struct jz4740_pwm_chip {
>  	struct pwm_chip chip;
>  	struct clk *clk;
> +	struct regmap *map;
>  };
>  
>  static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
> @@ -29,6 +31,8 @@ static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
>  
>  static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
>  {
> +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
> +
>  	/*
>  	 * Timers 0 and 1 are used for system tasks, so they are unavailable
>  	 * for use as PWMs.
> @@ -36,50 +40,53 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
>  	if (pwm->hwpwm < 2)
>  		return -EBUSY;
>  
> -	jz4740_timer_start(pwm->hwpwm);
> +	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));

jz4740_timer_start does

	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_CLEAR);

with

#define JZ_REG_TIMER_STOP_CLEAR         0x2C

and

#define TCU_REG_TSCR            0x3c

I wonder why the offsets are different.

>  
>  	return 0;
>  }
>  
>  static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
>  {
> -	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
> +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
>  
> -	jz4740_timer_stop(pwm->hwpwm);
> +	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));

jz4740_timer_set_ctrl writes to offset (((pwm->hwpwm) * 0x10) + 0x3C)
and jz4740_timer_stop to offset 0x1c. The regmap_write doesn't do both
of them but instead writes to offset 0x3c.

So this doesn't seem to be a 1:1 conversion. This either needs fixing,
splitting into several patches or a better commit log.

Stopping my review here.

Best regards
Uwe
Paul Cercueil Aug. 9, 2019, 5:04 p.m. UTC | #2
Hi Uwe,


Le ven. 9 août 2019 à 18:51, Uwe =?iso-8859-1?q?Kleine-K=F6nig?= 
<u.kleine-koenig@pengutronix.de> a écrit :
> On Fri, Aug 09, 2019 at 02:30:25PM +0200, Paul Cercueil wrote:
>>  The TCU registers are shared between a handful of drivers, accessing
>>  them through the same regmap.
>> 
>>  While this driver is devicetree-compatible, it is never (as of now)
>>  probed from devicetree, so this change does not introduce a ABI 
>> problem
>>  with current devicetree files.
> 
> If you adapt the binding also update the binding doc please.

It's already updated, in mips-next, so it'll be in the next -rc1.


>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  Tested-by: Mathieu Malaterre <malat@debian.org>
>>  Tested-by: Artur Rojek <contact@artur-rojek.eu>
> 
> nitpick: put your S-o-b line after the other tags you added.
> 
>>  ---
>>   drivers/pwm/Kconfig      |  1 +
>>   drivers/pwm/pwm-jz4740.c | 80 
>> ++++++++++++++++++++++++++--------------
>>   2 files changed, 53 insertions(+), 28 deletions(-)
>> 
>>  diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>>  index a7e57516959e..cc4df0ec978a 100644
>>  --- a/drivers/pwm/Kconfig
>>  +++ b/drivers/pwm/Kconfig
>>  @@ -225,6 +225,7 @@ config PWM_IMX_TPM
>>   config PWM_JZ4740
>>   	tristate "Ingenic JZ47xx PWM support"
>>   	depends on MACH_INGENIC
>>  +	select MFD_SYSCON
>>   	help
>>   	  Generic PWM framework driver for Ingenic JZ47xx based
>>   	  machines.
>>  diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
>>  index f901e8a0d33d..7aea5e0c6e18 100644
>>  --- a/drivers/pwm/pwm-jz4740.c
>>  +++ b/drivers/pwm/pwm-jz4740.c
>>  @@ -8,18 +8,20 @@
>>   #include <linux/err.h>
>>   #include <linux/gpio.h>
>>   #include <linux/kernel.h>
>>  +#include <linux/mfd/ingenic-tcu.h>
>>  +#include <linux/mfd/syscon.h>
>>   #include <linux/module.h>
>>   #include <linux/of_device.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/pwm.h>
>>  -
>>  -#include <asm/mach-jz4740/timer.h>
>>  +#include <linux/regmap.h>
>> 
>>   #define NUM_PWM 8
>> 
>>   struct jz4740_pwm_chip {
>>   	struct pwm_chip chip;
>>   	struct clk *clk;
>>  +	struct regmap *map;
>>   };
>> 
>>   static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip 
>> *chip)
>>  @@ -29,6 +31,8 @@ static inline struct jz4740_pwm_chip 
>> *to_jz4740(struct pwm_chip *chip)
>> 
>>   static int jz4740_pwm_request(struct pwm_chip *chip, struct 
>> pwm_device *pwm)
>>   {
>>  +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
>>  +
>>   	/*
>>   	 * Timers 0 and 1 are used for system tasks, so they are 
>> unavailable
>>   	 * for use as PWMs.
>>  @@ -36,50 +40,53 @@ static int jz4740_pwm_request(struct pwm_chip 
>> *chip, struct pwm_device *pwm)
>>   	if (pwm->hwpwm < 2)
>>   		return -EBUSY;
>> 
>>  -	jz4740_timer_start(pwm->hwpwm);
>>  +	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
> 
> jz4740_timer_start does
> 
> 	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_CLEAR);
> 
> with
> 
> #define JZ_REG_TIMER_STOP_CLEAR         0x2C
> 
> and
> 
> #define TCU_REG_TSCR            0x3c
> 
> I wonder why the offsets are different.

The offset are different because the base is different.


>> 
>>   	return 0;
>>   }
>> 
>>   static void jz4740_pwm_free(struct pwm_chip *chip, struct 
>> pwm_device *pwm)
>>   {
>>  -	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
>>  +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
>> 
>>  -	jz4740_timer_stop(pwm->hwpwm);
>>  +	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
> 
> jz4740_timer_set_ctrl writes to offset (((pwm->hwpwm) * 0x10) + 0x3C)
> and jz4740_timer_stop to offset 0x1c. The regmap_write doesn't do both
> of them but instead writes to offset 0x3c.

I guess it should have been TCU_REG_TSSR ("Timer Stop Set Register") and
I didn't notice because the next patch drops this write anyway.

I'll do as you suggested in your other reply and swap the two patches if
it makes things easier, it'll get rid of this write.


> So this doesn't seem to be a 1:1 conversion. This either needs fixing,
> splitting into several patches or a better commit log.
> 
> Stopping my review here.
> 
> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König        
>     |
> Industrial Linux Solutions                 | 
> http://www.pengutronix.de/  |
Uwe Kleine-König Aug. 12, 2019, 6:18 a.m. UTC | #3
Hello Paul,

On Fri, Aug 09, 2019 at 07:04:56PM +0200, Paul Cercueil wrote:
> Le ven. 9 août 2019 à 18:51, Uwe =?iso-8859-1?q?Kleine-K=F6nig?=
> <u.kleine-koenig@pengutronix.de> a écrit :
> > On Fri, Aug 09, 2019 at 02:30:25PM +0200, Paul Cercueil wrote:
> > >  The TCU registers are shared between a handful of drivers, accessing
> > >  them through the same regmap.
> > > 
> > >  While this driver is devicetree-compatible, it is never (as of now)
> > >  probed from devicetree, so this change does not introduce a ABI
> > > problem
> > >  with current devicetree files.
> > 
> > If you adapt the binding also update the binding doc please.
> 
> It's already updated, in mips-next, so it'll be in the next -rc1.

This is a useful information for the commit log.

> > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > >  Tested-by: Mathieu Malaterre <malat@debian.org>
> > >  Tested-by: Artur Rojek <contact@artur-rojek.eu>
> > 
> > nitpick: put your S-o-b line after the other tags you added.
> > 
> > >  ---
> > >   drivers/pwm/Kconfig      |  1 +
> > >   drivers/pwm/pwm-jz4740.c | 80 ++++++++++++++++++++++++++--------------
> > >   2 files changed, 53 insertions(+), 28 deletions(-)
> > > 
> > >  diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > >  index a7e57516959e..cc4df0ec978a 100644
> > >  --- a/drivers/pwm/Kconfig
> > >  +++ b/drivers/pwm/Kconfig
> > >  @@ -225,6 +225,7 @@ config PWM_IMX_TPM
> > >   config PWM_JZ4740
> > >   	tristate "Ingenic JZ47xx PWM support"
> > >   	depends on MACH_INGENIC
> > >  +	select MFD_SYSCON
> > >   	help
> > >   	  Generic PWM framework driver for Ingenic JZ47xx based
> > >   	  machines.
> > >  diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> > >  index f901e8a0d33d..7aea5e0c6e18 100644
> > >  --- a/drivers/pwm/pwm-jz4740.c
> > >  +++ b/drivers/pwm/pwm-jz4740.c
> > >  @@ -8,18 +8,20 @@
> > >   #include <linux/err.h>
> > >   #include <linux/gpio.h>
> > >   #include <linux/kernel.h>
> > >  +#include <linux/mfd/ingenic-tcu.h>
> > >  +#include <linux/mfd/syscon.h>
> > >   #include <linux/module.h>
> > >   #include <linux/of_device.h>
> > >   #include <linux/platform_device.h>
> > >   #include <linux/pwm.h>
> > >  -
> > >  -#include <asm/mach-jz4740/timer.h>
> > >  +#include <linux/regmap.h>
> > > 
> > >   #define NUM_PWM 8
> > > 
> > >   struct jz4740_pwm_chip {
> > >   	struct pwm_chip chip;
> > >   	struct clk *clk;
> > >  +	struct regmap *map;
> > >   };
> > > 
> > >   static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
> > >  @@ -29,6 +31,8 @@ static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
> > > 
> > >   static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> > >   {
> > >  +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
> > >  +
> > >   	/*
> > >   	 * Timers 0 and 1 are used for system tasks, so they are unavailable
> > >   	 * for use as PWMs.
> > >  @@ -36,50 +40,53 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> > >   	if (pwm->hwpwm < 2)
> > >   		return -EBUSY;
> > > 
> > >  -	jz4740_timer_start(pwm->hwpwm);
> > >  +	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
> > 
> > jz4740_timer_start does
> > 
> > 	writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_CLEAR);
> > 
> > with
> > 
> > #define JZ_REG_TIMER_STOP_CLEAR         0x2C
> > 
> > and
> > 
> > #define TCU_REG_TSCR            0x3c
> > 
> > I wonder why the offsets are different.
> 
> The offset are different because the base is different.

This is a useful information for the commit log.
 
> > > 
> > >   	return 0;
> > >   }
> > > 
> > >   static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > >   {
> > >  -	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
> > >  +	struct jz4740_pwm_chip *jz = to_jz4740(chip);
> > > 
> > >  -	jz4740_timer_stop(pwm->hwpwm);
> > >  +	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
> > 
> > jz4740_timer_set_ctrl writes to offset (((pwm->hwpwm) * 0x10) + 0x3C)
> > and jz4740_timer_stop to offset 0x1c. The regmap_write doesn't do both
> > of them but instead writes to offset 0x3c.
> 
> I guess it should have been TCU_REG_TSSR ("Timer Stop Set Register") and
> I didn't notice because the next patch drops this write anyway.
> 
> I'll do as you suggested in your other reply and swap the two patches if
> it makes things easier, it'll get rid of this write.

ok.

Best regards
Uwe

Patch
diff mbox series

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index a7e57516959e..cc4df0ec978a 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -225,6 +225,7 @@  config PWM_IMX_TPM
 config PWM_JZ4740
 	tristate "Ingenic JZ47xx PWM support"
 	depends on MACH_INGENIC
+	select MFD_SYSCON
 	help
 	  Generic PWM framework driver for Ingenic JZ47xx based
 	  machines.
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index f901e8a0d33d..7aea5e0c6e18 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -8,18 +8,20 @@ 
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/kernel.h>
+#include <linux/mfd/ingenic-tcu.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
-
-#include <asm/mach-jz4740/timer.h>
+#include <linux/regmap.h>
 
 #define NUM_PWM 8
 
 struct jz4740_pwm_chip {
 	struct pwm_chip chip;
 	struct clk *clk;
+	struct regmap *map;
 };
 
 static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
@@ -29,6 +31,8 @@  static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
 
 static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
+
 	/*
 	 * Timers 0 and 1 are used for system tasks, so they are unavailable
 	 * for use as PWMs.
@@ -36,50 +40,53 @@  static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (pwm->hwpwm < 2)
 		return -EBUSY;
 
-	jz4740_timer_start(pwm->hwpwm);
+	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
 
 	return 0;
 }
 
 static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
 
-	jz4740_timer_stop(pwm->hwpwm);
+	regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
 }
 
 static int jz4740_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->pwm);
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
+
+	/* Enable PWM output */
+	regmap_update_bits(jz->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PWM_EN, TCU_TCSR_PWM_EN);
 
-	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
-	jz4740_timer_enable(pwm->hwpwm);
+	/* Start counter */
+	regmap_write(jz->map, TCU_REG_TESR, BIT(pwm->hwpwm));
 
 	return 0;
 }
 
 static void jz4740_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->hwpwm);
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
 
 	/*
 	 * Set duty > period. This trick allows the TCU channels in TCU2 mode to
 	 * properly return to their init level.
 	 */
-	jz4740_timer_set_duty(pwm->hwpwm, 0xffff);
-	jz4740_timer_set_period(pwm->hwpwm, 0x0);
+	regmap_write(jz->map, TCU_REG_TDHRc(pwm->hwpwm), 0xffff);
+	regmap_write(jz->map, TCU_REG_TDFRc(pwm->hwpwm), 0x0);
 
 	/*
 	 * Disable PWM output.
 	 * In TCU2 mode (channel 1/2 on JZ4750+), this must be done before the
 	 * counter is stopped, while in TCU1 mode the order does not matter.
 	 */
-	ctrl &= ~JZ_TIMER_CTRL_PWM_ENABLE;
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+	regmap_update_bits(jz->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PWM_EN, 0);
 
 	/* Stop counter */
-	jz4740_timer_disable(pwm->hwpwm);
+	regmap_write(jz->map, TCU_REG_TECR, BIT(pwm->hwpwm));
 }
 
 static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -89,7 +96,6 @@  static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	unsigned long long tmp;
 	unsigned long period, duty;
 	unsigned int prescaler = 0;
-	uint16_t ctrl;
 
 	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * state->period;
 	do_div(tmp, 1000000000);
@@ -112,26 +118,37 @@  static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	jz4740_pwm_disable(chip, pwm);
 
-	jz4740_timer_set_count(pwm->hwpwm, 0);
-	jz4740_timer_set_duty(pwm->hwpwm, duty);
-	jz4740_timer_set_period(pwm->hwpwm, period);
+	/* Set abrupt shutdown */
+	regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PWM_SD, TCU_TCSR_PWM_SD);
+
+	/* Set clock prescale */
+	regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PRESCALE_MASK,
+			   prescaler << TCU_TCSR_PRESCALE_LSB);
+
+	/* Reset counter to 0 */
+	regmap_write(jz4740->map, TCU_REG_TCNTc(pwm->hwpwm), 0);
 
-	ctrl = JZ_TIMER_CTRL_PRESCALER(prescaler) | JZ_TIMER_CTRL_SRC_EXT |
-		JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
+	/* Set duty */
+	regmap_write(jz4740->map, TCU_REG_TDHRc(pwm->hwpwm), duty);
 
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+	/* Set period */
+	regmap_write(jz4740->map, TCU_REG_TDFRc(pwm->hwpwm), period);
 
+	/* Set polarity */
 	switch (state->polarity) {
 	case PWM_POLARITY_NORMAL:
-		ctrl &= ~JZ_TIMER_CTRL_PWM_ACTIVE_LOW;
+		regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+				   TCU_TCSR_PWM_INITL_HIGH, 0);
 		break;
 	case PWM_POLARITY_INVERSED:
-		ctrl |= JZ_TIMER_CTRL_PWM_ACTIVE_LOW;
+		regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+				   TCU_TCSR_PWM_INITL_HIGH,
+				   TCU_TCSR_PWM_INITL_HIGH);
 		break;
 	}
 
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
-
 	if (state->enabled)
 		jz4740_pwm_enable(chip, pwm);
 
@@ -148,8 +165,9 @@  static const struct pwm_ops jz4740_pwm_ops = {
 static int jz4740_pwm_probe(struct platform_device *pdev)
 {
 	struct jz4740_pwm_chip *jz4740;
+	struct device *dev = &pdev->dev;
 
-	jz4740 = devm_kzalloc(&pdev->dev, sizeof(*jz4740), GFP_KERNEL);
+	jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
 	if (!jz4740)
 		return -ENOMEM;
 
@@ -157,7 +175,13 @@  static int jz4740_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(jz4740->clk))
 		return PTR_ERR(jz4740->clk);
 
-	jz4740->chip.dev = &pdev->dev;
+	jz4740->map = device_node_to_regmap(dev->parent->of_node);
+	if (!jz4740->map) {
+		dev_err(dev, "regmap not found\n");
+		return -EINVAL;
+	}
+
+	jz4740->chip.dev = dev;
 	jz4740->chip.ops = &jz4740_pwm_ops;
 	jz4740->chip.npwm = NUM_PWM;
 	jz4740->chip.base = -1;