RE: [PATCH 2/5] pwm: rcar: Add support "atomic" API

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>
Cc: "Thierry Reding" <thierry.reding@gmail.com>,
	"Linux PWM List" <linux-pwm@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"Michal Vokáč" <Michal.Vokac@ysoft.com>
Subject: RE: [PATCH 2/5] pwm: rcar: Add support "atomic" API
Date: Mon, 10 Dec 2018 04:58:46 +0000	[thread overview]
Message-ID: <OSBPR01MB229367B0C11DD720F20CF823D8A50@OSBPR01MB2293.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <20181207104548.cuwlavf62f2f4hdk@pengutronix.de>

Hi Uwe,

> From: Uwe Kleine-König, Sent: Friday, December 7, 2018 7:46 PM
> 
> Hello,
> 
> On Fri, Dec 07, 2018 at 10:57:48AM +0100, Geert Uytterhoeven wrote:
> > > Is the documentation for this hardware publically available?
> >
> > Please check Section 59 of the "User's Manual: Hardware" at
> > https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz/rzg/rzg1m.html
> 
> Thanks.
> 
> So the ugly things here are:
> 
>  a) if the pwm is stopped (PWMCR.ENO = 0) the output is set to high after
>     completion of the currently running period.
>  b) setting a duty_cycle of 0 is forbidden

Thank you for checking the hardware specification.

> both are bad. A workaround for both is implementation of something
> similar to the switch to a gpio suggested by Michal for imx. But this
> cannot be done reliably because the current period's end isn't
> observable.

I agree. This R-Car PWM needs special handling when the driver disable the PWM.

> Alternatively a confirmation from the Renesas engineers that PWMCNT.PHO
> can be set to 0 with the intended effect despite being forbidden in the
> reference manual would be great. Did someone with access to such
> hardware test what happens if the PHO field is set to 0? Maybe the
> forbidden value is just a wrong copy&paste from the CYCO field?

I'm asking HW guy about this specification now.
# According to the state machine in the manual, it seems the PWM cannot output
# low level when the PWM is enabled though...

> I think it would be a good idea to add the link to the documentation
> into a comment at the top of the driver.

I think so.

Best regards,
Yoshihiro Shimoda

> @Thierry: Given that nobody seems to have an overview about the features
> and ugly implementation details of all the PWMs, what about documenting
> them in the driver files in a greppable way. For the rcar driver
> something like:
> 
>  - duty-counter-bits: 10
>  - period-counter-bits: 10
>  - hardware-polarity-support: false
>  - uglyness:
>    - OUTPUT-ACTIVE-ON-DISABLE
>    - NO-ZERO-DUTY-CYCLE
> 
> Best regards
> Uwe
> 
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |