RE: [PATCH v6 06/11] pwm: rzg2l-gpt: Convert to waveform callbacks

Biju Das <[email protected]> Mon, 27 Jul 2026 10:12:42 +0000
Newsgroups org.kernel.vger.linux-renesas-soc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pwm
Message-ID <TY3PR01MB11346293B0C51E263E139624F86CC2@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Hello Uwe,

Thanks for the feedback.

> -----Original Message-----
> From: Uwe Kleine-König <[email protected]>
> Sent: 16 July 2026 09:47
> Subject: Re: [PATCH v6 06/11] pwm: rzg2l-gpt: Convert to waveform callbacks
> 
> Hello Biju,
> 
> On Thu, Jun 04, 2026 at 10:56:36AM +0100, Biju wrote:
> > @@ -291,29 +285,84 @@ static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >  	if (rzg2l_gpt->channel_request_count[ch] > 1) {
> >  		u8 sibling_ch = rzg2l_gpt_sibling(pwm->hwpwm);
> >
> > -		if (rzg2l_gpt_is_ch_enabled(rzg2l_gpt, sibling_ch)) {
> > +		if (rzg2l_gpt_is_ch_enabled(rzg2l_gpt, sibling_ch, NULL)) {
> >  			if (period_ticks < rzg2l_gpt->period_ticks[ch])
> > -				return -EBUSY;
> > +				is_small_second_period = true;
> >
> >  			period_ticks = rzg2l_gpt->period_ticks[ch];
> >  		}
> >  	}
> >
> > -	prescale = rzg2l_gpt_calculate_prescale(period_ticks);
> > -	pv = rzg2l_gpt_calculate_pv_or_dc(period_ticks, prescale);
> > +	wfhw->prescale = rzg2l_gpt_calculate_prescale(period_ticks);
> > +	wfhw->gtpr = rzg2l_gpt_calculate_pv_or_dc(period_ticks, wfhw->prescale);
> > +	wfhw->gtccr = 0;
> > +	if (is_small_second_period)
> 
> If is_small_second_period == true, the prescale doesn't need to be recalculated as the other twin already
> has this configured? But not sure that reading the value (or storing it in drvdata) is better/faster than
> recalculating. So judge yourself.

OK, will calculate prescale in else path, so it won't be recalculated.

> Also moving
> `wfhw->gtccr = 0` into the if block reduces (my) confusion a bit.

Agreed.

> 
> > +		return 1;
> >
> > -	duty_ticks = mul_u64_u64_div_u64(state->duty_cycle, rzg2l_gpt->rate_khz, USEC_PER_SEC);
> > +	duty_ticks = mul_u64_u64_div_u64(wf->duty_length_ns,
> > +rzg2l_gpt->rate_khz, USEC_PER_SEC);
> 
> NSEC_PER_MSEC would be more logically sound here. (But this is a pre-existing mini issue.)

OK, will ad a patch for fixing this.

Cheers,
Biju