linux-kernel - RE: [PATCH] pwm: tegra: dynamic clk freq configuration by PWM driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 20 Apr 2020 16:04:03 +0000
From:   Sandipan Patra <spatra@...dia.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
CC:     Thierry Reding <treding@...dia.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Bibek Basu <bbasu@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] pwm: tegra: dynamic clk freq configuration by PWM driver

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@nvidia.com/
Please help reviewing further.


Thanks & Regards,
Sandipan


> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> Sent: Saturday, April 18, 2020 1:53 PM
> To: Sandipan Patra <spatra@...dia.com>
> Cc: Thierry Reding <treding@...dia.com>; robh+dt@...nel.org; Jonathan
> Hunter <jonathanh@...dia.com>; Bibek Basu <bbasu@...dia.com>; Laxman
> Dewangan <ldewangan@...dia.com>; linux-pwm@...r.kernel.org;
> devicetree@...r.kernel.org; linux-tegra@...r.kernel.org; linux-
> kernel@...r.kernel.org
> 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/ |