Re: [PATCH 1/5] pwm: rcar: add rcar_pwm_calc_counter() to calculate PWMCNT value only

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

Hello,

On Fri, Dec 07, 2018 at 05:29:29PM +0900, Yoshihiro Shimoda wrote:
> -static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
> -				int period_ns)
> +static void rcar_pwm_calc_counter(struct rcar_pwm_chip *rp, int div,
> +				  int duty_ns, int period_ns)
>  {
>  	unsigned long long one_cycle, tmp;	/* 0.01 nanoseconds */
>  	unsigned long clk_rate = clk_get_rate(rp->clk);
> @@ -120,11 +121,17 @@ static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
>  	do_div(tmp, one_cycle);
>  	ph = tmp & RCAR_PWMCNT_PH0_MASK;
>  
> +	rp->pwmcnt = cyc | ph;

Wouldn't it be more natural to let rcar_pwm_calc_counter return cyc | ph
and then ...

> +}
> +
> +static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp)
> +{
>  	/* Avoid prohibited setting */
> -	if (cyc == 0 || ph == 0)
> +	if ((rp->pwmcnt & RCAR_PWMCNT_CYC0_MASK) == 0 ||
> +	    (rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0)
>  		return -EINVAL;
>  
> -	rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> +	rcar_pwm_write(rp, rp->pwmcnt, RCAR_PWMCNT);
>  
>  	return 0;
>  }
> @@ -159,7 +166,8 @@ static int rcar_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>  
>  	rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
>  
> -	ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
> +	rcar_pwm_calc_counter(rp, div, duty_ns, period_ns);
> +	ret = rcar_pwm_set_counter(rp);
>  	if (!ret)
>  		rcar_pwm_set_clock_control(rp, div);

... pass this value to rcar_pwm_set_counter as u32. (Or pass div,
duty_ns and period_ns to rcar_pwm_set_counter and let this then call
rcar_pwm_calc_counter. Then you don't need a new member in the struct
rcar_pwm_chip.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K�nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |