Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: Mark Rutland <mark.rutland@arm.com>
Date: 2014-07-29 09:37:25
Also in:
linux-devicetree, lkml
On Mon, Jul 28, 2014 at 05:53:23PM +0100, Dan Murphy wrote:
quoted hunk ↗ jump to hunk
Add the TI drv260x haptics/vibrator driver. This device uses the input force feedback to produce a wave form to driver an ERM or LRA actuator device. The initial driver supports the devices real time playback mode. But the device has additional wave patterns in ROM. This functionality will be added in future patchsets. Product data sheet is located here: http://www.ti.com/product/drv2605 Signed-off-by: Dan Murphy <redacted> --- v2 - Fixed binding doc and patch headline - https://patchwork.kernel.org/patch/4619641/ .../devicetree/bindings/input/ti,drv260x.txt | 44 ++ drivers/input/misc/Kconfig | 9 + drivers/input/misc/Makefile | 1 + drivers/input/misc/drv260x.c | 537 ++++++++++++++++++++ include/dt-bindings/input/ti-drv260x.h | 30 ++ include/linux/input/drv260x.h | 181 +++++++ 6 files changed, 802 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/ti,drv260x.txt create mode 100644 drivers/input/misc/drv260x.c create mode 100644 include/dt-bindings/input/ti-drv260x.h create mode 100644 include/linux/input/drv260x.hdiff --git a/Documentation/devicetree/bindings/input/ti,drv260x.txt b/Documentation/devicetree/bindings/input/ti,drv260x.txt new file mode 100644 index 0000000..930429b --- /dev/null +++ b/Documentation/devicetree/bindings/input/ti,drv260x.txt@@ -0,0 +1,44 @@ +Texas Instruments - drv260x Haptics driver family + +The drv260x family serial control bus communicates through I2C protocols + +Required properties: + - compatible - One of: + "ti,drv2604" - DRV2604 + "ti,drv2605" - DRV2605 + "ti,drv2605l" - DRV2605L + - reg - I2C slave address + - mode - Power up mode of the chip (defined in include/dt-bindings/input/ti-drv260x.h) + DRV260X_RTP_MODE - Real time playback mode + DRV260X_LRA_MODE - Linear Resonance Actuator mode (Piezoelectric) + DRV260X_ERM_MODE - Eccentric Rotating Mass mode (Rotary vibrator)
What exactly are these, and why does the kernel not decide?
+ - library_sel - Library to use at power up (defined in include/dt-bindings/input/ti-drv260x.h)
Please s/_/-/ in all property names.
+ DRV260X_LIB_A - Pre-programmed Library + DRV260X_LIB_B - Pre-programmed Library + DRV260X_LIB_C - Pre-programmed Library + DRV260X_LIB_D - Pre-programmed Library + DRV260X_LIB_E - Pre-programmed Library + DRV260X_LIB_F - Pre-programmed Library
What exactly are these, and why does the kernel not decide?
+ +Optional properties: + - enable-gpio - gpio pin to enable/disable the device. + - vib_rated_voltage - The rated voltage of the actuator. If this is not + set then the value will be defaulted to 0x90 in the + driver.
What units is this in? Don't mention the driver, just say "if not 0x90" (but with a better description of what 0x90 actually means).
+ - vib_overdrive_voltage - The overdrive voltage of the actuator. + If this is not set then the value + will be defaulted to 0x90 in the driver.
Similarly on both points. [...]
+static int drv260x_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct drv260x_data *haptics;
+ struct device_node *np = client->dev.of_node;
+ struct drv260x_platform_data *pdata = client->dev.platform_data;
+ struct ff_device *ff;
+ int ret;
+ int voltage;
+
+ haptics = devm_kzalloc(&client->dev, sizeof(*haptics), GFP_KERNEL);
+ if (!haptics)
+ return -ENOMEM;
+
+ haptics->rated_voltage = DRV260X_DEF_OD_CLAMP_VOLT;
+ haptics->rated_voltage = DRV260X_DEF_RATED_VOLT;
+
+ if (np) {
+ ret = of_property_read_u32(np, "mode", &haptics->mode);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "%s: No entry for mode\n", __func__);
+
+ return ret;
+ }No sanity checking on the actual value? As far as I can see, haptics->mode is an int, not a u32.
+ ret = of_property_read_u32(np, "library_sel",
+ &haptics->library);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "%s: No entry for library selection\n", __func__);
+
+ return ret;
+ }Sanity checking? haptics->library is not a u32.
+ ret = of_property_read_u32(np, "vib_rated_voltage", + &voltage); + if (!ret) + haptics->rated_voltage = drv260x_calculate_voltage(voltage); + + + ret = of_property_read_u32(np, "vib_overdrive_voltage", + &voltage); + if (!ret) + haptics->overdrive_voltage = drv260x_calculate_voltage(voltage); +
And again, on both points.
+ } else if (pdata) {
+ haptics->mode = pdata->mode;
+ haptics->library = pdata->library_selection;
+ if (pdata->vib_overdrive_voltage)
+ haptics->overdrive_voltage = drv260x_calculate_voltage(pdata->vib_overdrive_voltage);
+ if (pdata->vib_rated_voltage)
+ haptics->rated_voltage = drv260x_calculate_voltage(pdata->vib_rated_voltage);
+ } else {
+ dev_err(&client->dev, "Platform data not set\n");
+ return -ENODEV;
+ }
+
+ haptics->regulator = regulator_get(&client->dev, "vbat");This wasn't in the binding. Thanks, Mark.