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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 31 May 2019 12:51:38 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Brian Masney <masneyb@...tation.org>,
        "thierry.reding@...il.com" <thierry.reding@...il.com>
Cc:     Stephen Boyd <sboyd@...nel.org>, Andy Gross <agross@...nel.org>,
        David Brown <david.brown@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        MSM <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND] ARM: dts: qcom: msm8974-hammerhead: add device
 tree bindings for vibrator

On Wed, May 29, 2019 at 12:12 PM Brian Masney <masneyb@...tation.org> wrote:

> My first revision of this vibrator driver used the Linux PWM framework
> due to the variable duty cycle:

So what I perceive if I get the thread right is that actually a lot of
qcom clocks (all with the M/N/D counter set-up) have variable duty
cycle. Very few consumers use that feature.

It would be a bit much to ask that they all be implemented as PWMs
and then cast into clocks for the 50/50 dutycycle case, I get that.

What about simply doing both?

Export the same clocks from the clk and pwm frameworks and be
happy. Of course with some mutex inside the driver so that it can't
be used from both ends at the same time.

Further Thierry comments
https://lore.kernel.org/lkml/20181012114749.GC31561@ulmo/

> The device itself doesn't seem to be a
> generic PWM in the way that the PWM framework
> expects it.

I don't see why.  I just look at this function from the original
patch series:

+static int msm_vibra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+ struct msm_vibra_pwm *msm_pwm = to_msm_vibra_pwm(chip);
+ int d_reg_val;
+
+ d_reg_val = 127 - (((duty_ns / 1000) * 126) / (period_ns / 1000));
+
+ msm_vibra_pwm_write(msm_pwm, REG_CFG_RCGR,
+    (2 << 12) | /* dual edge mode */
+    (0 << 8) |  /* cxo */
+    (7 << 0));
+ msm_vibra_pwm_write(msm_pwm, REG_M, 1);
+ msm_vibra_pwm_write(msm_pwm, REG_N, 128);
+ msm_vibra_pwm_write(msm_pwm, REG_D, d_reg_val);
+ msm_vibra_pwm_write(msm_pwm, REG_CMD_RCGR, 1);
+ msm_vibra_pwm_write(msm_pwm, REG_CBCR, 1);
+
+ return 0;
+}

How is this NOT a a generic PWM in the way that the PWM
framework expects it? It configures the period and duty cycle on
a square wave, that is what a generic PWM is in my book.

Yours,
Linus Walleij