Message-ID: <20160204150150.553d36df@bbrezillon>
Date:	Thu, 4 Feb 2016 15:01:50 +0100
From:	Boris Brezillon <boris.brezillon@...e-electrons.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	Doug Anderson <dianders@...gle.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Heiko Stübner <heiko@...ech.de>,
	linux-pwm <linux-pwm@...r.kernel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Jingoo Han <jingoohan1@...il.com>,
	Lee Jones <lee.jones@...aro.org>, linux-fbdev@...r.kernel.org,
	Bryan Wu <cooloney@...il.com>,
	Richard Purdie <rpurdie@...ys.net>,
	Jacek Anaszewski <j.anaszewski@...sung.com>,
	linux-leds@...r.kernel.org,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	linux-sunxi@...glegroups.com,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Tomi Valkeinen <tomi.valkeinen@...com>,
	Daniel Mack <daniel@...que.org>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Robert Jarzmik <robert.jarzmik@...e.fr>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>, Olof Johansson <olof@...om.net>
Subject: Re: [PATCH v3 00/12] pwm: add support for atomic update

Hi Mark, Thierry,

On Thu, 4 Feb 2016 11:02:03 +0000
Mark Brown <broonie@...nel.org> wrote:

> On Wed, Feb 03, 2016 at 11:04:20AM -0800, Doug Anderson wrote:
> 
> > Sure.  ...but you agree that somehow you need a new API call for this,
> > right?  Somehow the PWM regulator needs to be able to say that it
> > wants the hardware state, not the initial state as specified in the
> > device tree.
> 
> Wouldn't the most direct way to do that be to just not specify anything
> in the DT?  If there *is* something in DT but we ignore it that's a bit
> weird.

Just adding some inputs on this specific aspect. The reason we have to
specify a period (and, to a lesser extent, the polarity) in the DT or
PWM lookup table is because what most PWM users want is to specify a
dutycycle relatively to a predefined period value.
If we decide to remove those information from the DT, then you'll need
a way to define it somewhere else, and then the is question is 'where?'.

Users that really want to control their period (this could the case for
the clk-pwm driver) could completely ignore DT/lookup-table information
and set the period and absolute dutycycle directly.

Now, from what I seen, what most PWM users want to do is:

	pwm_set_rel_duty_scale(pwm, rel_value, scale);
or 
	rel_duty = pwm_get_rel_duty_cycle(pwm, scale);

where scale depends on the precision you need for your use case (most
of the time it's expressed in percent).

So, how about providing this kind of API (this is what I proposed in
one of my previous email)?

This would not only solve our problem (say you have a period at
boot-time that differs from the one you'll set when first applying a
new relative duty cycle, then the resulting relative value would still
be correct), but it would also remove a lot of boiler plate code from
PWM users code (if you take a look at pwm-regulator, pwm-leds, pwm-fan
and probably others, you'll see that they are all doing this conversion
manually).

Now, the last blocking point is, what if the PWM driver does not
implement HW-readout. In this case, the pwm-regulator will probably
expose a 0V output (IIRC, dutycycle is set to 0 by default) when it's
actually providing something else. But is this really important? I
mean, if the user really wants to have a reliable information, then he
will implement initial-state retrieval in its PWM controller driver.
Alternatively, we could put a flag specifying whether the PWM chip
supports initial state retrieval.

Am I missing something?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Powered by blists - more mailing lists