Linux Input Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/7] qcom: add clk-vibrator driver
@ 2019-12-05  0:24 Brian Masney
  2019-12-05  0:24 ` [PATCH 1/7] clk: qcom: add support for setting the duty cycle Brian Masney
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:24 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

This patch series adds support for the vibrator that's found on the
Nexus 5 phone. I previously added a msm-vibrator driver to the input
subsystem, however that's not the correct approach since the direct
register writes should occur from within the clk subsystem based on the
conversation at
https://lore.kernel.org/lkml/20190516085018.2207-1-masneyb@onstation.org/

So this patch series:

  - Adds support for setting the clock duty cycle to clk-rcg2.c
  - Removes the msm-vibrator driver and adds a generic clk-vibrator
    driver in its place. No one is using this driver at the moment so we
    shouldn't get any complaints.

I also included the defconfig and dts changes. Once this whole series is
deemed to be ready, it can be merged in pieces through the different
subsystems. I included everything here as one patch series so everyone
can see the complete picture of what I'm doing.

Sorry it took me awhile to get back to correcting this; was working on
other tasks on this phone.

Brian Masney (7):
  clk: qcom: add support for setting the duty cycle
  dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator
  Input: drop msm-vibrator in favor of clk-vibrator driver
  dt-bindings: Input: introduce new clock vibrator bindings
  Input: introduce new clock vibrator driver
  ARM: qcom_defconfig: drop msm-vibrator in favor of clk-vibrator driver
  ARM: dts: qcom: msm8974-hammerhead: add support for vibrator

 .../bindings/input/clk-vibrator.yaml          |  60 ++++++++
 .../bindings/input/msm-vibrator.txt           |  36 -----
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    |  30 ++++
 arch/arm/configs/qcom_defconfig               |   2 +-
 drivers/clk/qcom/clk-rcg.h                    |   4 +
 drivers/clk/qcom/clk-rcg2.c                   |  61 +++++++-
 drivers/input/misc/Kconfig                    |  20 +--
 drivers/input/misc/Makefile                   |   2 +-
 .../misc/{msm-vibrator.c => clk-vibrator.c}   | 138 +++++++-----------
 9 files changed, 216 insertions(+), 137 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/clk-vibrator.yaml
 delete mode 100644 Documentation/devicetree/bindings/input/msm-vibrator.txt
 rename drivers/input/misc/{msm-vibrator.c => clk-vibrator.c} (51%)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/7] clk: qcom: add support for setting the duty cycle
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
@ 2019-12-05  0:24 ` Brian Masney
  2019-12-10  4:47   ` Taniya Das
       [not found]   ` <0101016eee224b50-8a5545e2-837f-41c2-9574-b385e111a6b3-000000@us-west-2.amazonses.com>
  2019-12-05  0:24 ` [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator Brian Masney
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:24 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Add support for setting the duty cycle via the D register for the
Qualcomm clocks.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
A few quirks that were noted when varying the speed of CAMSS_GP1_CLK on
msm8974 (Nexus 5 phone):

  - The mnd_width is set to 8 bits, however the d width is actually 7
    bits, at least for this clock. I'm not sure about the other clocks.

  - When the d register has a value less than 17, the following error
    from update_config() is shown: rcg didn't update its configuration.
    So this patch keeps the value of the d register within the range
    [17, 127].

I'm not sure about the relationship of the m, n, and d values,
especially how the 50% duty cycle is calculated by inverting the n
value, so that's why I'm saving the duty cycle ratio for
get_duty_cycle().

 drivers/clk/qcom/clk-rcg.h  |  4 +++
 drivers/clk/qcom/clk-rcg2.c | 61 +++++++++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 78358b81d249..c3b8732cec8f 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -139,6 +139,8 @@ extern const struct clk_ops clk_dyn_rcg_ops;
  * @freq_tbl: frequency table
  * @clkr: regmap clock handle
  * @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
+ * @duty_cycle_num: Numerator of the duty cycle ratio
+ * @duty_cycle_den: Denominator of the duty cycle ratio
  */
 struct clk_rcg2 {
 	u32			cmd_rcgr;
@@ -149,6 +151,8 @@ struct clk_rcg2 {
 	const struct freq_tbl	*freq_tbl;
 	struct clk_regmap	clkr;
 	u8			cfg_off;
+	int			duty_cycle_num;
+	int			duty_cycle_den;
 };
 
 #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 8f4b9bec2956..8d685baefe50 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -258,7 +258,11 @@ static int clk_rcg2_determine_floor_rate(struct clk_hw *hw,
 	return _freq_tbl_determine_rate(hw, rcg->freq_tbl, req, FLOOR);
 }
 
-static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
+static int __clk_rcg2_configure_with_duty_cycle(struct clk_rcg2 *rcg,
+						const struct freq_tbl *f,
+						int d_reg_val,
+						int duty_cycle_num,
+						int duty_cycle_den)
 {
 	u32 cfg, mask;
 	struct clk_hw *hw = &rcg->clkr.hw;
@@ -280,9 +284,12 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
 			return ret;
 
 		ret = regmap_update_bits(rcg->clkr.regmap,
-				RCG_D_OFFSET(rcg), mask, ~f->n);
+				RCG_D_OFFSET(rcg), mask, d_reg_val);
 		if (ret)
 			return ret;
+
+		rcg->duty_cycle_num = duty_cycle_num;
+		rcg->duty_cycle_den = duty_cycle_den;
 	}
 
 	mask = BIT(rcg->hid_width) - 1;
@@ -295,6 +302,12 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
 					mask, cfg);
 }
 
+static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
+{
+	/* Set a 50% duty cycle */
+	return __clk_rcg2_configure_with_duty_cycle(rcg, f, ~f->n, 1, 2);
+}
+
 static int clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
 {
 	int ret;
@@ -353,6 +366,32 @@ static int clk_rcg2_set_floor_rate_and_parent(struct clk_hw *hw,
 	return __clk_rcg2_set_rate(hw, rate, FLOOR);
 }
 
+static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+
+	duty->num = rcg->duty_cycle_num;
+	duty->den = rcg->duty_cycle_den;
+
+	return 0;
+}
+
+static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+	int ret, d_reg_val, mask;
+
+	mask = BIT(rcg->mnd_width - 1) - 1;
+	d_reg_val = mask - (((mask - 17) * duty->num) / duty->den);
+	ret = __clk_rcg2_configure_with_duty_cycle(rcg, rcg->freq_tbl,
+						   d_reg_val, duty->num,
+						   duty->den);
+	if (ret)
+		return ret;
+
+	return update_config(rcg);
+}
+
 const struct clk_ops clk_rcg2_ops = {
 	.is_enabled = clk_rcg2_is_enabled,
 	.get_parent = clk_rcg2_get_parent,
@@ -361,6 +400,8 @@ const struct clk_ops clk_rcg2_ops = {
 	.determine_rate = clk_rcg2_determine_rate,
 	.set_rate = clk_rcg2_set_rate,
 	.set_rate_and_parent = clk_rcg2_set_rate_and_parent,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_rcg2_ops);
 
@@ -372,6 +413,8 @@ const struct clk_ops clk_rcg2_floor_ops = {
 	.determine_rate = clk_rcg2_determine_floor_rate,
 	.set_rate = clk_rcg2_set_floor_rate,
 	.set_rate_and_parent = clk_rcg2_set_floor_rate_and_parent,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_rcg2_floor_ops);
 
@@ -499,6 +542,8 @@ const struct clk_ops clk_edp_pixel_ops = {
 	.set_rate = clk_edp_pixel_set_rate,
 	.set_rate_and_parent = clk_edp_pixel_set_rate_and_parent,
 	.determine_rate = clk_edp_pixel_determine_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_edp_pixel_ops);
 
@@ -557,6 +602,8 @@ const struct clk_ops clk_byte_ops = {
 	.set_rate = clk_byte_set_rate,
 	.set_rate_and_parent = clk_byte_set_rate_and_parent,
 	.determine_rate = clk_byte_determine_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_byte_ops);
 
@@ -627,6 +674,8 @@ const struct clk_ops clk_byte2_ops = {
 	.set_rate = clk_byte2_set_rate,
 	.set_rate_and_parent = clk_byte2_set_rate_and_parent,
 	.determine_rate = clk_byte2_determine_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_byte2_ops);
 
@@ -717,6 +766,8 @@ const struct clk_ops clk_pixel_ops = {
 	.set_rate = clk_pixel_set_rate,
 	.set_rate_and_parent = clk_pixel_set_rate_and_parent,
 	.determine_rate = clk_pixel_determine_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_pixel_ops);
 
@@ -804,6 +855,8 @@ const struct clk_ops clk_gfx3d_ops = {
 	.set_rate = clk_gfx3d_set_rate,
 	.set_rate_and_parent = clk_gfx3d_set_rate_and_parent,
 	.determine_rate = clk_gfx3d_determine_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_gfx3d_ops);
 
@@ -942,6 +995,8 @@ const struct clk_ops clk_rcg2_shared_ops = {
 	.determine_rate = clk_rcg2_determine_rate,
 	.set_rate = clk_rcg2_shared_set_rate,
 	.set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
 
@@ -1081,6 +1136,8 @@ static const struct clk_ops clk_rcg2_dfs_ops = {
 	.get_parent = clk_rcg2_get_parent,
 	.determine_rate = clk_rcg2_dfs_determine_rate,
 	.recalc_rate = clk_rcg2_dfs_recalc_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+	.set_duty_cycle = clk_rcg2_set_duty_cycle,
 };
 
 static int clk_rcg2_enable_dfs(const struct clk_rcg_dfs_data *data,
-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
  2019-12-05  0:24 ` [PATCH 1/7] clk: qcom: add support for setting the duty cycle Brian Masney
@ 2019-12-05  0:24 ` Brian Masney
  2019-12-17 14:11   ` Rob Herring
  2019-12-05  0:24 ` [PATCH 3/7] Input: drop msm-vibrator in favor of clk-vibrator driver Brian Masney
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:24 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Let's drop the msm-vibrator bindings so that the more generic
clk-vibrator can be used instead. No one is currently using these
bindings so this won't affect any users.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../bindings/input/msm-vibrator.txt           | 36 -------------------
 1 file changed, 36 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/msm-vibrator.txt

diff --git a/Documentation/devicetree/bindings/input/msm-vibrator.txt b/Documentation/devicetree/bindings/input/msm-vibrator.txt
deleted file mode 100644
index 8dcf014ef2e5..000000000000
--- a/Documentation/devicetree/bindings/input/msm-vibrator.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-* Device tree bindings for the Qualcomm MSM vibrator
-
-Required properties:
-
-  - compatible: Should be one of
-		"qcom,msm8226-vibrator"
-		"qcom,msm8974-vibrator"
-  - reg: the base address and length of the IO memory for the registers.
-  - pinctrl-names: set to default.
-  - pinctrl-0: phandles pointing to pin configuration nodes. See
-               Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-  - clock-names: set to pwm
-  - clocks: phandle of the clock. See
-            Documentation/devicetree/bindings/clock/clock-bindings.txt
-  - enable-gpios: GPIO that enables the vibrator.
-
-Optional properties:
-
-  - vcc-supply: phandle to the regulator that provides power to the sensor.
-
-Example from a LG Nexus 5 (hammerhead) phone:
-
-vibrator@fd8c3450 {
-	reg = <0xfd8c3450 0x400>;
-	compatible = "qcom,msm8974-vibrator";
-
-	vcc-supply = <&pm8941_l19>;
-
-	clocks = <&mmcc CAMSS_GP1_CLK>;
-	clock-names = "pwm";
-
-	enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
-
-	pinctrl-names = "default";
-	pinctrl-0 = <&vibrator_pin>;
-};
-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/7] Input: drop msm-vibrator in favor of clk-vibrator driver
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
  2019-12-05  0:24 ` [PATCH 1/7] clk: qcom: add support for setting the duty cycle Brian Masney
  2019-12-05  0:24 ` [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator Brian Masney
@ 2019-12-05  0:24 ` Brian Masney
  2019-12-05  0:25 ` [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings Brian Masney
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:24 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

The msm-vibrator driver is directly controlling the duty cycle of a
clock through register writes. Let's drop the msm-vibrator driver in
favor of using the more generic clk-vibrator driver that calls
clk_set_duty_cycle().

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/input/misc/Kconfig        |  10 --
 drivers/input/misc/Makefile       |   1 -
 drivers/input/misc/msm-vibrator.c | 281 ------------------------------
 3 files changed, 292 deletions(-)
 delete mode 100644 drivers/input/misc/msm-vibrator.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 7e2e658d551c..b56da7a5efb9 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -117,16 +117,6 @@ config INPUT_E3X0_BUTTON
 	  To compile this driver as a module, choose M here: the
 	  module will be called e3x0_button.
 
-config INPUT_MSM_VIBRATOR
-	tristate "Qualcomm MSM vibrator driver"
-	select INPUT_FF_MEMLESS
-	help
-	  Support for the vibrator that is found on various Qualcomm MSM
-	  SOCs.
-
-	  To compile this driver as a module, choose M here: the module
-	  will be called msm_vibrator.
-
 config INPUT_PCSPKR
 	tristate "PC Speaker support"
 	depends on PCSPKR_PLATFORM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 8fd187f314bd..e6768b61a955 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -50,7 +50,6 @@ obj-$(CONFIG_INPUT_MAX8925_ONKEY)	+= max8925_onkey.o
 obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
 obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
 obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
-obj-$(CONFIG_INPUT_MSM_VIBRATOR)	+= msm-vibrator.o
 obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
diff --git a/drivers/input/misc/msm-vibrator.c b/drivers/input/misc/msm-vibrator.c
deleted file mode 100644
index b60f1aaee705..000000000000
--- a/drivers/input/misc/msm-vibrator.c
+++ /dev/null
@@ -1,281 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Qualcomm MSM vibrator driver
- *
- * Copyright (c) 2018 Brian Masney <masneyb@onstation.org>
- *
- * Based on qcom,pwm-vibrator.c from:
- * Copyright (c) 2018 Jonathan Marek <jonathan@marek.ca>
- *
- * Based on msm_pwm_vibrator.c from downstream Android sources:
- * Copyright (C) 2009-2014 LGE, Inc.
- */
-
-#include <linux/clk.h>
-#include <linux/err.h>
-#include <linux/gpio/consumer.h>
-#include <linux/input.h>
-#include <linux/io.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/regulator/consumer.h>
-
-#define REG_CMD_RCGR           0x00
-#define REG_CFG_RCGR           0x04
-#define REG_M                  0x08
-#define REG_N                  0x0C
-#define REG_D                  0x10
-#define REG_CBCR               0x24
-#define MMSS_CC_M_DEFAULT      1
-
-struct msm_vibrator {
-	struct input_dev *input;
-	struct mutex mutex;
-	struct work_struct worker;
-	void __iomem *base;
-	struct regulator *vcc;
-	struct clk *clk;
-	struct gpio_desc *enable_gpio;
-	u16 magnitude;
-	bool enabled;
-};
-
-static void msm_vibrator_write(struct msm_vibrator *vibrator, int offset,
-			       u32 value)
-{
-	writel(value, vibrator->base + offset);
-}
-
-static int msm_vibrator_start(struct msm_vibrator *vibrator)
-{
-	int d_reg_val, ret = 0;
-
-	mutex_lock(&vibrator->mutex);
-
-	if (!vibrator->enabled) {
-		ret = clk_set_rate(vibrator->clk, 24000);
-		if (ret) {
-			dev_err(&vibrator->input->dev,
-				"Failed to set clock rate: %d\n", ret);
-			goto unlock;
-		}
-
-		ret = clk_prepare_enable(vibrator->clk);
-		if (ret) {
-			dev_err(&vibrator->input->dev,
-				"Failed to enable clock: %d\n", ret);
-			goto unlock;
-		}
-
-		ret = regulator_enable(vibrator->vcc);
-		if (ret) {
-			dev_err(&vibrator->input->dev,
-				"Failed to enable regulator: %d\n", ret);
-			clk_disable(vibrator->clk);
-			goto unlock;
-		}
-
-		gpiod_set_value_cansleep(vibrator->enable_gpio, 1);
-
-		vibrator->enabled = true;
-	}
-
-	d_reg_val = 127 - ((126 * vibrator->magnitude) / 0xffff);
-	msm_vibrator_write(vibrator, REG_CFG_RCGR,
-			   (2 << 12) | /* dual edge mode */
-			   (0 << 8) |  /* cxo */
-			   (7 << 0));
-	msm_vibrator_write(vibrator, REG_M, 1);
-	msm_vibrator_write(vibrator, REG_N, 128);
-	msm_vibrator_write(vibrator, REG_D, d_reg_val);
-	msm_vibrator_write(vibrator, REG_CMD_RCGR, 1);
-	msm_vibrator_write(vibrator, REG_CBCR, 1);
-
-unlock:
-	mutex_unlock(&vibrator->mutex);
-
-	return ret;
-}
-
-static void msm_vibrator_stop(struct msm_vibrator *vibrator)
-{
-	mutex_lock(&vibrator->mutex);
-
-	if (vibrator->enabled) {
-		gpiod_set_value_cansleep(vibrator->enable_gpio, 0);
-		regulator_disable(vibrator->vcc);
-		clk_disable(vibrator->clk);
-		vibrator->enabled = false;
-	}
-
-	mutex_unlock(&vibrator->mutex);
-}
-
-static void msm_vibrator_worker(struct work_struct *work)
-{
-	struct msm_vibrator *vibrator = container_of(work,
-						     struct msm_vibrator,
-						     worker);
-
-	if (vibrator->magnitude)
-		msm_vibrator_start(vibrator);
-	else
-		msm_vibrator_stop(vibrator);
-}
-
-static int msm_vibrator_play_effect(struct input_dev *dev, void *data,
-				    struct ff_effect *effect)
-{
-	struct msm_vibrator *vibrator = input_get_drvdata(dev);
-
-	mutex_lock(&vibrator->mutex);
-
-	if (effect->u.rumble.strong_magnitude > 0)
-		vibrator->magnitude = effect->u.rumble.strong_magnitude;
-	else
-		vibrator->magnitude = effect->u.rumble.weak_magnitude;
-
-	mutex_unlock(&vibrator->mutex);
-
-	schedule_work(&vibrator->worker);
-
-	return 0;
-}
-
-static void msm_vibrator_close(struct input_dev *input)
-{
-	struct msm_vibrator *vibrator = input_get_drvdata(input);
-
-	cancel_work_sync(&vibrator->worker);
-	msm_vibrator_stop(vibrator);
-}
-
-static int msm_vibrator_probe(struct platform_device *pdev)
-{
-	struct msm_vibrator *vibrator;
-	struct resource *res;
-	int ret;
-
-	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
-	if (!vibrator)
-		return -ENOMEM;
-
-	vibrator->input = devm_input_allocate_device(&pdev->dev);
-	if (!vibrator->input)
-		return -ENOMEM;
-
-	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
-	if (IS_ERR(vibrator->vcc)) {
-		if (PTR_ERR(vibrator->vcc) != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "Failed to get regulator: %ld\n",
-				PTR_ERR(vibrator->vcc));
-		return PTR_ERR(vibrator->vcc);
-	}
-
-	vibrator->enable_gpio = devm_gpiod_get(&pdev->dev, "enable",
-					       GPIOD_OUT_LOW);
-	if (IS_ERR(vibrator->enable_gpio)) {
-		if (PTR_ERR(vibrator->enable_gpio) != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "Failed to get enable gpio: %ld\n",
-				PTR_ERR(vibrator->enable_gpio));
-		return PTR_ERR(vibrator->enable_gpio);
-	}
-
-	vibrator->clk = devm_clk_get(&pdev->dev, "pwm");
-	if (IS_ERR(vibrator->clk)) {
-		if (PTR_ERR(vibrator->clk) != -EPROBE_DEFER)
-			dev_err(&pdev->dev, "Failed to lookup pwm clock: %ld\n",
-				PTR_ERR(vibrator->clk));
-		return PTR_ERR(vibrator->clk);
-	}
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "Failed to get platform resource\n");
-		return -ENODEV;
-	}
-
-	vibrator->base = devm_ioremap(&pdev->dev, res->start,
-				     resource_size(res));
-	if (!vibrator->base) {
-		dev_err(&pdev->dev, "Failed to iomap resource.\n");
-		return -ENOMEM;
-	}
-
-	vibrator->enabled = false;
-	mutex_init(&vibrator->mutex);
-	INIT_WORK(&vibrator->worker, msm_vibrator_worker);
-
-	vibrator->input->name = "msm-vibrator";
-	vibrator->input->id.bustype = BUS_HOST;
-	vibrator->input->close = msm_vibrator_close;
-
-	input_set_drvdata(vibrator->input, vibrator);
-	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
-
-	ret = input_ff_create_memless(vibrator->input, NULL,
-				      msm_vibrator_play_effect);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to create ff memless: %d", ret);
-		return ret;
-	}
-
-	ret = input_register_device(vibrator->input);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to register input device: %d", ret);
-		return ret;
-	}
-
-	platform_set_drvdata(pdev, vibrator);
-
-	return 0;
-}
-
-static int __maybe_unused msm_vibrator_suspend(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct msm_vibrator *vibrator = platform_get_drvdata(pdev);
-
-	cancel_work_sync(&vibrator->worker);
-
-	if (vibrator->enabled)
-		msm_vibrator_stop(vibrator);
-
-	return 0;
-}
-
-static int __maybe_unused msm_vibrator_resume(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct msm_vibrator *vibrator = platform_get_drvdata(pdev);
-
-	if (vibrator->enabled)
-		msm_vibrator_start(vibrator);
-
-	return 0;
-}
-
-static SIMPLE_DEV_PM_OPS(msm_vibrator_pm_ops, msm_vibrator_suspend,
-			 msm_vibrator_resume);
-
-static const struct of_device_id msm_vibrator_of_match[] = {
-	{ .compatible = "qcom,msm8226-vibrator" },
-	{ .compatible = "qcom,msm8974-vibrator" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, msm_vibrator_of_match);
-
-static struct platform_driver msm_vibrator_driver = {
-	.probe	= msm_vibrator_probe,
-	.driver	= {
-		.name = "msm-vibrator",
-		.pm = &msm_vibrator_pm_ops,
-		.of_match_table = of_match_ptr(msm_vibrator_of_match),
-	},
-};
-module_platform_driver(msm_vibrator_driver);
-
-MODULE_AUTHOR("Brian Masney <masneyb@onstation.org>");
-MODULE_DESCRIPTION("Qualcomm MSM vibrator driver");
-MODULE_LICENSE("GPL");
-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
                   ` (2 preceding siblings ...)
  2019-12-05  0:24 ` [PATCH 3/7] Input: drop msm-vibrator in favor of clk-vibrator driver Brian Masney
@ 2019-12-05  0:25 ` Brian Masney
  2019-12-05 13:56   ` Rob Herring
  2020-01-05  8:35   ` Stephen Boyd
  2019-12-05  0:25 ` [PATCH 5/7] Input: introduce new clock vibrator driver Brian Masney
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:25 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Add support for clock-based vibrator devices where the speed can be
controlled by changing the duty cycle.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../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 <masneyb@onstation.org>
+
+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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 5/7] Input: introduce new clock vibrator driver
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
                   ` (3 preceding siblings ...)
  2019-12-05  0:25 ` [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings Brian Masney
@ 2019-12-05  0:25 ` Brian Masney
  2019-12-05  0:25 ` [PATCH 6/7] ARM: qcom_defconfig: drop msm-vibrator in favor of clk-vibrator driver Brian Masney
  2019-12-05  0:25 ` [PATCH 7/7] ARM: dts: qcom: msm8974-hammerhead: add support for vibrator Brian Masney
  6 siblings, 0 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:25 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Add support for clock-based vibrator devices where the speed can be
controlled by changing the duty cycle.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 drivers/input/misc/Kconfig        |  10 ++
 drivers/input/misc/Makefile       |   1 +
 drivers/input/misc/clk-vibrator.c | 245 ++++++++++++++++++++++++++++++
 3 files changed, 256 insertions(+)
 create mode 100644 drivers/input/misc/clk-vibrator.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b56da7a5efb9..8c95b927bce6 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -425,6 +425,16 @@ config INPUT_YEALINK
 	  To compile this driver as a module, choose M here: the module will be
 	  called yealink.
 
+config INPUT_CLK_VIBRATOR
+	tristate "Clock vibrator driver"
+	select INPUT_FF_MEMLESS
+	help
+	  Support for clock-based vibrator devices where the speed can be
+	  controlled by changing the duty cycle.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called clk_vibrator.
+
 config INPUT_CM109
 	tristate "C-Media CM109 USB I/O Controller"
 	depends on USB_ARCH_HAS_HCD
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index e6768b61a955..ca8a33cd91a5 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
 obj-$(CONFIG_INPUT_ATMEL_CAPTOUCH)	+= atmel_captouch.o
 obj-$(CONFIG_INPUT_BMA150)		+= bma150.o
+obj-$(CONFIG_INPUT_CLK_VIBRATOR)	+= clk-vibrator.o
 obj-$(CONFIG_INPUT_CM109)		+= cm109.o
 obj-$(CONFIG_INPUT_CMA3000)		+= cma3000_d0x.o
 obj-$(CONFIG_INPUT_CMA3000_I2C)		+= cma3000_d0x_i2c.o
diff --git a/drivers/input/misc/clk-vibrator.c b/drivers/input/misc/clk-vibrator.c
new file mode 100644
index 000000000000..71b7bd0f9b42
--- /dev/null
+++ b/drivers/input/misc/clk-vibrator.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Clock vibrator driver
+ *
+ * Copyright (c) 2019 Brian Masney <masneyb@onstation.org>
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct clk_vibrator {
+	struct input_dev *input;
+	struct mutex mutex;
+	struct work_struct worker;
+	struct regulator *vcc;
+	struct clk *clk;
+	u32 clk_rate;
+	struct gpio_desc *enable_gpio;
+	u16 magnitude;
+	bool enabled;
+};
+
+static int clk_vibrator_start(struct clk_vibrator *vibrator)
+{
+	int ret;
+
+	mutex_lock(&vibrator->mutex);
+
+	if (!vibrator->enabled) {
+		ret = clk_set_rate(vibrator->clk, vibrator->clk_rate);
+		if (ret) {
+			dev_err(&vibrator->input->dev,
+				"Failed to set clock rate: %d\n", ret);
+			goto unlock;
+		}
+
+		ret = clk_prepare_enable(vibrator->clk);
+		if (ret) {
+			dev_err(&vibrator->input->dev,
+				"Failed to enable clock: %d\n", ret);
+			goto unlock;
+		}
+
+		ret = regulator_enable(vibrator->vcc);
+		if (ret) {
+			dev_err(&vibrator->input->dev,
+				"Failed to enable regulator: %d\n", ret);
+			clk_disable(vibrator->clk);
+			goto unlock;
+		}
+
+		gpiod_set_value_cansleep(vibrator->enable_gpio, 1);
+
+		vibrator->enabled = true;
+	}
+
+	ret = clk_set_duty_cycle(vibrator->clk, vibrator->magnitude, 0xffff);
+
+unlock:
+	mutex_unlock(&vibrator->mutex);
+
+	return ret;
+}
+
+static void clk_vibrator_stop(struct clk_vibrator *vibrator)
+{
+	mutex_lock(&vibrator->mutex);
+
+	if (vibrator->enabled) {
+		gpiod_set_value_cansleep(vibrator->enable_gpio, 0);
+		regulator_disable(vibrator->vcc);
+		clk_disable(vibrator->clk);
+		vibrator->enabled = false;
+	}
+
+	mutex_unlock(&vibrator->mutex);
+}
+
+static void clk_vibrator_worker(struct work_struct *work)
+{
+	struct clk_vibrator *vibrator = container_of(work,
+						     struct clk_vibrator,
+						     worker);
+
+	if (vibrator->magnitude)
+		clk_vibrator_start(vibrator);
+	else
+		clk_vibrator_stop(vibrator);
+}
+
+static int clk_vibrator_play_effect(struct input_dev *dev, void *data,
+				    struct ff_effect *effect)
+{
+	struct clk_vibrator *vibrator = input_get_drvdata(dev);
+
+	mutex_lock(&vibrator->mutex);
+
+	if (effect->u.rumble.strong_magnitude > 0)
+		vibrator->magnitude = effect->u.rumble.strong_magnitude;
+	else
+		vibrator->magnitude = effect->u.rumble.weak_magnitude;
+
+	mutex_unlock(&vibrator->mutex);
+
+	schedule_work(&vibrator->worker);
+
+	return 0;
+}
+
+static void clk_vibrator_close(struct input_dev *input)
+{
+	struct clk_vibrator *vibrator = input_get_drvdata(input);
+
+	cancel_work_sync(&vibrator->worker);
+	clk_vibrator_stop(vibrator);
+}
+
+static int clk_vibrator_probe(struct platform_device *pdev)
+{
+	struct clk_vibrator *vibrator;
+	int ret;
+
+	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
+	if (!vibrator)
+		return -ENOMEM;
+
+	vibrator->input = devm_input_allocate_device(&pdev->dev);
+	if (!vibrator->input)
+		return -ENOMEM;
+
+	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
+	if (IS_ERR(vibrator->vcc)) {
+		if (PTR_ERR(vibrator->vcc) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to get regulator: %ld\n",
+				PTR_ERR(vibrator->vcc));
+		return PTR_ERR(vibrator->vcc);
+	}
+
+	vibrator->enable_gpio = devm_gpiod_get(&pdev->dev, "enable",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(vibrator->enable_gpio)) {
+		if (PTR_ERR(vibrator->enable_gpio) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to get enable gpio: %ld\n",
+				PTR_ERR(vibrator->enable_gpio));
+		return PTR_ERR(vibrator->enable_gpio);
+	}
+
+	vibrator->clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(vibrator->clk)) {
+		if (PTR_ERR(vibrator->clk) != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"Failed to lookup core clock: %ld\n",
+				PTR_ERR(vibrator->clk));
+		return PTR_ERR(vibrator->clk);
+	}
+
+	ret = of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+				   &vibrator->clk_rate);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot read clock-frequency: %d\n", ret);
+		return ret;
+	}
+
+	vibrator->enabled = false;
+	mutex_init(&vibrator->mutex);
+	INIT_WORK(&vibrator->worker, clk_vibrator_worker);
+
+	vibrator->input->name = "clk-vibrator";
+	vibrator->input->id.bustype = BUS_HOST;
+	vibrator->input->close = clk_vibrator_close;
+
+	input_set_drvdata(vibrator->input, vibrator);
+	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
+
+	ret = input_ff_create_memless(vibrator->input, NULL,
+				      clk_vibrator_play_effect);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to create ff memless: %d", ret);
+		return ret;
+	}
+
+	ret = input_register_device(vibrator->input);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register input device: %d", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, vibrator);
+
+	return 0;
+}
+
+static int __maybe_unused clk_vibrator_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct clk_vibrator *vibrator = platform_get_drvdata(pdev);
+
+	cancel_work_sync(&vibrator->worker);
+
+	if (vibrator->enabled)
+		clk_vibrator_stop(vibrator);
+
+	return 0;
+}
+
+static int __maybe_unused clk_vibrator_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct clk_vibrator *vibrator = platform_get_drvdata(pdev);
+
+	if (vibrator->enabled)
+		clk_vibrator_start(vibrator);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(clk_vibrator_pm_ops, clk_vibrator_suspend,
+			 clk_vibrator_resume);
+
+static const struct of_device_id clk_vibrator_of_match[] = {
+	{ .compatible = "clk-vibrator" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, clk_vibrator_of_match);
+
+static struct platform_driver clk_vibrator_driver = {
+	.probe	= clk_vibrator_probe,
+	.driver	= {
+		.name = "clk-vibrator",
+		.pm = &clk_vibrator_pm_ops,
+		.of_match_table = of_match_ptr(clk_vibrator_of_match),
+	},
+};
+module_platform_driver(clk_vibrator_driver);
+
+MODULE_AUTHOR("Brian Masney <masneyb@onstation.org>");
+MODULE_DESCRIPTION("Clock vibrator driver");
+MODULE_LICENSE("GPL");
-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 6/7] ARM: qcom_defconfig: drop msm-vibrator in favor of clk-vibrator driver
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
                   ` (4 preceding siblings ...)
  2019-12-05  0:25 ` [PATCH 5/7] Input: introduce new clock vibrator driver Brian Masney
@ 2019-12-05  0:25 ` Brian Masney
  2019-12-05  0:25 ` [PATCH 7/7] ARM: dts: qcom: msm8974-hammerhead: add support for vibrator Brian Masney
  6 siblings, 0 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:25 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

The msm-vibrator driver no longer exists, so let's enable the more
generic clk-vibrator driver instead.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 arch/arm/configs/qcom_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index 201e20bc6189..6c7c42ffe5a4 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -99,7 +99,7 @@ CONFIG_KEYBOARD_PMIC8XXX=y
 CONFIG_INPUT_JOYSTICK=y
 CONFIG_INPUT_TOUCHSCREEN=y
 CONFIG_INPUT_MISC=y
-CONFIG_INPUT_MSM_VIBRATOR=m
+CONFIG_INPUT_CLK_VIBRATOR=m
 CONFIG_INPUT_PM8941_PWRKEY=m
 CONFIG_INPUT_PM8XXX_VIBRATOR=y
 CONFIG_INPUT_PMIC8XXX_PWRKEY=y
-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 7/7] ARM: dts: qcom: msm8974-hammerhead: add support for vibrator
  2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
                   ` (5 preceding siblings ...)
  2019-12-05  0:25 ` [PATCH 6/7] ARM: qcom_defconfig: drop msm-vibrator in favor of clk-vibrator driver Brian Masney
@ 2019-12-05  0:25 ` Brian Masney
  6 siblings, 0 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-05  0:25 UTC (permalink / raw)
  To: sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Add support for the vibrator found on the Nexus 5 phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 797a43be844e..e17ea4f602c1 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -234,6 +234,21 @@
 		pinctrl-names = "default";
 		pinctrl-0 = <&wlan_regulator_pin>;
 	};
+
+	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>;
+	};
 };
 
 &soc {
@@ -355,6 +370,21 @@
 				bias-disable;
 			};
 		};
+
+		vibrator_pin: vibrator {
+			core {
+				pins = "gpio27";
+				function = "gp1_clk";
+
+				drive-strength = <6>;
+				bias-disable;
+			};
+
+			enable {
+				pins = "gpio60";
+				function = "gpio";
+			};
+		};
 	};
 
 	sdhci@f9824900 {
-- 
2.21.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2019-12-05  0:25 ` [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings Brian Masney
@ 2019-12-05 13:56   ` Rob Herring
  2019-12-09  0:54     ` Brian Masney
  2020-01-05  8:35   ` Stephen Boyd
  1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2019-12-05 13:56 UTC (permalink / raw)
  To: Brian Masney
  Cc: Stephen Boyd, Dmitry Torokhov, Mark Rutland, Andy Gross,
	Bjorn Andersson, Michael Turquette, Linux Input, devicetree,
	linux-kernel, linux-arm-msm, linux-clk

On Wed, Dec 4, 2019 at 6:25 PM Brian Masney <masneyb@onstation.org> wrote:
>
> Add support for clock-based vibrator devices where the speed can be
> controlled by changing the duty cycle.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../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 <masneyb@onstation.org>
> +
> +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
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2019-12-05 13:56   ` Rob Herring
@ 2019-12-09  0:54     ` Brian Masney
  2019-12-09 16:16       ` Rob Herring
  0 siblings, 1 reply; 20+ messages in thread
From: Brian Masney @ 2019-12-09  0:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Stephen Boyd, Dmitry Torokhov, Mark Rutland, Andy Gross,
	Bjorn Andersson, Michael Turquette, Linux Input, devicetree,
	linux-kernel, linux-arm-msm, linux-clk

On Thu, Dec 05, 2019 at 07:56:10AM -0600, Rob Herring wrote:
> On Wed, Dec 4, 2019 at 6:25 PM Brian Masney <masneyb@onstation.org> wrote:
> >
> > Add support for clock-based vibrator devices where the speed can be
> > controlled by changing the duty cycle.
> >
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > ---
> >  .../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 <masneyb@onstation.org>
> > +
> > +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
> >

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2019-12-09  0:54     ` Brian Masney
@ 2019-12-09 16:16       ` Rob Herring
  2019-12-09 16:55         ` Brian Masney
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2019-12-09 16:16 UTC (permalink / raw)
  To: Brian Masney
  Cc: Stephen Boyd, Dmitry Torokhov, Mark Rutland, Andy Gross,
	Bjorn Andersson, Michael Turquette, Linux Input, devicetree,
	linux-kernel, linux-arm-msm, linux-clk

On Sun, Dec 8, 2019 at 6:54 PM Brian Masney <masneyb@onstation.org> 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 <masneyb@onstation.org> wrote:
> > >
> > > Add support for clock-based vibrator devices where the speed can be
> > > controlled by changing the duty cycle.
> > >
> > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > ---
> > >  .../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 <masneyb@onstation.org>
> > > +
> > > +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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2019-12-09 16:16       ` Rob Herring
@ 2019-12-09 16:55         ` Brian Masney
  0 siblings, 0 replies; 20+ messages in thread
From: Brian Masney @ 2019-12-09 16:55 UTC (permalink / raw)
  To: Rob Herring, Stephen Boyd
  Cc: Dmitry Torokhov, Mark Rutland, Andy Gross, Bjorn Andersson,
	Michael Turquette, Linux Input, devicetree, linux-kernel,
	linux-arm-msm, linux-clk, Linus Walleij, thierry.reding

On Mon, Dec 09, 2019 at 10:16:26AM -0600, Rob Herring wrote:
> On Sun, Dec 8, 2019 at 6:54 PM Brian Masney <masneyb@onstation.org> 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 <masneyb@onstation.org> wrote:
> > > >
> > > > Add support for clock-based vibrator devices where the speed can be
> > > > controlled by changing the duty cycle.
> > > >
> > > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > > ---
> > > >  .../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 <masneyb@onstation.org>
> > > > +
> > > > +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/20190627234929.B78E520815@mail.kernel.org/

Brian

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle
  2019-12-05  0:24 ` [PATCH 1/7] clk: qcom: add support for setting the duty cycle Brian Masney
@ 2019-12-10  4:47   ` Taniya Das
       [not found]   ` <0101016eee224b50-8a5545e2-837f-41c2-9574-b385e111a6b3-000000@us-west-2.amazonses.com>
  1 sibling, 0 replies; 20+ messages in thread
From: Taniya Das @ 2019-12-10  4:47 UTC (permalink / raw)
  To: Brian Masney, sboyd, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Hi Brian,

On 12/5/2019 5:54 AM, Brian Masney wrote:
> Add support for setting the duty cycle via the D register for the
> Qualcomm clocks.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
> A few quirks that were noted when varying the speed of CAMSS_GP1_CLK on
> msm8974 (Nexus 5 phone):
> 
>    - The mnd_width is set to 8 bits, however the d width is actually 7
>      bits, at least for this clock. I'm not sure about the other clocks.
> 
>    - When the d register has a value less than 17, the following error
>      from update_config() is shown: rcg didn't update its configuration.
>      So this patch keeps the value of the d register within the range
>      [17, 127].
> 
> I'm not sure about the relationship of the m, n, and d values,
> especially how the 50% duty cycle is calculated by inverting the n
> value, so that's why I'm saving the duty cycle ratio for
> get_duty_cycle().
> 
>   drivers/clk/qcom/clk-rcg.h  |  4 +++
>   drivers/clk/qcom/clk-rcg2.c | 61 +++++++++++++++++++++++++++++++++++--
>   2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 78358b81d249..c3b8732cec8f 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -139,6 +139,8 @@ extern const struct clk_ops clk_dyn_rcg_ops;
>    * @freq_tbl: frequency table
>    * @clkr: regmap clock handle
>    * @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
> + * @duty_cycle_num: Numerator of the duty cycle ratio
> + * @duty_cycle_den: Denominator of the duty cycle ratio
>    */
>   struct clk_rcg2 {
>   	u32			cmd_rcgr;
> @@ -149,6 +151,8 @@ struct clk_rcg2 {
>   	const struct freq_tbl	*freq_tbl;
>   	struct clk_regmap	clkr;
>   	u8			cfg_off;
> +	int			duty_cycle_num;
> +	int			duty_cycle_den;
>   };
>   
>   #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 8f4b9bec2956..8d685baefe50 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -258,7 +258,11 @@ static int clk_rcg2_determine_floor_rate(struct clk_hw *hw,
>   	return _freq_tbl_determine_rate(hw, rcg->freq_tbl, req, FLOOR);
>   }
>   
> -static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> +static int __clk_rcg2_configure_with_duty_cycle(struct clk_rcg2 *rcg,
> +						const struct freq_tbl *f,
> +						int d_reg_val,
> +						int duty_cycle_num,
> +						int duty_cycle_den)
>   {
>   	u32 cfg, mask;
>   	struct clk_hw *hw = &rcg->clkr.hw;
> @@ -280,9 +284,12 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>   			return ret;
>   
>   		ret = regmap_update_bits(rcg->clkr.regmap,
> -				RCG_D_OFFSET(rcg), mask, ~f->n);
> +				RCG_D_OFFSET(rcg), mask, d_reg_val);
>   		if (ret)
>   			return ret;
> +
> +		rcg->duty_cycle_num = duty_cycle_num;
> +		rcg->duty_cycle_den = duty_cycle_den;
>   	}
>   
>   	mask = BIT(rcg->hid_width) - 1;
> @@ -295,6 +302,12 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>   					mask, cfg);
>   }
>   
> +static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> +{
> +	/* Set a 50% duty cycle */
> +	return __clk_rcg2_configure_with_duty_cycle(rcg, f, ~f->n, 1, 2);
> +}
> +
>   static int clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>   {
>   	int ret;
> @@ -353,6 +366,32 @@ static int clk_rcg2_set_floor_rate_and_parent(struct clk_hw *hw,
>   	return __clk_rcg2_set_rate(hw, rate, FLOOR);
>   }
>   
> +static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
> +{
> +	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +
> +	duty->num = rcg->duty_cycle_num;
> +	duty->den = rcg->duty_cycle_den;
> +
> +	return 0;
> +}
> +
> +static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
> +{
> +	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +	int ret, d_reg_val, mask;
> +
> +	mask = BIT(rcg->mnd_width - 1) - 1;
> +	d_reg_val = mask - (((mask - 17) * duty->num) / duty->den);
> +	ret = __clk_rcg2_configure_with_duty_cycle(rcg, rcg->freq_tbl,
> +						   d_reg_val, duty->num,
> +						   duty->den);

The duty-cycle calculation is not accurate.
There is already a plan to submit the duty-cycle changes from my side.
> +	if (ret)
> +		return ret;
> +
> +	return update_config(rcg);
> +}
> +
>   const struct clk_ops clk_rcg2_ops = {
>   	.is_enabled = clk_rcg2_is_enabled,
>   	.get_parent = clk_rcg2_get_parent,
> @@ -361,6 +400,8 @@ const struct clk_ops clk_rcg2_ops = {
>   	.determine_rate = clk_rcg2_determine_rate,
>   	.set_rate = clk_rcg2_set_rate,
>   	.set_rate_and_parent = clk_rcg2_set_rate_and_parent,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_rcg2_ops);
>   
> @@ -372,6 +413,8 @@ const struct clk_ops clk_rcg2_floor_ops = {
>   	.determine_rate = clk_rcg2_determine_floor_rate,
>   	.set_rate = clk_rcg2_set_floor_rate,
>   	.set_rate_and_parent = clk_rcg2_set_floor_rate_and_parent,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_rcg2_floor_ops);
>   
> @@ -499,6 +542,8 @@ const struct clk_ops clk_edp_pixel_ops = {
>   	.set_rate = clk_edp_pixel_set_rate,
>   	.set_rate_and_parent = clk_edp_pixel_set_rate_and_parent,
>   	.determine_rate = clk_edp_pixel_determine_rate,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_edp_pixel_ops);
>   
> @@ -557,6 +602,8 @@ const struct clk_ops clk_byte_ops = {
>   	.set_rate = clk_byte_set_rate,
>   	.set_rate_and_parent = clk_byte_set_rate_and_parent,
>   	.determine_rate = clk_byte_determine_rate,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_byte_ops);
>   
> @@ -627,6 +674,8 @@ const struct clk_ops clk_byte2_ops = {
>   	.set_rate = clk_byte2_set_rate,
>   	.set_rate_and_parent = clk_byte2_set_rate_and_parent,
>   	.determine_rate = clk_byte2_determine_rate,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_byte2_ops);
>   
> @@ -717,6 +766,8 @@ const struct clk_ops clk_pixel_ops = {
>   	.set_rate = clk_pixel_set_rate,
>   	.set_rate_and_parent = clk_pixel_set_rate_and_parent,
>   	.determine_rate = clk_pixel_determine_rate,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_pixel_ops);
>   
> @@ -804,6 +855,8 @@ const struct clk_ops clk_gfx3d_ops = {
>   	.set_rate = clk_gfx3d_set_rate,
>   	.set_rate_and_parent = clk_gfx3d_set_rate_and_parent,
>   	.determine_rate = clk_gfx3d_determine_rate,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_gfx3d_ops);
>   
> @@ -942,6 +995,8 @@ const struct clk_ops clk_rcg2_shared_ops = {
>   	.determine_rate = clk_rcg2_determine_rate,
>   	.set_rate = clk_rcg2_shared_set_rate,
>   	.set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
>   EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
>   
> @@ -1081,6 +1136,8 @@ static const struct clk_ops clk_rcg2_dfs_ops = {
>   	.get_parent = clk_rcg2_get_parent,
>   	.determine_rate = clk_rcg2_dfs_determine_rate,
>   	.recalc_rate = clk_rcg2_dfs_recalc_rate,
> +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
>   };
> 

Why do you want to support duty-cycle for other RCGs when you are 
specifically want it for GP clocks only.
The DFS can never handle duty-cycle set/get.

>   static int clk_rcg2_enable_dfs(const struct clk_rcg_dfs_data *data,
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle
       [not found]   ` <0101016eee224b50-8a5545e2-837f-41c2-9574-b385e111a6b3-000000@us-west-2.amazonses.com>
@ 2019-12-10 11:51     ` Brian Masney
  2019-12-13 13:56       ` Linus Walleij
  0 siblings, 1 reply; 20+ messages in thread
From: Brian Masney @ 2019-12-10 11:51 UTC (permalink / raw)
  To: Taniya Das
  Cc: sboyd, dmitry.torokhov, robh+dt, mark.rutland, agross,
	bjorn.andersson, mturquette, linux-input, devicetree,
	linux-kernel, linux-arm-msm, linux-clk

Hi Taniya,

On Tue, Dec 10, 2019 at 04:47:35AM +0000, Taniya Das wrote:
> On 12/5/2019 5:54 AM, Brian Masney wrote:
> > Add support for setting the duty cycle via the D register for the
> > Qualcomm clocks.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > ---
> > A few quirks that were noted when varying the speed of CAMSS_GP1_CLK on
> > msm8974 (Nexus 5 phone):
> > 
> >    - The mnd_width is set to 8 bits, however the d width is actually 7
> >      bits, at least for this clock. I'm not sure about the other clocks.
> > 
> >    - When the d register has a value less than 17, the following error
> >      from update_config() is shown: rcg didn't update its configuration.
> >      So this patch keeps the value of the d register within the range
> >      [17, 127].
> > 
> > I'm not sure about the relationship of the m, n, and d values,
> > especially how the 50% duty cycle is calculated by inverting the n
> > value, so that's why I'm saving the duty cycle ratio for
> > get_duty_cycle().
> > 
> >   drivers/clk/qcom/clk-rcg.h  |  4 +++
> >   drivers/clk/qcom/clk-rcg2.c | 61 +++++++++++++++++++++++++++++++++++--
> >   2 files changed, 63 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> > index 78358b81d249..c3b8732cec8f 100644
> > --- a/drivers/clk/qcom/clk-rcg.h
> > +++ b/drivers/clk/qcom/clk-rcg.h
> > @@ -139,6 +139,8 @@ extern const struct clk_ops clk_dyn_rcg_ops;
> >    * @freq_tbl: frequency table
> >    * @clkr: regmap clock handle
> >    * @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
> > + * @duty_cycle_num: Numerator of the duty cycle ratio
> > + * @duty_cycle_den: Denominator of the duty cycle ratio
> >    */
> >   struct clk_rcg2 {
> >   	u32			cmd_rcgr;
> > @@ -149,6 +151,8 @@ struct clk_rcg2 {
> >   	const struct freq_tbl	*freq_tbl;
> >   	struct clk_regmap	clkr;
> >   	u8			cfg_off;
> > +	int			duty_cycle_num;
> > +	int			duty_cycle_den;
> >   };
> >   #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> > diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> > index 8f4b9bec2956..8d685baefe50 100644
> > --- a/drivers/clk/qcom/clk-rcg2.c
> > +++ b/drivers/clk/qcom/clk-rcg2.c
> > @@ -258,7 +258,11 @@ static int clk_rcg2_determine_floor_rate(struct clk_hw *hw,
> >   	return _freq_tbl_determine_rate(hw, rcg->freq_tbl, req, FLOOR);
> >   }
> > -static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> > +static int __clk_rcg2_configure_with_duty_cycle(struct clk_rcg2 *rcg,
> > +						const struct freq_tbl *f,
> > +						int d_reg_val,
> > +						int duty_cycle_num,
> > +						int duty_cycle_den)
> >   {
> >   	u32 cfg, mask;
> >   	struct clk_hw *hw = &rcg->clkr.hw;
> > @@ -280,9 +284,12 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >   			return ret;
> >   		ret = regmap_update_bits(rcg->clkr.regmap,
> > -				RCG_D_OFFSET(rcg), mask, ~f->n);
> > +				RCG_D_OFFSET(rcg), mask, d_reg_val);
> >   		if (ret)
> >   			return ret;
> > +
> > +		rcg->duty_cycle_num = duty_cycle_num;
> > +		rcg->duty_cycle_den = duty_cycle_den;
> >   	}
> >   	mask = BIT(rcg->hid_width) - 1;
> > @@ -295,6 +302,12 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >   					mask, cfg);
> >   }
> > +static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> > +{
> > +	/* Set a 50% duty cycle */
> > +	return __clk_rcg2_configure_with_duty_cycle(rcg, f, ~f->n, 1, 2);
> > +}
> > +
> >   static int clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >   {
> >   	int ret;
> > @@ -353,6 +366,32 @@ static int clk_rcg2_set_floor_rate_and_parent(struct clk_hw *hw,
> >   	return __clk_rcg2_set_rate(hw, rate, FLOOR);
> >   }
> > +static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
> > +{
> > +	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> > +
> > +	duty->num = rcg->duty_cycle_num;
> > +	duty->den = rcg->duty_cycle_den;
> > +
> > +	return 0;
> > +}
> > +
> > +static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
> > +{
> > +	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> > +	int ret, d_reg_val, mask;
> > +
> > +	mask = BIT(rcg->mnd_width - 1) - 1;
> > +	d_reg_val = mask - (((mask - 17) * duty->num) / duty->den);
> > +	ret = __clk_rcg2_configure_with_duty_cycle(rcg, rcg->freq_tbl,
> > +						   d_reg_val, duty->num,
> > +						   duty->den);
> 
> The duty-cycle calculation is not accurate.
> There is already a plan to submit the duty-cycle changes from my side.

OK... I assume that the m and n values need to be changed as well. I
couldn't find any docs online about the meaning of the m, n, and d
values and how they relate to each other.

Feel free to take over this patch if you find any value in what I posted
here.

> > +	if (ret)
> > +		return ret;
> > +
> > +	return update_config(rcg);
> > +}
> > +
> >   const struct clk_ops clk_rcg2_ops = {
> >   	.is_enabled = clk_rcg2_is_enabled,
> >   	.get_parent = clk_rcg2_get_parent,
> > @@ -361,6 +400,8 @@ const struct clk_ops clk_rcg2_ops = {
> >   	.determine_rate = clk_rcg2_determine_rate,
> >   	.set_rate = clk_rcg2_set_rate,
> >   	.set_rate_and_parent = clk_rcg2_set_rate_and_parent,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_rcg2_ops);
> > @@ -372,6 +413,8 @@ const struct clk_ops clk_rcg2_floor_ops = {
> >   	.determine_rate = clk_rcg2_determine_floor_rate,
> >   	.set_rate = clk_rcg2_set_floor_rate,
> >   	.set_rate_and_parent = clk_rcg2_set_floor_rate_and_parent,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_rcg2_floor_ops);
> > @@ -499,6 +542,8 @@ const struct clk_ops clk_edp_pixel_ops = {
> >   	.set_rate = clk_edp_pixel_set_rate,
> >   	.set_rate_and_parent = clk_edp_pixel_set_rate_and_parent,
> >   	.determine_rate = clk_edp_pixel_determine_rate,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_edp_pixel_ops);
> > @@ -557,6 +602,8 @@ const struct clk_ops clk_byte_ops = {
> >   	.set_rate = clk_byte_set_rate,
> >   	.set_rate_and_parent = clk_byte_set_rate_and_parent,
> >   	.determine_rate = clk_byte_determine_rate,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_byte_ops);
> > @@ -627,6 +674,8 @@ const struct clk_ops clk_byte2_ops = {
> >   	.set_rate = clk_byte2_set_rate,
> >   	.set_rate_and_parent = clk_byte2_set_rate_and_parent,
> >   	.determine_rate = clk_byte2_determine_rate,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_byte2_ops);
> > @@ -717,6 +766,8 @@ const struct clk_ops clk_pixel_ops = {
> >   	.set_rate = clk_pixel_set_rate,
> >   	.set_rate_and_parent = clk_pixel_set_rate_and_parent,
> >   	.determine_rate = clk_pixel_determine_rate,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_pixel_ops);
> > @@ -804,6 +855,8 @@ const struct clk_ops clk_gfx3d_ops = {
> >   	.set_rate = clk_gfx3d_set_rate,
> >   	.set_rate_and_parent = clk_gfx3d_set_rate_and_parent,
> >   	.determine_rate = clk_gfx3d_determine_rate,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_gfx3d_ops);
> > @@ -942,6 +995,8 @@ const struct clk_ops clk_rcg2_shared_ops = {
> >   	.determine_rate = clk_rcg2_determine_rate,
> >   	.set_rate = clk_rcg2_shared_set_rate,
> >   	.set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> >   EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
> > @@ -1081,6 +1136,8 @@ static const struct clk_ops clk_rcg2_dfs_ops = {
> >   	.get_parent = clk_rcg2_get_parent,
> >   	.determine_rate = clk_rcg2_dfs_determine_rate,
> >   	.recalc_rate = clk_rcg2_dfs_recalc_rate,
> > +	.get_duty_cycle = clk_rcg2_get_duty_cycle,
> > +	.set_duty_cycle = clk_rcg2_set_duty_cycle,
> >   };
> > 
> 
> Why do you want to support duty-cycle for other RCGs when you are
> specifically want it for GP clocks only.
> The DFS can never handle duty-cycle set/get.

I wrongly assumed that all of variants supported this. I did this
without any of the hardware documentation.

Brian

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle
  2019-12-10 11:51     ` Brian Masney
@ 2019-12-13 13:56       ` Linus Walleij
  0 siblings, 0 replies; 20+ messages in thread
From: Linus Walleij @ 2019-12-13 13:56 UTC (permalink / raw)
  To: Brian Masney
  Cc: Taniya Das, Stephen Boyd, Dmitry Torokhov, Rob Herring,
	Mark Rutland, Andy Gross, Bjorn Andersson, Michael Turquette,
	Linux Input,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, MSM, linux-clk

On Tue, Dec 10, 2019 at 12:52 PM Brian Masney <masneyb@onstation.org> wrote:
> On Tue, Dec 10, 2019 at 04:47:35AM +0000, Taniya Das wrote:
> > On 12/5/2019 5:54 AM, Brian Masney wrote:

> > > I'm not sure about the relationship of the m, n, and d values,
> > > especially how the 50% duty cycle is calculated by inverting the n
> > > value, so that's why I'm saving the duty cycle ratio for
> > > get_duty_cycle().
(...)
> > > +static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
> > > +{
> > > +   struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> > > +   int ret, d_reg_val, mask;
> > > +
> > > +   mask = BIT(rcg->mnd_width - 1) - 1;
> > > +   d_reg_val = mask - (((mask - 17) * duty->num) / duty->den);
> > > +   ret = __clk_rcg2_configure_with_duty_cycle(rcg, rcg->freq_tbl,
> > > +                                              d_reg_val, duty->num,
> > > +                                              duty->den);
> >
> > The duty-cycle calculation is not accurate.
> > There is already a plan to submit the duty-cycle changes from my side.
>
> OK... I assume that the m and n values need to be changed as well. I
> couldn't find any docs online about the meaning of the m, n, and d
> values and how they relate to each other.

I have also at times struggled to understand this.

If someone could just in a very concise form describe how these
rcg[2] clock dividers work and how m,n,d work that'd be GREAT.
ASCII art etc would be a bonus :)

Like with a patch with a big comment in
drivers/clk/qcom/clk-rcg.h

As these tend to be quite regularly reused and incarnated in
ever new silicon a complete picture for developers would be
much appreciated.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator
  2019-12-05  0:24 ` [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator Brian Masney
@ 2019-12-17 14:11   ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2019-12-17 14:11 UTC (permalink / raw)
  To: Brian Masney
  Cc: sboyd, dmitry.torokhov, robh+dt, mark.rutland, agross,
	bjorn.andersson, mturquette, linux-input, devicetree,
	linux-kernel, linux-arm-msm, linux-clk

On Wed,  4 Dec 2019 19:24:58 -0500, Brian Masney wrote:
> Let's drop the msm-vibrator bindings so that the more generic
> clk-vibrator can be used instead. No one is currently using these
> bindings so this won't affect any users.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../bindings/input/msm-vibrator.txt           | 36 -------------------
>  1 file changed, 36 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/msm-vibrator.txt
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2019-12-05  0:25 ` [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings Brian Masney
  2019-12-05 13:56   ` Rob Herring
@ 2020-01-05  8:35   ` Stephen Boyd
  2020-01-07 12:03     ` Brian Masney
  1 sibling, 1 reply; 20+ messages in thread
From: Stephen Boyd @ 2020-01-05  8:35 UTC (permalink / raw)
  To: Brian Masney, dmitry.torokhov, robh+dt
  Cc: mark.rutland, agross, bjorn.andersson, mturquette, linux-input,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

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 <masneyb@onstation.org>
> +
> +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.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2020-01-05  8:35   ` Stephen Boyd
@ 2020-01-07 12:03     ` Brian Masney
  2020-01-07 17:52       ` Stephen Boyd
  0 siblings, 1 reply; 20+ messages in thread
From: Brian Masney @ 2020-01-07 12:03 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: dmitry.torokhov, robh+dt, mark.rutland, agross, bjorn.andersson,
	mturquette, linux-input, devicetree, linux-kernel, linux-arm-msm,
	linux-clk

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2020-01-07 12:03     ` Brian Masney
@ 2020-01-07 17:52       ` Stephen Boyd
  2020-01-07 23:18         ` Brian Masney
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Boyd @ 2020-01-07 17:52 UTC (permalink / raw)
  To: Brian Masney
  Cc: dmitry.torokhov, robh+dt, mark.rutland, agross, bjorn.andersson,
	mturquette, linux-input, devicetree, linux-kernel, linux-arm-msm,
	linux-clk

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.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings
  2020-01-07 17:52       ` Stephen Boyd
@ 2020-01-07 23:18         ` Brian Masney
  0 siblings, 0 replies; 20+ messages in thread
From: Brian Masney @ 2020-01-07 23:18 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: dmitry.torokhov, robh+dt, mark.rutland, agross, bjorn.andersson,
	mturquette, linux-input, devicetree, linux-kernel, linux-arm-msm,
	linux-clk

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, back to index

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05  0:24 [PATCH 0/7] qcom: add clk-vibrator driver Brian Masney
2019-12-05  0:24 ` [PATCH 1/7] clk: qcom: add support for setting the duty cycle Brian Masney
2019-12-10  4:47   ` Taniya Das
     [not found]   ` <0101016eee224b50-8a5545e2-837f-41c2-9574-b385e111a6b3-000000@us-west-2.amazonses.com>
2019-12-10 11:51     ` Brian Masney
2019-12-13 13:56       ` Linus Walleij
2019-12-05  0:24 ` [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator Brian Masney
2019-12-17 14:11   ` Rob Herring
2019-12-05  0:24 ` [PATCH 3/7] Input: drop msm-vibrator in favor of clk-vibrator driver Brian Masney
2019-12-05  0:25 ` [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings Brian Masney
2019-12-05 13:56   ` Rob Herring
2019-12-09  0:54     ` Brian Masney
2019-12-09 16:16       ` Rob Herring
2019-12-09 16:55         ` Brian Masney
2020-01-05  8:35   ` Stephen Boyd
2020-01-07 12:03     ` Brian Masney
2020-01-07 17:52       ` Stephen Boyd
2020-01-07 23:18         ` Brian Masney
2019-12-05  0:25 ` [PATCH 5/7] Input: introduce new clock vibrator driver Brian Masney
2019-12-05  0:25 ` [PATCH 6/7] ARM: qcom_defconfig: drop msm-vibrator in favor of clk-vibrator driver Brian Masney
2019-12-05  0:25 ` [PATCH 7/7] ARM: dts: qcom: msm8974-hammerhead: add support for vibrator Brian Masney

Linux Input Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-input/0 linux-input/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-input linux-input/ https://lore.kernel.org/linux-input \
		linux-input@vger.kernel.org
	public-inbox-index linux-input

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-input


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git