Re: [PATCH RESEND] ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for vibrator

From: Stephen Boyd <sboyd@kernel.org>
To: Brian Masney <masneyb@onstation.org>,
	Linus Walleij <linus.walleij@linaro.org>
Cc: "thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	Andy Gross <agross@kernel.org>,
	David Brown <david.brown@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	MSM <linux-arm-msm@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RESEND] ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for vibrator
Date: Thu, 27 Jun 2019 16:49:28 -0700	[thread overview]
Message-ID: <20190627234929.B78E520815@mail.kernel.org> (raw)
In-Reply-To: <20190625005434.GA6401@onstation.org>

Quoting Brian Masney (2019-06-24 17:54:34)
> On Tue, Jun 25, 2019 at 12:29:29AM +0200, Linus Walleij wrote:
> > On Sun, Jun 23, 2019 at 12:53 PM Brian Masney <masneyb@onstation.org> wrote:
> > 
> > > 2) Do what Linus suggests above. We can use v1 of this series from last
> > >    September (see below for link) that adds this to the pwm subsystem.
> > >    The locking would need to be added so that it won't conflict with the
> > >    clk subsystem. This can be tied into the input subsystem with the
> > >    existing pwm-vibra driver.
> > 
> > What I imagined was that the clk driver would double as a pwm driver.
> > Just register both interfaces.
> > 
> > There are already plenty of combines clk+reset drivers for example.
> > 
> > Otherwise I'm all for this approach (but that's just me).
> 
> I agree that this makes sense. I especially like that it'll allow us
> to use the existing pwm-vibra driver in the input subsystem with this
> approach.
> 

This whole discussion is ignoring that clk_set_duty_cycle() exists.
Maybe you can look back on the history of why the duty cycle API was
added to the clk framework to make a strong argument for the replacement
of this API with your proposal of registering to two different
frameworks instead?

Here's the first few parts of the commit text of 9fba738a53dd ("clk: add
duty cycle support"):

    Add the possibility to apply and query the clock signal duty cycle ratio.

    This is useful when the duty cycle of the clock signal depends on some
    other parameters controlled by the clock framework.

    For example, the duty cycle of a divider may depends on the raw divider
    setting (ratio = N / div) , which is controlled by the CCF. In such case,
    going through the pwm framework to control the duty cycle ratio of this
    clock would be a burden.

In the case of qcom clks, I seem to recall the frequency of the clk
depends on the duty cycle settings. The duty cycle is constrained by the
M/N values which determine the frequency of the clk after it's
pre-divided. We did some sort of bit trick to set the duty cycle to the
N value inverted or something so that we always got 50% duty cycle.