Re: [PATCH] pwm: poke users of legacy drivers with a warning

[Thierry Reding <thierry.reding@gmail.com>]

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

On Tue, Mar 12, 2019 at 01:03:55PM +0100, Uwe Kleine-König wrote:
> On Tue, Mar 12, 2019 at 12:39:44PM +0100, Thierry Reding wrote:
> > On Tue, Mar 12, 2019 at 10:42:49AM +0100, Uwe Kleine-König wrote:
> > > As it is hard to convert a given driver from the legacy functions
> > > (.enable, .disable, .config) to the atomic stuff (.apply) without
> > > testing make users aware that there is something to do with a warning to
> > > eventually get rid of the legacy drivers.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/pwm/core.c | 14 +++++++++-----
> > >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > I don't think that's a good idea. What are regular users supposed to do
> > when they see this warning?
> 
> Ask on this list for directions or volunteer as tester.

I don't think that's fair to expect from users. There's nothing risky
about using a legacy API driver, so users shouldn't need to do anything.
This is a maintenance issue that users don't have to be concerned about.

> Then in a few years we can as a next step make these drivers fail to
> probe if they are still unfixed.

Erm... no. Why would you want to purposely break a driver that's working
perfectly fine?

> If you don't agree I wonder how you want to complete the task to convert
> all drivers to the atomic API.

There's technically nothing wrong with using the legacy API. It's worked
well for a number of years and a large number of drivers. We have
helpers that allow atomic API users to work with legacy API drivers.
From my point of view there's no need to convert drivers that work fine
with the legacy API.

That said, it would be nice if it did happen eventually, if for no other
reason than to get rid of the compatibility shims, which aren't really
that bad in the first place. But I think the conversion could be done in
a relatively automated way. pwm_apply_state() codifies what the sequence
of calls is for the atomic API, so that's the sequence that ->apply()
would need to implement as well, given the existing legacy callbacks.
Another way of saying this is to push pwm_apply_state() down into driver
implementations. That's probably not the ideal way of implementing the
changes in a driver, but it's certainly one that should work.

Thierry

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