RE: [PATCH 5/5] pwm: rcar: add workaround to output "pseudo" low level

[Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>]

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/  |