Re: [PATCH v8 11/26] pwm: jz4740: Use regmap and clocks from TCU driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Le jeu. 13 déc. 2018 à 10:30, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> a écrit :
Hello,

On Wed, Dec 12, 2018 at 11:09:06PM +0100, Paul Cercueil wrote:
 [...]
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);

 -	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
 -	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
 -	jz4740_timer_enable(pwm->hwpwm);
 +	/* Enable PWM output */
 +	regmap_update_bits(jz->map, TCU_REG_TCSRc(pwm->hwpwm),
 +				TCU_TCSR_PWM_EN, TCU_TCSR_PWM_EN);

Usually follow-up lines are indented to the matching parenthesis.

OK.

 [...]
static int jz4740_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
  			     int duty_ns, int period_ns)
  {
  	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
 +	struct clk *clk = jz4740->clks[pwm->hwpwm];
 +	unsigned long rate, new_rate, period, duty;
  	unsigned long long tmp;
 -	unsigned long period, duty;
 -	unsigned int prescaler = 0;
 -	uint16_t ctrl;
 +	unsigned int tcsr;
  	bool is_enabled;

 -	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * period_ns;
 -	do_div(tmp, 1000000000);
 -	period = tmp;
 +	rate = clk_get_rate(clk);
 +
 +	for (;;) {
 +		tmp = (unsigned long long) rate * period_ns;
 +		do_div(tmp, 1000000000);

 -	while (period > 0xffff && prescaler < 6) {
 -		period >>= 2;
 -		++prescaler;
 +		if (tmp <= 0xffff)
 +			break;
 +
 +		new_rate = clk_round_rate(clk, rate / 2);
 +
 +		if (new_rate < rate)
 +			rate = new_rate;
 +		else
 +			return -EINVAL;
  	}

 -	if (prescaler == 6)
 -		return -EINVAL;
 +	clk_set_rate(clk, rate);

Maybe this could better live in a separate patch. If you split still
further to have the conversion to regmap in a single patch, then the
conversion to the clk_* functions and then improve the algorithm for the clk settings each of the patches is easier to review than this one patch
that does all three things at once.

I can try.

Best regards
Uwe

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





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux