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
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
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
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
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
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
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
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
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 >
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 > >
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
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
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. --
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
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>
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.
On Sun, Jan 05, 2020 at 12:35:33AM -0800, Stephen Boyd wrote:
> Quoting Brian Masney (2019-12-04 16:25:00)
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > + #include <dt-bindings/gpio/gpio.h>
> > +
> > + vibrator {
> > + compatible = "clk-vibrator";
> > +
> > + vcc-supply = <&pm8941_l19>;
> > +
> > + clocks = <&mmcc CAMSS_GP1_CLK>;
> > + clock-names = "core";
> > + clock-frequency = <24000>;
> > +
> > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> > +
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&vibrator_pin>;
>
> I'm still trying to wrap my head around this. I think we can have a pwm
> provider in a clk controller node (so imagine &mmcc has #pwm-cells) and
> then this 'clk-vibrator' binding wouldn't exist? Instead we would have
> some sort of binding for a device that expects a pwm and whatever else
> is required, like the enable gpio and power supply. Is there an actual
> hardware block that is this way? Does it have a real product id and is
> made by some company? Right now this looks a little too generic to not
> just be a catch-all for something that buzzes.
So have some of the Qualcomm clocks like this one register with both the
clk and the pwm frameworks? I feel that approach would better represent
the hardware in device tree.
If we did that, then the pwm-vibra driver in the input subsystem could
be used.
Brian
Quoting Brian Masney (2020-01-07 04:03:17) > On Sun, Jan 05, 2020 at 12:35:33AM -0800, Stephen Boyd wrote: > > Quoting Brian Masney (2019-12-04 16:25:00) > > > +examples: > > > + - | > > > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h> > > > + #include <dt-bindings/gpio/gpio.h> > > > + > > > + vibrator { > > > + compatible = "clk-vibrator"; > > > + > > > + vcc-supply = <&pm8941_l19>; > > > + > > > + clocks = <&mmcc CAMSS_GP1_CLK>; > > > + clock-names = "core"; > > > + clock-frequency = <24000>; > > > + > > > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>; > > > + > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&vibrator_pin>; > > > > I'm still trying to wrap my head around this. I think we can have a pwm > > provider in a clk controller node (so imagine &mmcc has #pwm-cells) and > > then this 'clk-vibrator' binding wouldn't exist? Instead we would have > > some sort of binding for a device that expects a pwm and whatever else > > is required, like the enable gpio and power supply. Is there an actual > > hardware block that is this way? Does it have a real product id and is > > made by some company? Right now this looks a little too generic to not > > just be a catch-all for something that buzzes. > > So have some of the Qualcomm clocks like this one register with both the > clk and the pwm frameworks? I feel that approach would better represent > the hardware in device tree. That is one option. Or another option would be to have another node that "adapts" a clk signal to a pwm provider. Similar to how we adapt a gpio to make a clk gate or mux. Something like: gcc: clock-controller@f00d { reg = <0xf00d 0xd00d>; #clock-cells = <1>; }; pwm { compatible = "pwm-clk"; #pwm-cells = <0>; clocks = <&gcc 45>; assigned-clocks = <&gcc 45>; assigned-clock-rates = <1400000>; }; And then the pwm-clk driver would adjust the duty cycle to generate a pwm. > > If we did that, then the pwm-vibra driver in the input subsystem could > be used.
On Tue, Jan 07, 2020 at 09:52:21AM -0800, Stephen Boyd wrote:
> Quoting Brian Masney (2020-01-07 04:03:17)
> > On Sun, Jan 05, 2020 at 12:35:33AM -0800, Stephen Boyd wrote:
> > > Quoting Brian Masney (2019-12-04 16:25:00)
> > > > +examples:
> > > > + - |
> > > > + #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
> > > > + #include <dt-bindings/gpio/gpio.h>
> > > > +
> > > > + vibrator {
> > > > + compatible = "clk-vibrator";
> > > > +
> > > > + vcc-supply = <&pm8941_l19>;
> > > > +
> > > > + clocks = <&mmcc CAMSS_GP1_CLK>;
> > > > + clock-names = "core";
> > > > + clock-frequency = <24000>;
> > > > +
> > > > + enable-gpios = <&msmgpio 60 GPIO_ACTIVE_HIGH>;
> > > > +
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&vibrator_pin>;
> > >
> > > I'm still trying to wrap my head around this. I think we can have a pwm
> > > provider in a clk controller node (so imagine &mmcc has #pwm-cells) and
> > > then this 'clk-vibrator' binding wouldn't exist? Instead we would have
> > > some sort of binding for a device that expects a pwm and whatever else
> > > is required, like the enable gpio and power supply. Is there an actual
> > > hardware block that is this way? Does it have a real product id and is
> > > made by some company? Right now this looks a little too generic to not
> > > just be a catch-all for something that buzzes.
> >
> > So have some of the Qualcomm clocks like this one register with both the
> > clk and the pwm frameworks? I feel that approach would better represent
> > the hardware in device tree.
>
> That is one option. Or another option would be to have another node that
> "adapts" a clk signal to a pwm provider. Similar to how we adapt a gpio
> to make a clk gate or mux. Something like:
>
> gcc: clock-controller@f00d {
> reg = <0xf00d 0xd00d>;
> #clock-cells = <1>;
> };
>
>
> pwm {
> compatible = "pwm-clk";
> #pwm-cells = <0>;
> clocks = <&gcc 45>;
> assigned-clocks = <&gcc 45>;
> assigned-clock-rates = <1400000>;
> };
>
> And then the pwm-clk driver would adjust the duty cycle to generate a
> pwm.
OK, that makes sense.
I'll pick this up after someone from Qualcomm posts a patch that
implements the clock duty cycle. I'm willing to do that work if someone
explains the relationship between the m, n, and d values on these clocks.
Brian
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?