Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1551764

Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2

Show all headers | View raw

On Thu,  5 Jan 2017 00:36:50 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> This commit provides apply() callback implementation for i.MX's PWMv2.
> 
> Suggested-by: Stefan Agner <stefan@agner.ch>
> Suggested-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
> Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Changes for v4:
> - Avoid recalculation of PWM parameters when disabling PWM signal
> - Unconditionally call clk_prepare_enable(imx->clk_per) and
>   clk_disable_unprepare(imx->clk_per)

I don't see that one, but I'm not sure this is actually needed.
In the enabled path we enable the clk before accessing the registers,
and in the disable path, assuming you change the code according to my
suggestion, the clk should already be enabled when you write to
MX3_PWMCR.

> 
> Changes for v3:
> - Remove ipg clock enable/disable functions
> 
> Changes for v2:
> - None
> ---
>  drivers/pwm/pwm-imx.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 60cdc5c..134dd66 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -249,6 +249,72 @@ static int imx_pwm_config(struct pwm_chip *chip,
>  	return ret;
>  }
>  
> +static int imx_pwm_apply_v2(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    struct pwm_state *state)
> +{
> +	unsigned long period_cycles, duty_cycles, prescale;
> +	struct imx_chip *imx = to_imx_chip(chip);
> +	struct pwm_state cstate;
> +	unsigned long long c;
> +	int ret;
> +
> +	pwm_get_state(pwm, &cstate);
> +
> +	if (state->enabled) {
> +		c = clk_get_rate(imx->clk_per);
> +		c *= state->period;
> +
> +		do_div(c, 1000000000);
> +		period_cycles = c;
> +
> +		prescale = period_cycles / 0x10000 + 1;
> +
> +		period_cycles /= prescale;
> +		c = (unsigned long long)period_cycles * state->duty_cycle;
> +		do_div(c, state->period);
> +		duty_cycles = c;
> +
> +		/*
> +		 * according to imx pwm RM, the real period value should be
> +		 * PERIOD value in PWMPR plus 2.
> +		 */
> +		if (period_cycles > 2)
> +			period_cycles -= 2;
> +		else
> +			period_cycles = 0;
> +

Starting here...

> +		ret = clk_prepare_enable(imx->clk_per);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Wait for a free FIFO slot if the PWM is already enabled, and
> +		 * flush the FIFO if the PWM was disabled and is about to be
> +		 * enabled.
> +		 */
> +		if (cstate.enabled)
> +			imx_pwm_wait_fifo_slot(chip, pwm);
> +		else if (state->enabled)
> +			imx_pwm_sw_reset(chip);

... till here, should be replaced by:


		/*
		 * Wait for a free FIFO slot if the PWM is already enabled, and
		 * flush the FIFO if the PWM was disabled and is about to be
		 * enabled.
		 */
		if (cstate.enabled) {
			imx_pwm_wait_fifo_slot(chip, pwm);
		} else {
			ret = clk_prepare_enable(imx->clk_per);
			if (ret)
				return ret;

			imx_pwm_sw_reset(chip);
		}

Otherwise you keep incrementing the prepare/enable counters of the clk
when you change the config of an already enabled PWM.

> +
> +		writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
> +		writel(period_cycles, imx->mmio_base + MX3_PWMPR);
> +
> +		writel(MX3_PWMCR_PRESCALER(prescale) |
> +		       MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
> +		       MX3_PWMCR_DBGEN | MX3_PWMCR_CLKSRC_IPG_HIGH |
> +		       MX3_PWMCR_EN,
> +		       imx->mmio_base + MX3_PWMCR);
> +
> +	} else {

	} else if (cstate.enabled) {

Again, this is to avoid unbalanced prepare/enable counters on the per
clk if the PWM config is changed several times when the PWM is disabled.

> +		writel(0, imx->mmio_base + MX3_PWMCR);
> +
> +		clk_disable_unprepare(imx->clk_per);
> +	}
> +
> +	return 0;
> +}
> +
>  static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>  {
>  	struct imx_chip *imx = to_imx_chip(chip);
> @@ -280,6 +346,7 @@ static struct pwm_ops imx_pwm_ops_v1 = {
>  };
>  
>  static struct pwm_ops imx_pwm_ops_v2 = {
> +	.apply = imx_pwm_apply_v2,
>  	.enable = imx_pwm_enable,
>  	.disable = imx_pwm_disable,
>  	.config = imx_pwm_config,

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-05 00:40 +0100
  Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-01-05 10:00 +0100
    Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-05 10:10 +0100
      Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-01-05 10:30 +0100
        Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-05 10:50 +0100
          Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-01-05 11:10 +0100
            Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Stefan Agner <stefan@agner.ch> - 2017-01-10 04:20 +0100
              Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-01-10 09:00 +0100
        Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX PWMv2 Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-05 22:30 +0100
          Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-01-06 08:10 +0100
        Re: [PATCH v4 07/11] pwm: imx: Provide atomic PWM support for i.MX  PWMv2 Lukasz Majewski <lukma@denx.de> - 2017-01-07 19:40 +0100

csiph-web