Hi Uwem
Thank you for your review!
> 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:
<snip>
> > +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.
You're right.
> Are you aware of the series adding such gpio support to the imx driver?
I didn't know that.
> @Thierry: So there are three drivers now that could benefit for a
> generic approach.
Should I wait for Thierry's opinion whether PWM subsystem will have
a generic approach or not?
Best regards,
Yoshihiro Shimoda
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
