Add support for clock-based vibrator devices where the speed can be
controlled by changing the duty cycle.
Signed-off-by: Brian Masney <[email protected]>
---
.../bindings/input/clk-vibrator.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/clk-vibrator.yaml
diff --git a/Documentation/devicetree/bindings/input/clk-vibrator.yaml b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
new file mode 100644
index 000000000000..2103a5694fad
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/input/clk-vibrator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock vibrator
+
+maintainers:
+ - Brian Masney <[email protected]>
+
+description: |
+ Support for clock-based vibrator devices where the speed can be controlled
+ by changing the duty cycle.
+
+properties:
+ compatible:
+ const: clk-vibrator
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ description: output clock that controls the speed
+ items:
+ - const: core
+
+ clock-frequency: true
+
+ enable-gpios:
+ maxItems: 1
+
+ vcc-supply:
+ description: Regulator that provides power
+
+required:
+ - compatible
+ - clocks
+ - clock-names
+ - clock-frequency
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ vibrator {
+ compatible = "clk-vibrator";
+
+ vcc-supply = <&pm8941_l19>;
+
+ clocks = <&mmcc CAMSS_GP1_CLK>;
+ clock-names = "core";
+ clock-frequency = <24000>;
+
+ enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&vibrator_pin>;
+ };
--
2.21.0
On Wed, Dec 4, 2019 at 6:25 PM Brian Masney <[email protected]> wrote:
>
> Add support for clock-based vibrator devices where the speed can be
> controlled by changing the duty cycle.
>
> Signed-off-by: Brian Masney <[email protected]>
> ---
> .../bindings/input/clk-vibrator.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/clk-vibrator.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/clk-vibrator.yaml b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> new file mode 100644
> index 000000000000..2103a5694fad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/input/clk-vibrator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Clock vibrator
> +
> +maintainers:
> + - Brian Masney <[email protected]>
> +
> +description: |
> + Support for clock-based vibrator devices where the speed can be controlled
> + by changing the duty cycle.
> +
> +properties:
> + compatible:
> + const: clk-vibrator
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + description: output clock that controls the speed
> + items:
> + - const: core
No point in making up a name when there's only one clock, so drop.
> +
> + clock-frequency: true
Given the frequency is variable, what does this mean in this case?
> + enable-gpios:
> + maxItems: 1
> +
> + vcc-supply:
> + description: Regulator that provides power
> +
> +required:
> + - compatible
> + - clocks
> + - clock-names
> + - clock-frequency
Add:
additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + vibrator {
> + compatible = "clk-vibrator";
> +
> + vcc-supply = <&pm8941_l19>;
> +
> + clocks = <&mmcc CAMSS_GP1_CLK>;
> + clock-names = "core";
> + clock-frequency = <24000>;
> +
> + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&vibrator_pin>;
> + };
> --
> 2.21.0
>
On Thu, Dec 05, 2019 at 07:56:10AM -0600, Rob Herring wrote:
> On Wed, Dec 4, 2019 at 6:25 PM Brian Masney <[email protected]> wrote:
> >
> > Add support for clock-based vibrator devices where the speed can be
> > controlled by changing the duty cycle.
> >
> > Signed-off-by: Brian Masney <[email protected]>
> > ---
> > .../bindings/input/clk-vibrator.yaml | 60 +++++++++++++++++++
> > 1 file changed, 60 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/input/clk-vibrator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/input/clk-vibrator.yaml b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > new file mode 100644
> > index 000000000000..2103a5694fad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bindings/input/clk-vibrator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Clock vibrator
> > +
> > +maintainers:
> > + - Brian Masney <[email protected]>
> > +
> > +description: |
> > + Support for clock-based vibrator devices where the speed can be controlled
> > + by changing the duty cycle.
> > +
> > +properties:
> > + compatible:
> > + const: clk-vibrator
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + description: output clock that controls the speed
> > + items:
> > + - const: core
>
> No point in making up a name when there's only one clock, so drop.
OK, will do.
>
> > +
> > + clock-frequency: true
>
> Given the frequency is variable, what does this mean in this case?
The clock frequency is fixed. The duty cycle is what's variable.
Brian
>
> > + enable-gpios:
> > + maxItems: 1
> > +
> > + vcc-supply:
> > + description: Regulator that provides power
> > +
> > +required:
> > + - compatible
> > + - clocks
> > + - clock-names
> > + - clock-frequency
>
> Add:
>
> additionalProperties: false
>
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > + #include <dt-bindings/gpio/gpio.h>
> > +
> > + vibrator {
> > + compatible = "clk-vibrator";
> > +
> > + vcc-supply = <&pm8941_l19>;
> > +
> > + clocks = <&mmcc CAMSS_GP1_CLK>;
> > + clock-names = "core";
> > + clock-frequency = <24000>;
> > +
> > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&vibrator_pin>;
> > + };
> > --
> > 2.21.0
> >
On Sun, Dec 8, 2019 at 6:54 PM Brian Masney <[email protected]> wrote:
>
> On Thu, Dec 05, 2019 at 07:56:10AM -0600, Rob Herring wrote:
> > On Wed, Dec 4, 2019 at 6:25 PM Brian Masney <[email protected]> wrote:
> > >
> > > Add support for clock-based vibrator devices where the speed can be
> > > controlled by changing the duty cycle.
> > >
> > > Signed-off-by: Brian Masney <[email protected]>
> > > ---
> > > .../bindings/input/clk-vibrator.yaml | 60 +++++++++++++++++++
> > > 1 file changed, 60 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/clk-vibrator.yaml b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > > new file mode 100644
> > > index 000000000000..2103a5694fad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > > @@ -0,0 +1,60 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/bindings/input/clk-vibrator.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Clock vibrator
> > > +
> > > +maintainers:
> > > + - Brian Masney <[email protected]>
> > > +
> > > +description: |
> > > + Support for clock-based vibrator devices where the speed can be controlled
> > > + by changing the duty cycle.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: clk-vibrator
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > +
> > > + clock-names:
> > > + description: output clock that controls the speed
> > > + items:
> > > + - const: core
> >
> > No point in making up a name when there's only one clock, so drop.
>
> OK, will do.
>
> >
> > > +
> > > + clock-frequency: true
> >
> > Given the frequency is variable, what does this mean in this case?
>
> The clock frequency is fixed. The duty cycle is what's variable.
That sounds like a PWM then...
Rob
On Mon, Dec 09, 2019 at 10:16:26AM -0600, Rob Herring wrote:
> On Sun, Dec 8, 2019 at 6:54 PM Brian Masney <[email protected]> wrote:
> >
> > On Thu, Dec 05, 2019 at 07:56:10AM -0600, Rob Herring wrote:
> > > On Wed, Dec 4, 2019 at 6:25 PM Brian Masney <[email protected]> wrote:
> > > >
> > > > Add support for clock-based vibrator devices where the speed can be
> > > > controlled by changing the duty cycle.
> > > >
> > > > Signed-off-by: Brian Masney <[email protected]>
> > > > ---
> > > > .../bindings/input/clk-vibrator.yaml | 60 +++++++++++++++++++
> > > > 1 file changed, 60 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/clk-vibrator.yaml b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > > > new file mode 100644
> > > > index 000000000000..2103a5694fad
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> > > > @@ -0,0 +1,60 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/bindings/input/clk-vibrator.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Clock vibrator
> > > > +
> > > > +maintainers:
> > > > + - Brian Masney <[email protected]>
> > > > +
> > > > +description: |
> > > > + Support for clock-based vibrator devices where the speed can be controlled
> > > > + by changing the duty cycle.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: clk-vibrator
> > > > +
> > > > + clocks:
> > > > + maxItems: 1
> > > > +
> > > > + clock-names:
> > > > + description: output clock that controls the speed
> > > > + items:
> > > > + - const: core
> > >
> > > No point in making up a name when there's only one clock, so drop.
> >
> > OK, will do.
> >
> > >
> > > > +
> > > > + clock-frequency: true
> > >
> > > Given the frequency is variable, what does this mean in this case?
> >
> > The clock frequency is fixed. The duty cycle is what's variable.
>
> That sounds like a PWM then...
Yes... See this message from Stephen with some more background
information about why this is in the clk subsystem:
https://lore.kernel.org/lkml/[email protected]/
Brian
Quoting Brian Masney (2019-12-04 16:25:00)
> diff --git a/Documentation/devicetree/bindings/input/clk-vibrator.yaml b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> new file mode 100644
> index 000000000000..2103a5694fad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/clk-vibrator.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/input/clk-vibrator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Clock vibrator
> +
> +maintainers:
> + - Brian Masney <[email protected]>
> +
> +description: |
> + Support for clock-based vibrator devices where the speed can be controlled
> + by changing the duty cycle.
> +
> +properties:
> + compatible:
> + const: clk-vibrator
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + description: output clock that controls the speed
> + items:
> + - const: core
> +
> + clock-frequency: true
Can you use assigned-clock-rates for this instead? Then the driver can
call clk_get_rate() if it wants to know the rate that was actually set
on the clk.
> +
> + enable-gpios:
> + maxItems: 1
> +
> + vcc-supply:
> + description: Regulator that provides power
> +
> +required:
> + - compatible
> + - clocks
> + - clock-names
> + - clock-frequency
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + vibrator {
> + compatible = "clk-vibrator";
> +
> + vcc-supply = <&pm8941_l19>;
> +
> + clocks = <&mmcc CAMSS_GP1_CLK>;
> + clock-names = "core";
> + clock-frequency = <24000>;
> +
> + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&vibrator_pin>;
I'm still trying to wrap my head around this. I think we can have a pwm
provider in a clk controller node (so imagine &mmcc has #pwm-cells) and
then this 'clk-vibrator' binding wouldn't exist? Instead we would have
some sort of binding for a device that expects a pwm and whatever else
is required, like the enable gpio and power supply. Is there an actual
hardware block that is this way? Does it have a real product id and is
made by some company? Right now this looks a little too generic to not
just be a catch-all for something that buzzes.
On Sun, Jan 05, 2020 at 12:35:33AM -0800, Stephen Boyd wrote:
> Quoting Brian Masney (2019-12-04 16:25:00)
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > + #include <dt-bindings/gpio/gpio.h>
> > +
> > + vibrator {
> > + compatible = "clk-vibrator";
> > +
> > + vcc-supply = <&pm8941_l19>;
> > +
> > + clocks = <&mmcc CAMSS_GP1_CLK>;
> > + clock-names = "core";
> > + clock-frequency = <24000>;
> > +
> > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&vibrator_pin>;
>
> I'm still trying to wrap my head around this. I think we can have a pwm
> provider in a clk controller node (so imagine &mmcc has #pwm-cells) and
> then this 'clk-vibrator' binding wouldn't exist? Instead we would have
> some sort of binding for a device that expects a pwm and whatever else
> is required, like the enable gpio and power supply. Is there an actual
> hardware block that is this way? Does it have a real product id and is
> made by some company? Right now this looks a little too generic to not
> just be a catch-all for something that buzzes.
So have some of the Qualcomm clocks like this one register with both the
clk and the pwm frameworks? I feel that approach would better represent
the hardware in device tree.
If we did that, then the pwm-vibra driver in the input subsystem could
be used.
Brian
Quoting Brian Masney (2020-01-07 04:03:17)
> On Sun, Jan 05, 2020 at 12:35:33AM -0800, Stephen Boyd wrote:
> > Quoting Brian Masney (2019-12-04 16:25:00)
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > > + #include <dt-bindings/gpio/gpio.h>
> > > +
> > > + vibrator {
> > > + compatible = "clk-vibrator";
> > > +
> > > + vcc-supply = <&pm8941_l19>;
> > > +
> > > + clocks = <&mmcc CAMSS_GP1_CLK>;
> > > + clock-names = "core";
> > > + clock-frequency = <24000>;
> > > +
> > > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> > > +
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&vibrator_pin>;
> >
> > I'm still trying to wrap my head around this. I think we can have a pwm
> > provider in a clk controller node (so imagine &mmcc has #pwm-cells) and
> > then this 'clk-vibrator' binding wouldn't exist? Instead we would have
> > some sort of binding for a device that expects a pwm and whatever else
> > is required, like the enable gpio and power supply. Is there an actual
> > hardware block that is this way? Does it have a real product id and is
> > made by some company? Right now this looks a little too generic to not
> > just be a catch-all for something that buzzes.
>
> So have some of the Qualcomm clocks like this one register with both the
> clk and the pwm frameworks? I feel that approach would better represent
> the hardware in device tree.
That is one option. Or another option would be to have another node that
"adapts" a clk signal to a pwm provider. Similar to how we adapt a gpio
to make a clk gate or mux. Something like:
gcc: clock-controller@f00d {
reg = <0xf00d 0xd00d>;
#clock-cells = <1>;
};
pwm {
compatible = "pwm-clk";
#pwm-cells = <0>;
clocks = <&gcc 45>;
assigned-clocks = <&gcc 45>;
assigned-clock-rates = <1400000>;
};
And then the pwm-clk driver would adjust the duty cycle to generate a
pwm.
>
> If we did that, then the pwm-vibra driver in the input subsystem could
> be used.
On Tue, Jan 07, 2020 at 09:52:21AM -0800, Stephen Boyd wrote:
> Quoting Brian Masney (2020-01-07 04:03:17)
> > On Sun, Jan 05, 2020 at 12:35:33AM -0800, Stephen Boyd wrote:
> > > Quoting Brian Masney (2019-12-04 16:25:00)
> > > > +examples:
> > > > + - |
> > > > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > > > + #include <dt-bindings/gpio/gpio.h>
> > > > +
> > > > + vibrator {
> > > > + compatible = "clk-vibrator";
> > > > +
> > > > + vcc-supply = <&pm8941_l19>;
> > > > +
> > > > + clocks = <&mmcc CAMSS_GP1_CLK>;
> > > > + clock-names = "core";
> > > > + clock-frequency = <24000>;
> > > > +
> > > > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> > > > +
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&vibrator_pin>;
> > >
> > > I'm still trying to wrap my head around this. I think we can have a pwm
> > > provider in a clk controller node (so imagine &mmcc has #pwm-cells) and
> > > then this 'clk-vibrator' binding wouldn't exist? Instead we would have
> > > some sort of binding for a device that expects a pwm and whatever else
> > > is required, like the enable gpio and power supply. Is there an actual
> > > hardware block that is this way? Does it have a real product id and is
> > > made by some company? Right now this looks a little too generic to not
> > > just be a catch-all for something that buzzes.
> >
> > So have some of the Qualcomm clocks like this one register with both the
> > clk and the pwm frameworks? I feel that approach would better represent
> > the hardware in device tree.
>
> That is one option. Or another option would be to have another node that
> "adapts" a clk signal to a pwm provider. Similar to how we adapt a gpio
> to make a clk gate or mux. Something like:
>
> gcc: clock-controller@f00d {
> reg = <0xf00d 0xd00d>;
> #clock-cells = <1>;
> };
>
>
> pwm {
> compatible = "pwm-clk";
> #pwm-cells = <0>;
> clocks = <&gcc 45>;
> assigned-clocks = <&gcc 45>;
> assigned-clock-rates = <1400000>;
> };
>
> And then the pwm-clk driver would adjust the duty cycle to generate a
> pwm.
OK, that makes sense.
I'll pick this up after someone from Qualcomm posts a patch that
implements the clock duty cycle. I'm willing to do that work if someone
explains the relationship between the m, n, and d values on these clocks.
Brian