Hi Uwe, Thanks for the feedback. > Subject: Re: [PATCH v3 2/4] pwm: Add support for RZ/V2M PWM driver > > Hello, > > > + * > > + https://www.renesas.com/in/en/document/mah/rzv2m-users-manual-hardwa > > + re?language=en > > I took a look into that now, and there are a few things I noticed. > > The PWMCYC register description has: > > To change the setting value of the PWM cycle setting register > (PWMm_PWMCYC), set the PWME bit of the PWM control register > (PWMm_PWMCTR) to 0b and stop the counter operation. If it is > changed during counter operation, PWM output may not be > performed correctly. OK, I will fix it in the driver. > > This isn't repected in the driver. Please either fix that or add a comment > why you think this is not necessary. If you choose to adhere to that, also > note it in the Limitations section that I asked you to add. > > In .apply() you subtract 1 from the calculated value of PWMCYC. When looking > through section 17.4 Function Details I don't see this justified. However in > 17.3.2.2 the formula is as you quoted in the driver (i.e. PWMm_PWMCYC = (PWM > period (ns) / (PWM_CLK period (ns) × Division ratio)) − 1). Can you maybe > test which of the two is correct, maybe adapt the driver code and note in a > comment about the difference? I got confirmation from HW team that below formula is correct. PWMm_PWMCYC = (PWM period (ns) / (PWM_CLK period (ns) × Division ratio)) − 1) > > Also comment would be nice about the fact that the native polarity of the > hardware is inverted (i.e. it starts with the low part). I didn't recheck, > maybe the inversion bit handling must be switched? > > A 100% duty cycle is only possible (according to Figure 17.4-2) with PWMLOW > > PWMCYC. Assuming this is correct, there is the problem that the two > registers have the same width, so if PWMCYC is 0xffffff a 100% duty isn't > possible. So please stick to only using values < 0xffffff for PWMCYC. Will add below code to take care this. /* * A 100% duty cycle is only possible with PWMLOW > PWMCYC. if PWMCYC is 0xffffff * a 100% duty cycle is not possible. */ if (pwm_cyc == pwm_low && (pwm_cyc == FIELD_MAX(RZV2M_PWMCYC_PERIOD))) pwm_cyc -= 1; Cheers, Biju
