Re: [PATCH v3] pwm: lpss: Add get_state callback

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, 2017-03-10 at 22:13 +0100, Hans de Goede wrote:
> Hi,
> 
> On 03/10/2017 04:49 PM, Andy Shevchenko wrote:
> > On Tue, 2017-02-28 at 15:09 +0100, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 27-02-17 23:20, Andy Shevchenko wrote:
> > > > On Mon, Feb 27, 2017 at 6:59 PM, Hans de Goede <hdegoede@redhat.
> > > > com>
> > > > wrote:

> > > Ok, so maybe we should do this then ?
> > > 
> > > 	if (freq == 0) {
> > > 		state->period = NSEC_PER_SEC;
> > > 		state->duty_cycle = NSEC_PER_SEC;
> > > 		return;
> > > 	}
> > > 
> > > ?
> > 
> > But the period is not that one. Can we assign them to 0?
> 
> That will cause a divide by 0 in apply() when a user
> calls apply without changing / setting period.

I have to look closer to this one. Perhaps next week together with yours
other change.

> > > Anyways since we now may get a pm-ref during probe
> > > I will add the balancing (and equally conditional)
> > > put of that ref to pwm_lpss_remove().
> > > 
> > > > Second question, how ->get_state() is supposed to work if power
> > > > is
> > > > off
> > > > when it enters it?
> > > 
> > > get_state only gets called on probe and the device is guaranteed
> > > to
> > > be powered on during probe().
> > 
> > Ah, seems I now understand what this call back actually does.
> > 
> > So, the name of the callback a bit confusing, to me it sounds rather
> > "get_to_[initial_]state()".
> > 
> > Thus, would it be better to split ->apply() to get a common part for
> > both?
> 
> I don't understand what you mean here ? Ah I guess you mean split
> out the pm_runtime_get / put stuff. I don't think that is going
> to work well as apply compares old and new state and we do not
> have old state when get_state gets called.

I mean to have similar flow in ->get_state() and ->apply() when we
enable the device.

I don't like ->get_state() name as I mentioned before. If its
functionality is "get *to* (initial) state", that what we need to
implement.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy