Hi Uwe, > -----Original Message----- > From: Biju Das > Sent: Friday, December 8, 2023 2:12 PM > Subject: RE: [PATCH v17 3/4] pwm: Add support for RZ/G2L GPT > > Hi Uwe Kleine-König, > > > -----Original Message----- > > From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > > Sent: Friday, December 8, 2023 2:07 PM > > Subject: Re: [PATCH v17 3/4] pwm: Add support for RZ/G2L GPT > > > > Hello Biju, > > > > On Fri, Dec 08, 2023 at 10:34:55AM +0000, Biju Das wrote: > > > > -----Original Message----- > > > > From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > > > > Sent: Thursday, December 7, 2023 9:42 PM > > > > Subject: Re: [PATCH v17 3/4] pwm: Add support for RZ/G2L GPT > > > > > > > > Hello Biju, > > > > > > > > On Thu, Dec 07, 2023 at 06:26:44PM +0000, Biju Das wrote: > > > > > ######[ 304.213944] pwm-rzg2l-gpt 10048000.pwm: .apply is not > > > > > idempotent (ena=1 pol=0 5500000000000/43980352512000) -> (ena=1 > > > > > pol=0 > > > > > 5500000000000/43980239923200) > > > > > High setting## > > > > > [ 304.230854] pwm-rzg2l-gpt 10048000.pwm: .apply is not > > > > > idempotent > > > > > (ena=1 pol=0 23980465100800/43980352512000) -> (ena=1 pol=0 > > > > > 23980465100800/43980239923200) > > > > > > > > Have you tried to understand that? What is the clk rate when this > > happens? > > > > You're not suggesting that mul_u64_u64_div_u64 is wrong, are you? > > > > > > mul_u64_u64_div_u64() works for certain values. But for very high > > > values we are losing precision and is giving unexpected values. > > > > Can you reduce the problem to a bogus result of mul_u64_u64_div_u64()? > > I'd be very surprised if the problem was mul_u64_u64_div_u64() and not > > how it's used in your pwm driver. > > When I looked last time, it drops precision here[1]. I will recheck again. > On RZ/G2L family devices, the PWM rate is 100MHz. > [1] https://elixir.bootlin.com/linux/v6.7-rc4/source/lib/math/div64.c#L214 Please find the bug details in mul_u64_u64_div_u64() compared to mul_u64_u32_div() Theoretical calculation: Period = 43980465100800 nsec Duty_cycle = 23980465100800 nsec PWM rate = 100MHz period_cycles(tmp) = 43980465100800 * (100 * 10 ^ 6) / (10 ^ 9) = 4398046510080 prescale = ((43980465100800 >> 32) >= 256) = 5 period_cycles = min (round_up(4398046510080,( 1 << (2 * 5 )), U32_MAX) = min (4295162607, U32_MAX) = U32_MAX = 0xFFFFFFFF duty_cycles = min (2398046510080, ,( 1 << (2 * 5 )), U32_MAX) = min (2341842295, U32_MAX) = 0x8B95AD77 with mul_u64_u64_div_u64 (ARM64): [ 54.551612] ##### period_cycles_norm=43980465100800 [ 54.305923] ##### period_cycles_tmp=4398035251080 ---> This is the bug. [ 54.311828] ##### pv=ffffd50c [ 54.386047] ##### duty_cycles_tmp=2398046510080 [ 54.390903] ##### dc=8b95ad77 [ 54.395574] ##### period_cycles_norm=43980352512000 [ 54.398715] ##### period_cycles_tmp=4398023992229 [ 54.403717] ##### pv=ffffaa19 [ 54.408594] ##### duty_cycles_norm=23980465100800 [ 54.411673] ##### duty_cycles_tmp=2398046510080 [ 54.416557] ##### dc=8b95ad77 with mul_u64_u32_div(ARM64): [ 46.725909] ##### period_cycles_norm=43980465100800 [ 46.732468] ##### period_cycles_tmp=4398046510080 [ 46.740484] ##### pv=ffffffff [ 46.748457] ##### duty_cycles_norm=23980465100800 [ 46.751510] ##### duty_cycles_tmp=2398046510080 [ 46.760462] ##### dc=8b95ad77 [ 46.765256] ##### period_cycles_norm=43980465100800 [ 46.768282] ##### period_cycles_tmp=4398046510080 [ 46.776590] ##### pv=ffffffff [ 46.782762] ##### duty_cycles_norm=23980465100800 [ 46.802617] ##### dc=8b95ad77 Cheers, Biju
