Messages in this thread | | | Date | Mon, 30 Mar 2020 18:02:38 +0200 | From | Thierry Reding <> | Subject | Re: [PATCH 1/4] pwm: pca9685: remove unused duty_cycle struct element |
| |
On Mon, Mar 30, 2020 at 04:18:22PM +0300, Andy Shevchenko wrote: > On Mon, Mar 30, 2020 at 4:09 PM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > On Wed, Feb 26, 2020 at 02:52:26PM +0100, Matthias Schiffer wrote: > > > duty_cycle was only set, never read. > > > > > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > > > --- > > > drivers/pwm/pwm-pca9685.c | 4 ---- > > > 1 file changed, 4 deletions(-) > > > > Applied, thanks. > > I'm not sure this patch is correct.
What makes you say that? If you look at the code, the driver sets this field to either 0 or some duty cycle value but ends up never using it. Why would it be wrong to remove that code?
> We already have broken GPIO in this driver. Do we need more breakage?
My understanding is that nobody was able to pinpoint exactly when this regressed, or if this only worked by accident to begin with. It sounds like Clemens has a way of testing this driver, so perhaps we can solve that GPIO issue while we're at it.
The last discussion on this seems to have been around the time when you posted a fix for it:
https://patchwork.ozlabs.org/patch/1156012/
But then Sven had concerns that that also wasn't guaranteed to work:
https://lkml.org/lkml/2019/6/2/73
So I think we could either apply your patch to restore the old behaviour which I assume you tested, so at least it seems to work in practice, even if there's still a potential race that Sven pointed out in the above link.
I'd prefer something alternative because it's obviously confusing and completely undocumented. Mika had already proposed something that's a little bit better, though still somewhat confusing.
Oh... actually reading further through those threads there seems to be a patch from Sven that was reviewed by Mika but then nothing happened:
https://lkml.org/lkml/2019/6/4/1039
with the corresponding patchwork URL:
https://patchwork.ozlabs.org/patch/1110083/
Andy, Clemens, do you have a way of testing the GPIO functionality of this driver? If so, it'd be great if you could check the above patch from Sven to fix PWM/GPIO interop.
Thierry [unhandled content-type:application/pgp-signature] | |