[PATCH v3 0/3] ARM: qcom: add vibrator support for various MSM SOCs

[PATCH v3 0/3] ARM: qcom: add vibrator support for various MSM SOCs

[Brian Masney]

This patch set adds support for the vibrator found on various Qualcomm
MSM SOCs. This is based on work from:

Jonathan Marek from qcom,pwm-vibrator.c in the PostmarketOS repo:
https://gitlab.com/postmarketOS/linux-postmarketos/commit/7647fb36cb1cbd060f8b52087a68ab93583292b5

Jongrak Kwon and Devin Kim from msm_pwm_vibrator.c in the downstream
Android 3.4.0 sources:
https://android.googlesource.com/kernel/msm/+/android-msm-lenok-3.10-lollipop-wear-release/drivers/misc/msm_pwm_vibrator.c

Driver was tested on a LG Nexus 5 (hammerhead) phone using rumble-test:
https://git.collabora.com/cgit/user/sre/rumble-test.git/plain/rumble-test.c

Changes since v2
- Moved from pwm to input subsystem based on feedback from
  https://lore.kernel.org/lkml/20181012114749.GC31561@ulmo/. I
  previously wired this into the input system via pwm-vibra.

Changes since v1
- Update device tree binding document based on feedback from Rob
  Herring.
- Changed the driver description to: Qualcomm PWM driver for the MSM
  vibrator.

Brian Masney (3):
  dt-bindings: Input: new bindings for MSM vibrator
  Input: add new vibrator driver for various MSM SOCs
  ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for
    vibrator

 .../bindings/input/msm-vibrator.txt           |  36 +++
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    |  31 ++
 drivers/input/misc/Kconfig                    |  10 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/msm-vibrator.c             | 276 ++++++++++++++++++
 5 files changed, 354 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/msm-vibrator.txt
 create mode 100644 drivers/input/misc/msm-vibrator.c

-- 
2.17.2

[PATCH v3 1/3] dt-bindings: Input: new bindings for MSM vibrator

[Brian Masney]

This patch adds the device tree bindings for the vibrator found on
various Qualcomm MSM SOCs.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../bindings/input/msm-vibrator.txt           | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create 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
new file mode 100644
index 000000000000..8dcf014ef2e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/msm-vibrator.txt
@@ -0,0 +1,36 @@
+* 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.17.2

[PATCH v3 2/3] Input: add new vibrator driver for various MSM SOCs

[Brian Masney]

This patch adds a new vibrator driver that supports various Qualcomm
MSM SOCs. Driver was tested on a LG Nexus 5 (hammerhead) phone.

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

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ca59a2be9bc5..e39aef84f357 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -117,6 +117,16 @@ 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 9d0f9d1ff68f..96a6419cb1f2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -48,6 +48,7 @@ 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
new file mode 100644
index 000000000000..bb862f30a780
--- /dev/null
+++ b/drivers/input/misc/msm-vibrator.c
@@ -0,0 +1,276 @@
+// 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.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;
+};
+
+#define msm_vibrator_write(msm_vibrator, offset, value) \
+	writel((value), (void __iomem *)((msm_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)) {
+		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)) {
+		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 (IS_ERR(vibrator->base)) {
+		dev_err(&pdev->dev, "Failed to iomap resource: %ld\n",
+			PTR_ERR(vibrator->base));
+		return PTR_ERR(vibrator->base);
+	}
+
+	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->dev.parent = &pdev->dev;
+	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 msm_vibrator *vibrator = dev_get_drvdata(dev);
+
+	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 msm_vibrator *vibrator = dev_get_drvdata(dev);
+
+	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.17.2

[PATCH v3 3/3] ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for vibrator

[Brian Masney]

This patch adds device device tree bindings for the vibrator found on
the LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 31 +++++++++++++++++++
 1 file changed, 31 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 ed8f064d0895..de956eea31ac 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -5,6 +5,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/clock/qcom,mmcc-msm8974.h>
 
 / {
 	model = "LGE MSM 8974 HAMMERHEAD";
@@ -268,6 +269,36 @@
 				input-enable;
 			};
 		};
+
+		vibrator_pin: vibrator {
+			pwm {
+				pins = "gpio27";
+				function = "gp1_clk";
+
+				drive-strength = <6>;
+				bias-disable;
+			};
+
+			enable {
+				pins = "gpio60";
+				function = "gpio";
+			};
+		};
+	};
+
+	vibrator@fd8c3450 {
+		compatible = "qcom,msm8974-vibrator";
+		reg = <0xfd8c3450 0x400>;
+
+		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>;
 	};
 
 	sdhci@f9824900 {
-- 
2.17.2

Re: [PATCH v3 1/3] dt-bindings: Input: new bindings for MSM vibrator

[Rob Herring]

On Wed, 24 Oct 2018 21:29:35 -0400, Brian Masney wrote:
> This patch adds the device tree bindings for the vibrator found on
> various Qualcomm MSM SOCs.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../bindings/input/msm-vibrator.txt           | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/msm-vibrator.txt
> 

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

Re: [PATCH v3 1/3] dt-bindings: Input: new bindings for MSM vibrator

[Rob Herring]

On Wed, 24 Oct 2018 21:29:35 -0400, Brian Masney wrote:
> This patch adds the device tree bindings for the vibrator found on
> various Qualcomm MSM SOCs.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../bindings/input/msm-vibrator.txt           | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/msm-vibrator.txt
> 

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

Re: [PATCH v3 2/3] Input: add new vibrator driver for various MSM SOCs

[Brian Masney]

Hi Dmitry,

On Wed, Oct 24, 2018 at 09:29:36PM -0400, Brian Masney wrote:
> This patch adds a new vibrator driver that supports various Qualcomm
> MSM SOCs. Driver was tested on a LG Nexus 5 (hammerhead) phone.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>

Any chance that I can get a review of this patch series? Let me know if
you're not the right maintainer that I should send this series to.

Thanks,

Brian

Re: [PATCH v3 2/3] Input: add new vibrator driver for various MSM SOCs

[Dmitry Torokhov]

Hi Brian,

On Wed, Oct 24, 2018 at 09:29:36PM -0400, Brian Masney wrote:
> This patch adds a new vibrator driver that supports various Qualcomm
> MSM SOCs. Driver was tested on a LG Nexus 5 (hammerhead) phone.
> 

...

> +
> +#define msm_vibrator_write(msm_vibrator, offset, value) \
> +	writel((value), (void __iomem *)((msm_vibrator)->base + (offset)))

Make in a function? It will be inlined anyways...

> +
> +	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)) {

You have explicit deferral handling for the regulator, but not gpio. I'd
prefer if we did (or not) it consistently.

> +		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)) {

Same here.

> +		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 (IS_ERR(vibrator->base)) {

devm_ioremap() returns NULL on error. You either need to check for NULL
or see of you can use devm_ioremap_resource().

> +		dev_err(&pdev->dev, "Failed to iomap resource: %ld\n",
> +			PTR_ERR(vibrator->base));
> +		return PTR_ERR(vibrator->base);
> +	}
> +
> +	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->dev.parent = &pdev->dev;

You allocated input device with devm so there is no need to set parent
by hand.

> +	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 msm_vibrator *vibrator = dev_get_drvdata(dev);

Prefer explicit platform_get_drvdata() since we are working with
platform device (even though it resolves to the same thing).

> +
> +	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 msm_vibrator *vibrator = dev_get_drvdata(dev);

Same here.

Thanks.

-- 
Dmitry

Re: [PATCH v3 2/3] Input: add new vibrator driver for various MSM SOCs

[Brian Masney]

On Tue, Feb 05, 2019 at 11:42:46AM -0800, Dmitry Torokhov wrote:
> > +		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 (IS_ERR(vibrator->base)) {
> 
> devm_ioremap() returns NULL on error. You either need to check for NULL
> or see of you can use devm_ioremap_resource().

I originally tried to use devm_ioremap_resource() but the call to
devm_request_mem_region() would fail. I'll go with the NULL check here
for devm_ioremap().

Thanks for the review!

Brian