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

["Uwe Kleine-König" <ukleinek@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2075 bytes --]

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.
Also moving
`wfhw->gtccr = 0` into the if block reduces (my) confusion a bit.

> +		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.)

>  	if (duty_ticks > period_ticks)
>  		duty_ticks = period_ticks;
> -	dc = rzg2l_gpt_calculate_pv_or_dc(duty_ticks, prescale);
> +	wfhw->gtccr = rzg2l_gpt_calculate_pv_or_dc(duty_ticks, wfhw->prescale);
>  
> -	/*
> -	 * GPT counter is shared by multiple channels, we cache the period ticks
> -	 * from the first enabled channel and use the same value for both
> -	 * channels.
> -	 */
> -	rzg2l_gpt->period_ticks[ch] = period_ticks;
> +	return 0;
> +}

Remainder looks good.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]