| Message ID | 20191205002503.13088-1-masneyb@onstation.org |
|---|---|
| Headers | show |
| Series | qcom: add clk-vibrator driver | expand |
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, >
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
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
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
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?
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%)