Hi Uwe, I will send next version incorporating the review comments/discussion based on this patch series. Cheers, Biju > Subject: RE: [PATCH v3 2/4] pwm: Add support for RZ/V2M PWM driver > > 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-hard > > > + wa > > > + 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;
