[5/6] pwm: sun4i: Add support to output source clock directly
diff mbox series

Message ID 20190726184045.14669-6-jernej.skrabec@siol.net
State New
Headers show
Series
  • pwm: sun4i: Add support for Allwinner H6
Related show

Commit Message

Jernej Skrabec July 26, 2019, 6:40 p.m. UTC
PWM core has an option to bypass whole logic and output unchanged source
clock as PWM output. This is achieved by enabling bypass bit.

Note that when bypass is enabled, no other setting has any meaning, not
even enable bit.

This mode of operation is needed to achieve high enough frequency to
serve as clock source for AC200 chip, which is integrated into same
package as H6 SoC.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/pwm/pwm-sun4i.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

Comments

Maxime Ripard July 27, 2019, 10:50 a.m. UTC | #1
On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> PWM core has an option to bypass whole logic and output unchanged source
> clock as PWM output. This is achieved by enabling bypass bit.
>
> Note that when bypass is enabled, no other setting has any meaning, not
> even enable bit.
>
> This mode of operation is needed to achieve high enough frequency to
> serve as clock source for AC200 chip, which is integrated into same
> package as H6 SoC.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

It doesn't seem to be available on the A10 (at least) though. The A13
seem to have it, so you should probably check that, and make that
conditional to the compatible if not available on all of them.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Jernej Skrabec July 27, 2019, 2:28 p.m. UTC | #2
Dne sobota, 27. julij 2019 ob 12:50:08 CEST je Maxime Ripard napisal(a):
> On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > PWM core has an option to bypass whole logic and output unchanged source
> > clock as PWM output. This is achieved by enabling bypass bit.
> > 
> > Note that when bypass is enabled, no other setting has any meaning, not
> > even enable bit.
> > 
> > This mode of operation is needed to achieve high enough frequency to
> > serve as clock source for AC200 chip, which is integrated into same
> > package as H6 SoC.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> It doesn't seem to be available on the A10 (at least) though. The A13
> seem to have it, so you should probably check that, and make that
> conditional to the compatible if not available on all of them.

Ok, can you suggest the name for the quirk? "has_bypass" is suspiciously 
similar to "has_prescaler_bypass".

Also, how to name these sun4i_pwm_data structures? Now that there are (will 
be) three new quirks, name of the structure would be just too long, like 
"sun50i_pwm_dual_prescaler_bypass_clk_rst_bypass". 

Best regards,
Jernej

> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Chen-Yu Tsai July 27, 2019, 2:54 p.m. UTC | #3
On Sat, Jul 27, 2019 at 10:28 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Dne sobota, 27. julij 2019 ob 12:50:08 CEST je Maxime Ripard napisal(a):
> > On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > > PWM core has an option to bypass whole logic and output unchanged source
> > > clock as PWM output. This is achieved by enabling bypass bit.
> > >
> > > Note that when bypass is enabled, no other setting has any meaning, not
> > > even enable bit.
> > >
> > > This mode of operation is needed to achieve high enough frequency to
> > > serve as clock source for AC200 chip, which is integrated into same
> > > package as H6 SoC.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > It doesn't seem to be available on the A10 (at least) though. The A13
> > seem to have it, so you should probably check that, and make that
> > conditional to the compatible if not available on all of them.
>
> Ok, can you suggest the name for the quirk? "has_bypass" is suspiciously
> similar to "has_prescaler_bypass".

has_direct_mod_clk_output?

> Also, how to name these sun4i_pwm_data structures? Now that there are (will
> be) three new quirks, name of the structure would be just too long, like
> "sun50i_pwm_dual_prescaler_bypass_clk_rst_bypass".

Just use the SoC model. Any later ones that have the same quirks will likely
use the same compatible string anyway.

ChenYu
Uwe Kleine-König July 29, 2019, 7:06 a.m. UTC | #4
On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> PWM core has an option to bypass whole logic and output unchanged source
> clock as PWM output. This is achieved by enabling bypass bit.
> 
> Note that when bypass is enabled, no other setting has any meaning, not
> even enable bit.
> 
> This mode of operation is needed to achieve high enough frequency to
> serve as clock source for AC200 chip, which is integrated into same
> package as H6 SoC.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/pwm/pwm-sun4i.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 9e0eca79ff88..848cff26f385 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -120,6 +120,19 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
>  
>  	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
>  
> +	/*
> +	 * PWM chapter in H6 manual has a diagram which explains that if bypass
> +	 * bit is set, no other setting has any meaning. Even more, experiment
> +	 * proved that also enable bit is ignored in this case.
> +	 */
> +	if (val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) {
> +		state->period = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, clk_rate);
> +		state->duty_cycle = state->period / 2;
> +		state->polarity = PWM_POLARITY_NORMAL;
> +		state->enabled = true;
> +		return;
> +	}
> +
>  	if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
>  	    sun4i_pwm->data->has_prescaler_bypass)
>  		prescaler = 1;
> @@ -211,7 +224,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  {
>  	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
>  	struct pwm_state cstate;
> -	u32 ctrl;
> +	u32 ctrl, clk_rate;
> +	bool bypass;
>  	int ret;
>  	unsigned int delay_us;
>  	unsigned long now;
> @@ -226,6 +240,16 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  		}
>  	}
>  
> +	/*
> +	 * Although it would make much more sense to check for bypass in
> +	 * sun4i_pwm_calculate(), value of bypass bit also depends on "enabled".
> +	 * Period is allowed to be rounded up or down.
> +	 */

Every driver seems to implement rounding the way its driver considers it
sensible. @Thierry: This is another patch where it would be good to have
a global directive about how rounding is supposed to work to provide the
users an reliable and uniform way to work with PWMs.

> +	clk_rate = clk_get_rate(sun4i_pwm->clk);
> +	bypass = (state->period == NSEC_PER_SEC / clk_rate ||
> +		 state->period == DIV_ROUND_UP(NSEC_PER_SEC, clk_rate)) &&
> +		 state->enabled;

Not sure if the compiler is clever enough to notice the obvious
optimisation with this code construct, but you can write this test in a
more clever way which has zero instead of up to two divisions. Something
like:

bypass = ((state->period * clk_rate >= NSEC_PER_SEC &&
	   state->period * clk_rate < NSEC_PER_SEC + clk_rate) &&
	  state->enabled);

In the commit log you write the motivation for using bypass is that it
allows to implement higher frequency then with the "normal" operation.
As you don't skip calculating the normal parameters requesting such a
high-frequency setting either errors out or doesn't catch the impossible
request. In both cases there is something to fix.

> +
>  	spin_lock(&sun4i_pwm->ctrl_lock);
>  	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
>  
> @@ -273,6 +297,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  		ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
>  	}
>  
> +	if (bypass)
> +		ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
> +	else
> +		ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm);
> +

Does switching on (or off) the bypass bit complete the currently running
period?

>  	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
>  
>  	spin_unlock(&sun4i_pwm->ctrl_lock);

Best regards
Uwe
Jernej Skrabec July 29, 2019, 4:16 p.m. UTC | #5
Hi Uwe,

Dne ponedeljek, 29. julij 2019 ob 09:06:05 CEST je Uwe Kleine-König 
napisal(a):
> On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > PWM core has an option to bypass whole logic and output unchanged source
> > clock as PWM output. This is achieved by enabling bypass bit.
> > 
> > Note that when bypass is enabled, no other setting has any meaning, not
> > even enable bit.
> > 
> > This mode of operation is needed to achieve high enough frequency to
> > serve as clock source for AC200 chip, which is integrated into same
> > package as H6 SoC.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/pwm/pwm-sun4i.c | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 9e0eca79ff88..848cff26f385 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -120,6 +120,19 @@ static void sun4i_pwm_get_state(struct pwm_chip
> > *chip,
> > 
> >  	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > 
> > +	/*
> > +	 * PWM chapter in H6 manual has a diagram which explains that if 
bypass
> > +	 * bit is set, no other setting has any meaning. Even more, 
experiment
> > +	 * proved that also enable bit is ignored in this case.
> > +	 */
> > +	if (val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) {
> > +		state->period = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, 
clk_rate);
> > +		state->duty_cycle = state->period / 2;
> > +		state->polarity = PWM_POLARITY_NORMAL;
> > +		state->enabled = true;
> > +		return;
> > +	}
> > +
> > 
> >  	if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
> >  	
> >  	    sun4i_pwm->data->has_prescaler_bypass)
> >  		
> >  		prescaler = 1;
> > 
> > @@ -211,7 +224,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,> 
> >  {
> >  
> >  	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> >  	struct pwm_state cstate;
> > 
> > -	u32 ctrl;
> > +	u32 ctrl, clk_rate;
> > +	bool bypass;
> > 
> >  	int ret;
> >  	unsigned int delay_us;
> >  	unsigned long now;
> > 
> > @@ -226,6 +240,16 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,> 
> >  		}
> >  	
> >  	}
> > 
> > +	/*
> > +	 * Although it would make much more sense to check for bypass in
> > +	 * sun4i_pwm_calculate(), value of bypass bit also depends on 
"enabled".
> > +	 * Period is allowed to be rounded up or down.
> > +	 */
> 
> Every driver seems to implement rounding the way its driver considers it
> sensible. @Thierry: This is another patch where it would be good to have
> a global directive about how rounding is supposed to work to provide the
> users an reliable and uniform way to work with PWMs.
> 
> > +	clk_rate = clk_get_rate(sun4i_pwm->clk);
> > +	bypass = (state->period == NSEC_PER_SEC / clk_rate ||
> > +		 state->period == DIV_ROUND_UP(NSEC_PER_SEC, clk_rate)) 
&&
> > +		 state->enabled;
> 
> Not sure if the compiler is clever enough to notice the obvious
> optimisation with this code construct, but you can write this test in a
> more clever way which has zero instead of up to two divisions. Something
> like:
> 
> bypass = ((state->period * clk_rate >= NSEC_PER_SEC &&
> 	   state->period * clk_rate < NSEC_PER_SEC + clk_rate) &&
> 	  state->enabled);
> 
> In the commit log you write the motivation for using bypass is that it
> allows to implement higher frequency then with the "normal" operation.
> As you don't skip calculating the normal parameters requesting such a
> high-frequency setting either errors out or doesn't catch the impossible
> request. In both cases there is something to fix.

It's the latter, otherwise it wouldn't work for my case. I'll fix the check and 
skip additional logic.

> 
> > +
> > 
> >  	spin_lock(&sun4i_pwm->ctrl_lock);
> >  	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > 
> > @@ -273,6 +297,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > struct pwm_device *pwm,> 
> >  		ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> >  	
> >  	}
> > 
> > +	if (bypass)
> > +		ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > +	else
> > +		ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > +
> 
> Does switching on (or off) the bypass bit complete the currently running
> period?

I don't really know. If I understand correctly, it just bypasses PWM logic 
completely, so I would say it doesn't complete the currently running period.

Take a look at chapter 3.9.2 http://linux-sunxi.org/
File:Allwinner_H6_V200_User_Manual_V1.1.pdf

Best regards,
Jernej

> 
> >  	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
> >  	
> >  	spin_unlock(&sun4i_pwm->ctrl_lock);
> 
> Best regards
> Uwe
Uwe Kleine-König July 29, 2019, 4:29 p.m. UTC | #6
Hello,

On Mon, Jul 29, 2019 at 06:16:55PM +0200, Jernej Škrabec wrote:
> Dne ponedeljek, 29. julij 2019 ob 09:06:05 CEST je Uwe Kleine-König 
> napisal(a):
> > On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > > PWM core has an option to bypass whole logic and output unchanged source
> > > clock as PWM output. This is achieved by enabling bypass bit.
> > > 
> > > Note that when bypass is enabled, no other setting has any meaning, not
> > > even enable bit.
> > > 
> > > This mode of operation is needed to achieve high enough frequency to
> > > serve as clock source for AC200 chip, which is integrated into same
> > > package as H6 SoC.
> > > 
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > > 
> > >  drivers/pwm/pwm-sun4i.c | 31 ++++++++++++++++++++++++++++++-
> > >  1 file changed, 30 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > index 9e0eca79ff88..848cff26f385 100644
> > > --- a/drivers/pwm/pwm-sun4i.c
> > > +++ b/drivers/pwm/pwm-sun4i.c
> > > @@ -120,6 +120,19 @@ static void sun4i_pwm_get_state(struct pwm_chip
> > > *chip,
> > > 
> > >  	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > > 
> > > +	/*
> > > +	 * PWM chapter in H6 manual has a diagram which explains that if bypass
> > > +	 * bit is set, no other setting has any meaning. Even more, experiment
> > > +	 * proved that also enable bit is ignored in this case.
> > > +	 */
> > > +	if (val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) {
> > > +		state->period = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, clk_rate);
> > > +		state->duty_cycle = state->period / 2;
> > > +		state->polarity = PWM_POLARITY_NORMAL;
> > > +		state->enabled = true;
> > > +		return;
> > > +	}
> > > +
> > > 
> > >  	if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
> > >  	
> > >  	    sun4i_pwm->data->has_prescaler_bypass)
> > >  		
> > >  		prescaler = 1;
> > > 
> > > @@ -211,7 +224,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > > struct pwm_device *pwm,> 
> > >  {
> > >  
> > >  	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> > >  	struct pwm_state cstate;
> > > 
> > > -	u32 ctrl;
> > > +	u32 ctrl, clk_rate;
> > > +	bool bypass;
> > > 
> > >  	int ret;
> > >  	unsigned int delay_us;
> > >  	unsigned long now;
> > > 
> > > @@ -226,6 +240,16 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > > struct pwm_device *pwm,> 
> > >  		}
> > >  	
> > >  	}
> > > 
> > > +	/*
> > > +	 * Although it would make much more sense to check for bypass in
> > > +	 * sun4i_pwm_calculate(), value of bypass bit also depends on "enabled".
> > > +	 * Period is allowed to be rounded up or down.
> > > +	 */
> > 
> > Every driver seems to implement rounding the way its driver considers it
> > sensible. @Thierry: This is another patch where it would be good to have
> > a global directive about how rounding is supposed to work to provide the
> > users an reliable and uniform way to work with PWMs.
> > 
> > > +	clk_rate = clk_get_rate(sun4i_pwm->clk);
> > > +	bypass = (state->period == NSEC_PER_SEC / clk_rate ||
> > > +		 state->period == DIV_ROUND_UP(NSEC_PER_SEC, clk_rate)) &&
> > > +		 state->enabled;
> > 
> > Not sure if the compiler is clever enough to notice the obvious
> > optimisation with this code construct, but you can write this test in a
> > more clever way which has zero instead of up to two divisions. Something
> > like:
> > 
> > bypass = ((state->period * clk_rate >= NSEC_PER_SEC &&
> > 	   state->period * clk_rate < NSEC_PER_SEC + clk_rate) &&
> > 	  state->enabled);
> > 
> > In the commit log you write the motivation for using bypass is that it
> > allows to implement higher frequency then with the "normal" operation.
> > As you don't skip calculating the normal parameters requesting such a
> > high-frequency setting either errors out or doesn't catch the impossible
> > request. In both cases there is something to fix.
> 
> It's the latter, otherwise it wouldn't work for my case. I'll fix the check and 
> skip additional logic.

Great.

> > > +
> > > 
> > >  	spin_lock(&sun4i_pwm->ctrl_lock);
> > >  	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > > 
> > > @@ -273,6 +297,11 @@ static int sun4i_pwm_apply(struct pwm_chip *chip,
> > > struct pwm_device *pwm,> 
> > >  		ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
> > >  	
> > >  	}
> > > 
> > > +	if (bypass)
> > > +		ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > > +	else
> > > +		ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm);
> > > +
> > 
> > Does switching on (or off) the bypass bit complete the currently running
> > period?
> 
> I don't really know. If I understand correctly, it just bypasses PWM logic 
> completely, so I would say it doesn't complete the currently running period.

This is a bug. It's part of the promise of the PWM API that started
periods are completed. Please at least document this limitation at the
top of the driver. drivers/pwm/pwm-sifive.c has an example you might
want to use as a template.

Best regards
Uwe

Patch
diff mbox series

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 9e0eca79ff88..848cff26f385 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -120,6 +120,19 @@  static void sun4i_pwm_get_state(struct pwm_chip *chip,
 
 	val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
 
+	/*
+	 * PWM chapter in H6 manual has a diagram which explains that if bypass
+	 * bit is set, no other setting has any meaning. Even more, experiment
+	 * proved that also enable bit is ignored in this case.
+	 */
+	if (val & BIT_CH(PWM_BYPASS, pwm->hwpwm)) {
+		state->period = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, clk_rate);
+		state->duty_cycle = state->period / 2;
+		state->polarity = PWM_POLARITY_NORMAL;
+		state->enabled = true;
+		return;
+	}
+
 	if ((PWM_REG_PRESCAL(val, pwm->hwpwm) == PWM_PRESCAL_MASK) &&
 	    sun4i_pwm->data->has_prescaler_bypass)
 		prescaler = 1;
@@ -211,7 +224,8 @@  static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
 	struct pwm_state cstate;
-	u32 ctrl;
+	u32 ctrl, clk_rate;
+	bool bypass;
 	int ret;
 	unsigned int delay_us;
 	unsigned long now;
@@ -226,6 +240,16 @@  static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		}
 	}
 
+	/*
+	 * Although it would make much more sense to check for bypass in
+	 * sun4i_pwm_calculate(), value of bypass bit also depends on "enabled".
+	 * Period is allowed to be rounded up or down.
+	 */
+	clk_rate = clk_get_rate(sun4i_pwm->clk);
+	bypass = (state->period == NSEC_PER_SEC / clk_rate ||
+		 state->period == DIV_ROUND_UP(NSEC_PER_SEC, clk_rate)) &&
+		 state->enabled;
+
 	spin_lock(&sun4i_pwm->ctrl_lock);
 	ctrl = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
 
@@ -273,6 +297,11 @@  static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
 	}
 
+	if (bypass)
+		ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
+	else
+		ctrl &= ~BIT_CH(PWM_BYPASS, pwm->hwpwm);
+
 	sun4i_pwm_writel(sun4i_pwm, ctrl, PWM_CTRL_REG);
 
 	spin_unlock(&sun4i_pwm->ctrl_lock);