Re: [PATCH v6 06/13] pwm: add support for sl28cpld PWM controller

Uwe Kleine-König
2020-07-27T08:48:33Z

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

[stripping recipents as most probably won't be interested in this]

Hello Thierry,

On Mon, Jul 27, 2020 at 09:30:27AM +0200, Thierry Reding wrote:
> On Sun, Jul 26, 2020 at 01:18:27AM +0200, Michael Walle wrote:
> > + /* Polarity inversion is not supported */
> > + if (state->polarity != PWM_POLARITY_NORMAL)
> > + return -EINVAL;
>
> Just a note to myself since this just occurred to me: in the legacy API
> we used to have a ->set_polarity() callback that indicated whether or
> not a controller supports inversion. Since that criterion can no longer
> be used with the atomic API we may want to consider adding some sort of
> capability flags so that these checks can be performed in the core.

I'm not (yet?) convinced this is a good idea because these concepts are
somewhat blurry. For example a controller that only supports normal
polarity (probably) still can generate the wave corresponding to:

.enabled = true,
.polarity = PWM_POLARITY_INVERSED,
.duty_cycle = 0,
.period = 5000,

and unless we refuse this request at the core level (should we?) the
driver must be aware of .polarity anyhow. Ditto for .enabled = false.

> Looks good to me. I assume Lee will want to merge this along with the
> other changes through the MFD tree? If so:

I'd like to have another go through the patch. Will do so (hopefully)
later today. So please don't apply yet.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]