Re: [PATCH v4 1/4] pwm: pca9685: Switch to atomic API

[Uwe Kleine-König]

Hello Thierry,

On Fri, Dec 11, 2020 at 09:33:55AM +0100, Thierry Reding wrote:
> On Thu, Dec 10, 2020 at 09:39:26PM +0100, Uwe Kleine-König wrote:
> > On Thu, Dec 10, 2020 at 06:10:45PM +0100, Thierry Reding wrote:
> > > Like I said, that's not what I was saying. I was merely saying that if
> > > there aren't any use-cases that current users rely on that would be
> > > broken by using this simpler implementation, then I'm okay with it, even
> > > if it's less flexible than a more complicated implementation. It should
> > > be possible to determine what the current users are by inspecting device
> > > trees present in the kernel. Anything outside the kernel isn't something
> > > we need to consider, as usual.
> > 
> > If "users in mainline" is the criteria that's a word.
> 
> I didn't say "users in mainline", I said "use-cases". What I don't want
> to happen is for this change under discussion to break any existing use-
> cases of any existing users in the kernel. I said that we can determine
> what the existing users are by looking at which device trees use the
> compatible strings that the driver matches on.
> 
> > So you agree we remove the following drivers?:
> > 
> >  - pwm-hibvt.c
> >    Last driver specific change in Feb 2019, no mainline user
> >  - pwm-sprd.c
> >    Last driver specific change in Aug 2019, no mainline user
> 
> No, that's an extrapolation of what I was saying above. Drivers with no
> apparent users are a separate topic, so don't conflate it with the issue
> at hand.

I cannot follow (and I think that's the problem between us and why those
conflicts happen between us). For me it's a logic consequence of
"Anything outside the kernel isn't something we need to consider, as
usual." that drivers that are untouched for some period and have no
mainline users can/should go away. (Is "extrapolation" as strong as
"implication", or has it subjective interpretation added? My dictionary
isn't accurate enough for that question.) But it seems there is
something with my logic or you not saying exactly what you actually
mean. (Did I miss any option? If yes it might be covered by a problem in
my logic.)

Having said that, even in the question at hand (i.e. what is the better
compromise for mapping the inter-channel hardware limitations to
software policy in the pac9685 driver) the idea "let's inspecting device
trees present in the kernel" doesn't work, because for this driver there
are none, too. (It might be used by a mainline machine via ACPI, but
this is even less possible to consider for our judgements than a device
tree with such a device and no user but the sysfs PWM interface.)

> While it's certainly unfortunate that these don't seem to be used, I see
> no reason why we should remove them. They don't create much of a
> maintenance burden, so I'm fine with keeping them in the hopes that
> users may still show up at some point.

The problem I have with them is that I expect your voice of dissent when
I find the time to improve the rounding behaviour of these drivers.
My ultimate goal is to make the PWM framework a system where consumers
can rely on a consistent behaviour of the API and a way to actually
order what they need and get it. I'm not entirely sure we agree that
we're not there yet.

> > Most PWMs are added to cpu.dtsi files with status = "disabled", I wonder
> > if it makes sense to check the machine.dts files if some of the PMWs are
> > completely unused. Do you consider status = "okay" a use that we have to
> > retain even if the node has no phandle?
> 
> A PWM controller may be in use via sysfs even if it has no phandle.

Yeah, I expected you will say that. (And I agree.)

Best regards
Uwe

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

Attachment: signature.asc
Description: PGP signature