linux-kernel - Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGngYiUYOL6EF3VTGwcwTuN4EmE26ML3ye7689FTEpowjEcU2w@mail.gmail.com>
Date:   Thu, 17 Dec 2020 12:52:42 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Clemens Gruber <clemens.gruber@...ruber.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-pwm@...r.kernel.org,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout

On Thu, Dec 17, 2020 at 12:43 PM Clemens Gruber
<clemens.gruber@...ruber.com> wrote:
> >
> > Conclusion: .get_state() will always return "pwm disabled", so why do we
> > bother reading out the h/w?
>
> If there are no plans for the PWM core to call .get_state more often in
> the future, we could just read out the period and return 0 duty and
> disabled.

I'm not sure why we should even read out the period?
When a channel is disabled, the period is not externally visible,
therefore it's meaningless ?

As far as I can tell, we can use this for .get_state():
memset(&pwm->state, 0, sizeof(pwm_state));

>
> Thierry, Uwe, what's your take on this?
>
> > Of course, if we choose to leave the pwm enabled after .free(), then
> > .get_state() can even be left out! Do we want that? Genuine question, I do
> > not know the answer.
>
> I do not think we should leave it enabled after free. It is less
> complicated if we know that unrequested channels are not in use.
>

Good point, I agree with you.