Hi Uwe, Thanks for reviewing and providing your inputs to the changes. The review comments are addressed in the new patch. Patch (V2) is ready to be reviewed at: https://patchwork.ozlabs.org/project/linux-pwm/patch/1587398043-18767-1-git-send-email-spatra@xxxxxxxxxx/ Please help reviewing further. Thanks & Regards, Sandipan > -----Original Message----- > From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > Sent: Saturday, April 18, 2020 1:53 PM > To: Sandipan Patra <spatra@xxxxxxxxxx> > Cc: Thierry Reding <treding@xxxxxxxxxx>; robh+dt@xxxxxxxxxx; Jonathan > Hunter <jonathanh@xxxxxxxxxx>; Bibek Basu <bbasu@xxxxxxxxxx>; Laxman > Dewangan <ldewangan@xxxxxxxxxx>; linux-pwm@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] pwm: tegra: dynamic clk freq configuration by PWM driver > > External email: Use caution opening links or attachments > > > Hello, > > On Fri, Apr 17, 2020 at 02:53:22PM +0000, Sandipan Patra wrote: > > > To put my expression in words: pick the maximum of the possible > > > periods that are less or equal to the requested value. Maybe this > > > is better > > > understandable: > > > > > > max { x ∊ implementablePeriods | x <= requestedPeriod } > > > > > > ? > > > > I think I got your question. > > Should tegra_pwm_config() not return error (EINVAL) when the requested > > period is invalid but it should configure to a nearest possible value? > > If you cannot configure according to the above rule, yes, return an error code. > EINVAL is the usual one I think (some also return ERANGE). > > > > > Yes, the output stops as soon as the PWM_ENABLE bit is cleared in > > > > hardware. Then The output is set to 0 (which is inactive). > > > > Once .disable() => tegra_pwm_disable() gets invoked, enable bit is > > > > cleared and hence PWM will possess no output signal. > > > > tegra_pwm_config() will be invoked for any new configuration request. > > > > > > Some drivers already have a "Limitations" section in their header. > > > Please take a look at the existing examples and provide something > > > similar. (Note you still didn't answer "How does a running PWM > > > behave when the register is updated? Does it complete the currently > > > running period?". I assume the answer to the second question is "No" > > > (and the first is only there for rhetoric reasons).) > > > > > > > 1. I will add the below comments as Limitations: > > - When PWM is disabled, the output is driven to 0 and > > In fact, this is a good property. So the only problem is, that for both stop and > reconfiguration the currently running period isn't completed. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
