Re: [PATCH 1/4] pwm: Read initial hardware state at request time

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

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

On Mon, Oct 21, 2019 at 01:11:12PM +0200, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Mon, Oct 21, 2019 at 12:57:36PM +0200, Thierry Reding wrote:
> > The PWM core doesn't need to know about the hardware state of a PWM
> > unless there is a user for it. Defer initial hardware readout until
> > a PWM is requested.
> 
> A side effect is that for an unused PWM the get_state callback is never
> called (which is good), in return it is called more than once if the PWM
> is requested more often (which is bearable).

You can't request a PWM more than once. PWMs are always exclusive to a
single driver. Now I suppose you could have a single driver request it
multiple times (that driver would then also have to release it before
requesting it again), but I think it's reasonable for the subsystem to
query the hardware state everytime before a PWM is handed to a consumer.
The hardware state could have changed between the time where a consumer
releases the PWM and another requests it.

> 
> > As a side-effect, this allows the ->get_state() callback to rely on
> > per-PWM data.
> 
> Given that this is the motivation for your change I'd give more stress
> to this part of the commit log. Also I think this could be more
> understandable if you point out that the effect is that .get_state is
> only called after .request was called successfully which gives the low
> level driver more freedom by (for example) relying on memory allocated
> there.

Isn't that pretty much already in the above commit message just with
different words? I can try to reword this in a different way if that
makes you happier.

> I assume you target the next merge window for this change?

Yes. I'm not sure yet about the remainder of the series. Depending on
what we decide to do about drivers that can't (or don't want to) write
all state through to the hardware, patches 2-4 may become moot.

Thierry

> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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