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-pwm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-renesas-soc
Message-ID <TY3PR01MB11346293B0C51E263E139624F86CC2@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Hello Uwe,

Thanks for the feedback.

> -----Original Message-----
> From: Uwe Kleine-K=F6nig <[email protected]>
> Sent: 16 July 2026 09:47
> Subject: Re: [PATCH v6 06/11] pwm: rzg2l-gpt: Convert to waveform callbac=
ks
>=20
> Hello Biju,
>=20
> 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 =3D 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 =3D true;
> >
> >  			period_ticks =3D rzg2l_gpt->period_ticks[ch];
> >  		}
> >  	}
> >
> > -	prescale =3D rzg2l_gpt_calculate_prescale(period_ticks);
> > -	pv =3D rzg2l_gpt_calculate_pv_or_dc(period_ticks, prescale);
> > +	wfhw->prescale =3D rzg2l_gpt_calculate_prescale(period_ticks);
> > +	wfhw->gtpr =3D rzg2l_gpt_calculate_pv_or_dc(period_ticks, wfhw->presc=
ale);
> > +	wfhw->gtccr =3D 0;
> > +	if (is_small_second_period)
>=20
> If is_small_second_period =3D=3D true, the prescale doesn't need to be re=
calculated as the other twin already
> has this configured? But not sure that reading the value (or storing it i=
n 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 =3D 0` into the if block reduces (my) confusion a bit.

Agreed.

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

OK, will ad a patch for fixing this.

Cheers,
Biju