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  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]
Date:   Sun, 3 Jan 2021 18:04:10 +0100
From:   Clemens Gruber <clemens.gruber@...ruber.com>
To:     Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-K├Ânig 
        <u.kleine-koenig@...gutronix.de>,
        Sven Van Asbroeck <thesven73@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-pwm@...r.kernel.org
Subject: Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout

Hi everyone,

happy new year, hope you are all well!

On Thu, Dec 17, 2020 at 12:52:42PM -0500, Sven Van Asbroeck wrote:
> 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?

I will continue working on this series in the upcoming weeks.
Feedback on the .get_state issue would be greatly appreciated.

To summarize:
Is it OK for a driver to expect the chip->ops->get_state function to be
only called from the place in pwm core it is currently called from?
(Namely in pwm_device_request after chip->ops->request)

If yes, we could always return a 0 duty cycle and disabled state,
because this is the state we left it in after .probe (and .free).

However, if in the future, the pwm core adds additional calls to
chip->ops->get_state in other places, this could lead to problems.

--

Another point is the period: Sven suggested we do not read out the
period at all, as the PWM is disabled anyway (see above).
Is this acceptable?

And, if we never return anything but 0 in .get_state, should it be
implemented at all?

> >
> > > 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.

Thanks and best regards,
Clemens

Powered by blists - more mailing lists