[PATCH 0/7] qcom: add clk-vibrator driver

[PATCH 0/7] qcom: add clk-vibrator driver

[Brian Masney]

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

[PATCH 1/7] clk: qcom: add support for setting the duty cycle

[Brian Masney]

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

Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle

[Taniya Das]

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.

--

Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle

[Stephen Boyd]

Quoting Taniya Das (2019-12-09 20:47:35)
> Hi Brian,
> 
> On 12/5/2019 5:54 AM, Brian Masney wrote:
> > +     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.

What are the plans to submit this? Should we expect to see this support
in the next week? Month?

Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle

[Brian Masney]

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

Re: [PATCH 1/7] clk: qcom: add support for setting the duty cycle

[Linus Walleij]

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

[PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator

[Brian Masney]

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

Re: [PATCH 2/7] dt-bindings: Input: drop msm-vibrator in favor of clk-vibrator

[Rob Herring]

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>

[PATCH 3/7] Input: drop msm-vibrator in favor of clk-vibrator driver

[Brian Masney]

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

Re: [PATCH 3/7] Input: drop msm-vibrator in favor of clk-vibrator driver

[Brian Masney]

Hi Dmitry,

On Wed, Dec 04, 2019 at 07:24:59PM -0500, Brian Masney wrote:
> 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

I just sent out a version 2 of this patch that removes references to the
clk-vibrator driver in the commit description.

https://lore.kernel.org/lkml/20200211121318.144067-1-masneyb@onstation.org/

The msm-vibrator driver needs to be removed from upstream.

I'm waiting for someone from Qualcomm to either post a patch to support
setting the clock duty cycle or someone to post information about the
m,n,d registers for the clocks. Once that's done, no other changes
should be needed in the input subsystem.

Brian


> 
> 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

[PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Brian Masney]

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

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Rob Herring]

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
>

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Brian Masney]

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
> >

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Rob Herring]

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

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Brian Masney]

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

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Stephen Boyd]

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.

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Brian Masney]

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

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Stephen Boyd]

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.

Re: [PATCH 4/7] dt-bindings: Input: introduce new clock vibrator bindings

[Brian Masney]

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

[PATCH 5/7] Input: introduce new clock vibrator driver

[Brian Masney]

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

[PATCH 6/7] ARM: qcom_defconfig: drop msm-vibrator in favor of clk-vibrator driver

[Brian Masney]

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

[PATCH 7/7] ARM: dts: qcom: msm8974-hammerhead: add support for vibrator

[Brian Masney]

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