Hello, > From: Uwe Kleine-Konig, Sent: Thursday, December 13, 2018 6:53 PM > > Hello, > > On Thu, Dec 13, 2018 at 09:47:00AM +0000, Yoshihiro Shimoda wrote: > > > From: Uwe Kleine-Konig, Sent: Friday, December 7, 2018 6:14 PM > > > On Fri, Dec 07, 2018 at 05:29:33PM +0900, Yoshihiro Shimoda wrote: > > > > +static void rcar_pwm_workaround_output_low(struct rcar_pwm_chip *rp) > > > > +{ > > > > + /* > > > > + * This PWM Timer cannot output low because setting 0x000 is > > > > + * prohibited on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding > > > > + * the prohibited, this function changes the value from 0 to 1 as > > > > + * pseudo low level. > > > > + * > > > > + * TODO: Add GPIO handling to output low level. > > > > + */ > > > > + if ((rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0) > > > > + rp->pwmcnt |= 1; > > > > > > In my eyes this is too broken to do. Not sure I have the complete > > > picture, but given a small period (say 2) this 1 cycle might result in > > > 50 % duty cycle. Depending on how the hardware behaves if you disable > > > it, better do this instead. > > > > My colleague suggests that this workaround code also changes the period > > as maximum (1023) to avoid 50 % duty cycle for such a case. > > A negative side effect of that is that reenabling the pwm then takes > longer, right? For my mileage even a duty cycle of 1/1023 if 0 is > requested is rather unfortunate. You're right. > > What do you think that this idea is acceptable for upstream? Or, should > > I add gpio handling that Uwe suggested? > > Given that it's impossible to implement a gpio handling that results in > well defined periods only I'm not a big fan of that either. I got it. By the way, I checked R-Car Gen3 manual again (which is not public yet and RZ/G series manual doesn't mention it though), and then changing the pinctrl setting at runtime is not guarantee. So, I have no change to use gpio on the pwm-rcar.c. So, I only have a workaround about this at the moment... > I let Thierry the joy of deciding here. I hope Thierry accepts this workaround. Best regards, Yoshihiro Shimoda > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ |