* [PATCH v5 3/3] ARM: dts: msm8974-FP2: Add vibration motor
From: Luca Weiss @ 2019-04-26 19:47 UTC (permalink / raw)
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
Rob Herring, Baolin Wang, David Brown,
open list:ARM/QUALCOMM SUPPORT,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Luca Weiss
In-Reply-To: <20190426194747.22256-1-luca@z3ntu.xyz>
Add a node describing the vibration motor on the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
index 643c57f84818..bf402ae39226 100644
--- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
@@ -50,6 +50,12 @@
};
};
+ vibrator {
+ compatible = "gpio-vibrator";
+ enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+ vcc-supply = <&pm8941_l18>;
+ };
+
smd {
rpm {
rpm_requests {
--
2.21.0
^ permalink raw reply related
* [PATCH v5 2/3] Input: add a driver for GPIO controllable vibrators
From: Luca Weiss @ 2019-04-26 19:47 UTC (permalink / raw)
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
Rob Herring, Baolin Wang, David Brown,
open list:ARM/QUALCOMM SUPPORT,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Luca Weiss
In-Reply-To: <20190426194747.22256-1-luca@z3ntu.xyz>
Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
drivers/input/misc/Kconfig | 12 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gpio-vibra.c | 209 ++++++++++++++++++++++++++++++++
3 files changed, 222 insertions(+)
create mode 100644 drivers/input/misc/gpio-vibra.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index e15ed1bb8558..6dfe9e2fe5b1 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -290,6 +290,18 @@ config INPUT_GPIO_DECODER
To compile this driver as a module, choose M here: the module
will be called gpio_decoder.
+config INPUT_GPIO_VIBRA
+ tristate "GPIO vibrator support"
+ depends on GPIOLIB || COMPILE_TEST
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here to get support for GPIO based vibrator devices.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the module will be
+ called gpio-vibra.
+
config INPUT_IXP4XX_BEEPER
tristate "IXP4XX Beeper support"
depends on ARCH_IXP4XX
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index b936c5b1d4ac..f38ebbdb05e2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS) += drv2667.o
obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
+obj-$(CONFIG_INPUT_GPIO_VIBRA) += gpio-vibra.o
obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
new file mode 100644
index 000000000000..b76c81015de9
--- /dev/null
+++ b/drivers/input/misc/gpio-vibra.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * GPIO vibrator driver
+ *
+ * Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
+ *
+ * Based on PWM vibrator driver:
+ * Copyright (C) 2017 Collabora Ltd.
+ *
+ * Based on previous work from:
+ * Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
+ *
+ * Based on PWM beeper driver:
+ * Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+struct gpio_vibrator {
+ struct input_dev *input;
+ struct gpio_desc *gpio;
+ struct regulator *vcc;
+
+ struct work_struct play_work;
+ bool running;
+ bool vcc_on;
+};
+
+static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
+{
+ struct device *pdev = vibrator->input->dev.parent;
+ int err;
+
+ if (!vibrator->vcc_on) {
+ err = regulator_enable(vibrator->vcc);
+ if (err) {
+ dev_err(pdev, "failed to enable regulator: %d\n", err);
+ return err;
+ }
+ vibrator->vcc_on = true;
+ }
+
+ gpiod_set_value_cansleep(vibrator->gpio, 1);
+
+ return 0;
+}
+
+static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
+{
+ gpiod_set_value_cansleep(vibrator->gpio, 0);
+
+ if (vibrator->vcc_on) {
+ regulator_disable(vibrator->vcc);
+ vibrator->vcc_on = false;
+ }
+}
+
+static void gpio_vibrator_play_work(struct work_struct *work)
+{
+ struct gpio_vibrator *vibrator =
+ container_of(work, struct gpio_vibrator, play_work);
+
+ if (vibrator->running)
+ gpio_vibrator_start(vibrator);
+ else
+ gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct gpio_vibrator *vibrator = input_get_drvdata(dev);
+
+ int level = effect->u.rumble.strong_magnitude;
+
+ if (!level)
+ level = effect->u.rumble.weak_magnitude;
+
+ if (level)
+ vibrator->running = true;
+ else
+ vibrator->running = false;
+
+ schedule_work(&vibrator->play_work);
+
+ return 0;
+}
+
+static void gpio_vibrator_close(struct input_dev *input)
+{
+ struct gpio_vibrator *vibrator = input_get_drvdata(input);
+
+ cancel_work_sync(&vibrator->play_work);
+ gpio_vibrator_stop(vibrator);
+ vibrator->running = false;
+}
+
+static int gpio_vibrator_probe(struct platform_device *pdev)
+{
+ struct gpio_vibrator *vibrator;
+ int err;
+
+ 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");
+ err = PTR_ERR_OR_ZERO(vibrator->vcc);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to request regulator: %d\n",
+ err);
+ return err;
+ }
+
+ vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
+ err = PTR_ERR_OR_ZERO(vibrator->gpio);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to request main gpio: %d\n",
+ err);
+ return err;
+ }
+
+ INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
+
+ vibrator->input->name = "gpio-vibrator";
+ vibrator->input->id.bustype = BUS_HOST;
+ vibrator->input->close = gpio_vibrator_close;
+
+ input_set_drvdata(vibrator->input, vibrator);
+ input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
+
+ err = input_ff_create_memless(vibrator->input, NULL,
+ gpio_vibrator_play_effect);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't create FF dev: %d\n", err);
+ return err;
+ }
+
+ err = input_register_device(vibrator->input);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't register input dev: %d\n", err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, vibrator);
+
+ return 0;
+}
+
+static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
+{
+ struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+ cancel_work_sync(&vibrator->play_work);
+ if (vibrator->running)
+ gpio_vibrator_stop(vibrator);
+
+ return 0;
+}
+
+static int __maybe_unused gpio_vibrator_resume(struct device *dev)
+{
+ struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+ if (vibrator->running)
+ gpio_vibrator_start(vibrator);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
+ gpio_vibrator_suspend, gpio_vibrator_resume);
+
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_vibra_dt_match_table[] = {
+ { .compatible = "gpio-vibrator" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
+#endif
+
+static struct platform_driver gpio_vibrator_driver = {
+ .probe = gpio_vibrator_probe,
+ .driver = {
+ .name = "gpio-vibrator",
+ .pm = &gpio_vibrator_pm_ops,
+ .of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
+ },
+};
+module_platform_driver(gpio_vibrator_driver);
+
+MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
+MODULE_DESCRIPTION("GPIO vibrator driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio-vibrator");
--
2.21.0
^ permalink raw reply related
* [PATCH v5 1/3] dt-bindings: input: add GPIO controllable vibrator
From: Luca Weiss @ 2019-04-26 19:47 UTC (permalink / raw)
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
Rob Herring, Baolin Wang, David Brown,
open list:ARM/QUALCOMM SUPPORT,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Luca Weiss
Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes for v5:
- Simply compatible property definition
- Remove reference to regulator schema
.../bindings/input/gpio-vibrator.yaml | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-vibrator.yaml
diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.yaml b/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
new file mode 100644
index 000000000000..903475f52dbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-vibrator.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/input/gpio-vibrator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO vibrator
+
+maintainers:
+ - Luca Weiss <luca@z3ntu.xyz>
+
+description: |+
+ Registers a GPIO device as vibrator, where the on/off capability is controlled by a GPIO.
+
+properties:
+ compatible:
+ const: gpio-vibrator
+
+ enable-gpios:
+ maxItems: 1
+
+ vcc-supply:
+ description: Regulator that provides power
+
+required:
+ - compatible
+ - enable-gpios
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ vibrator {
+ compatible = "gpio-vibrator";
+ enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+ vcc-supply = <&pm8941_l18>;
+ };
--
2.21.0
^ permalink raw reply related
* [PATCH v2 3/3] input: keyboard: gpio-keys-polled: skip oftree code when CONFIG_OF disabled
From: Enrico Weigelt, metux IT consult @ 2019-04-26 19:01 UTC (permalink / raw)
To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1556305282-24148-1-git-send-email-info@metux.net>
we don't need to build in oftree probing stuff when oftree isn't
enabled at all.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/input/keyboard/gpio_keys_polled.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 3f773b2..65860bf 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -147,6 +147,7 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
static struct gpio_keys_platform_data *
gpio_keys_polled_get_devtree_pdata(struct device *dev)
{
+#ifdef CONFIG_OF
struct gpio_keys_platform_data *pdata;
struct gpio_keys_button *button;
struct fwnode_handle *child;
@@ -200,6 +201,9 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
}
return pdata;
+#else /* CONFIG_OF */
+ return ERR_PTR(-ENOENT);
+#endif /* CONFIG_OF */
}
static void gpio_keys_polled_set_abs_params(struct input_dev *input,
@@ -222,11 +226,13 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
input_set_abs_params(input, code, min, max, 0, 0);
}
+#ifdef CONFIG_OF
static const struct of_device_id gpio_keys_polled_of_match[] = {
{ .compatible = "gpio-keys-polled", },
{ },
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
+#endif /* CONFIG_OF */
static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
struct device *dev,
@@ -452,7 +458,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
.probe = gpio_keys_polled_probe,
.driver = {
.name = DRV_NAME,
+#ifdef CONFIG_OF
.of_match_table = gpio_keys_polled_of_match,
+#endif /* CONFIG_OF */
},
};
module_platform_driver(gpio_keys_polled_driver);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/3] input: keyboard: gpio_keys_polled: use gpio lookup table
From: Enrico Weigelt, metux IT consult @ 2019-04-26 19:01 UTC (permalink / raw)
To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1556305282-24148-1-git-send-email-info@metux.net>
Support the recently introduced gpio lookup tables for
attaching to gpio lines. So, harcoded gpio numbers aren't
needed anymore.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/input/keyboard/gpio_keys_polled.c | 167 +++++++++++++++++++++---------
1 file changed, 119 insertions(+), 48 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 3312186..3f773b2 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -24,6 +24,7 @@
#include <linux/platform_device.h>
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
#include <linux/gpio_keys.h>
#include <linux/property.h>
@@ -227,6 +228,119 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
+static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
+ struct device *dev,
+ int idx,
+ const char *desc)
+{
+ struct gpio_desc *gpiod;
+ struct fwnode_handle *child;
+ int x;
+
+ /* get the idx'th child node */
+ child = device_get_next_child_node(dev, NULL);
+ while (child && x) {
+ child = device_get_next_child_node(dev, child);
+ x--;
+ }
+
+ if (!child) {
+ dev_err(dev, "missing oftree child node #%d\n", idx);
+ return ERR_PTR(-EINVAL);
+ }
+
+ gpiod = devm_fwnode_get_gpiod_from_child(dev,
+ NULL,
+ child,
+ GPIOD_IN,
+ desc);
+ if (IS_ERR(gpiod)) {
+ if (PTR_ERR(gpiod) != -EPROBE_DEFER)
+ dev_err(dev,
+ "failed to get gpio: %ld\n",
+ PTR_ERR(gpiod));
+ fwnode_handle_put(child);
+ return gpiod;
+ }
+
+ return gpiod;
+}
+
+static struct gpio_desc *gpio_keys_polled_get_gpiod_legacy(
+ struct device *dev,
+ int idx,
+ const struct gpio_keys_button *button)
+{
+ /*
+ * Legacy GPIO number so request the GPIO here and
+ * convert it to descriptor.
+ */
+ unsigned int flags = GPIOF_IN;
+ struct gpio_desc *gpiod;
+ int error;
+
+ dev_info(dev, "hardcoded gpio IDs are deprecated.\n");
+
+ if (button->active_low)
+ flags |= GPIOF_ACTIVE_LOW;
+
+ error = devm_gpio_request_one(dev, button->gpio,
+ flags, button->desc ? : DRV_NAME);
+ if (error) {
+ dev_err(dev,
+ "unable to claim gpio %u, err=%d\n",
+ button->gpio, error);
+ return ERR_PTR(error);
+ }
+
+ gpiod = gpio_to_desc(button->gpio);
+ if (!gpiod) {
+ dev_err(dev,
+ "unable to convert gpio %u to descriptor\n",
+ button->gpio);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return gpiod;
+}
+
+static struct gpio_desc *gpio_keys_polled_get_gpiod(
+ struct device *dev,
+ int idx,
+ const struct gpio_keys_button *button)
+{
+ struct gpio_desc *gpiod = NULL;
+ int error;
+
+ /* No legacy static platform data - use oftree */
+ if (!dev_get_platdata(dev)) {
+ return gpio_keys_polled_get_gpiod_fwnode(
+ dev, idx, button->desc);
+ }
+
+ gpiod = devm_gpiod_get_index(dev, NULL, idx, GPIOF_IN);
+
+ if (!IS_ERR(gpiod)) {
+ dev_info(dev, "picked gpiod idx %d from gpio table\n", idx);
+ gpiod_set_consumer_name(gpiod, button->desc ? : DRV_NAME);
+ return gpiod;
+ }
+
+ if (PTR_ERR(gpiod) != -ENOENT) {
+ dev_err(dev, "failed fetching gpiod #%d: %d\n",
+ idx, PTR_ERR(gpiod));
+ return gpiod;
+ }
+
+ /* Use legacy gpio id, if defined */
+ if (gpio_is_valid(button->gpio)) {
+ return gpio_keys_polled_get_gpiod_legacy(
+ dev, idx, button);
+ }
+
+ return ERR_PTR(-ENOENT);
+}
+
static int gpio_keys_polled_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -291,57 +405,14 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
if (button->wakeup) {
dev_err(dev, DRV_NAME " does not support wakeup\n");
- fwnode_handle_put(child);
return -EINVAL;
}
- if (!dev_get_platdata(dev)) {
- /* No legacy static platform data */
- child = device_get_next_child_node(dev, child);
- if (!child) {
- dev_err(dev, "missing child device node\n");
- return -EINVAL;
- }
-
- bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev,
- NULL, child,
- GPIOD_IN,
- button->desc);
- if (IS_ERR(bdata->gpiod)) {
- error = PTR_ERR(bdata->gpiod);
- if (error != -EPROBE_DEFER)
- dev_err(dev,
- "failed to get gpio: %d\n",
- error);
- fwnode_handle_put(child);
- return error;
- }
- } else if (gpio_is_valid(button->gpio)) {
- /*
- * Legacy GPIO number so request the GPIO here and
- * convert it to descriptor.
- */
- unsigned flags = GPIOF_IN;
-
- if (button->active_low)
- flags |= GPIOF_ACTIVE_LOW;
-
- error = devm_gpio_request_one(dev, button->gpio,
- flags, button->desc ? : DRV_NAME);
- if (error) {
- dev_err(dev,
- "unable to claim gpio %u, err=%d\n",
- button->gpio, error);
- return error;
- }
-
- bdata->gpiod = gpio_to_desc(button->gpio);
- if (!bdata->gpiod) {
- dev_err(dev,
- "unable to convert gpio %u to descriptor\n",
- button->gpio);
- return -EINVAL;
- }
+ bdata->gpiod = gpio_keys_polled_get_gpiod(dev, i, button);
+
+ if (IS_ERR(bdata->gpiod)) {
+ dev_err(dev, "failed to fetch gpiod #%d\n", i);
+ return PTR_ERR(bdata->gpiod);
}
bdata->last_state = -1;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/3] input: keyboard: gpio-keys-polled: use input name from pdata if available
From: Enrico Weigelt, metux IT consult @ 2019-04-26 19:01 UTC (permalink / raw)
To: linux-kernel; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <1556305282-24148-1-git-send-email-info@metux.net>
Instead of hardcoding the input name to the driver name
('gpio-keys-polled'), allow the passing a name via platform data
('name' field was already present), but default to old behaviour
in case of NULL.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/input/keyboard/gpio_keys_polled.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index edc7262..3312186 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -272,7 +272,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
input = poll_dev->input;
- input->name = pdev->name;
+ input->name = (pdata->name ? pdata->name : pdev->name);
input->phys = DRV_NAME"/input0";
input->id.bustype = BUS_HOST;
--
1.9.1
^ permalink raw reply related
* gpio-keys-polled improvements v2
From: Enrico Weigelt, metux IT consult @ 2019-04-26 19:01 UTC (permalink / raw)
To: linux-kernel; +Cc: dmitry.torokhov, linux-input
Hello folks,
here's v2 of my gpio-keys-polled queue.
changes v2:
* removed the change in mod_devicetable.h
(MODULE_DEVICE_TABLE_OF macro)
* reworked the patch for building w/o oftree, so it works
w/o the MODULE_DEVICE_TABLE_OF macro
have fun,
--mtx
^ permalink raw reply
* [PATCH 2/2] HID: wacom: Don't report anything prior to the tool entering range
From: Gerecke, Jason @ 2019-04-26 16:35 UTC (permalink / raw)
To: # v3.17+, Sasha Levin
Cc: linux-input, Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke,
Aaron Armstrong Skomra
In-Reply-To: <CANRwn3Ru+7FGtsY=GaDa7pAJkuagdb6nFtvrFq1qhTWJR0rF9A@mail.gmail.com>
From: Jason Gerecke <jason.gerecke@wacom.com>
If the tool spends some time in prox before entering range, a series of
events (e.g. ABS_DISTANCE, MSC_SERIAL) can be sent before we or userspace
have any clue about the pen whose data is being reported. We need to hold
off on reporting anything until the pen has entered range. Since we still
want to report events that occur "in prox" after the pen has *left* range
we use 'wacom-tool[0]' as the indicator that the pen did at one point
enter range and provide us/userspace with tool type and serial number
information.
Fixes: a48324de6d ("HID: wacom: Bluetooth IRQ for Intuos Pro should handle prox/range")
Cc: <stable@vger.kernel.org> # 4.11+
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
---
Version of patch specifically targeted to stable v4.14.113
drivers/hid/wacom_wac.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 03b04bc742dd..e4aeffa56018 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1271,17 +1271,20 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
input_report_abs(pen_input, ABS_Z, rotation);
input_report_abs(pen_input, ABS_WHEEL, get_unaligned_le16(&frame[11]));
}
- input_report_abs(pen_input, ABS_PRESSURE, get_unaligned_le16(&frame[5]));
- input_report_abs(pen_input, ABS_DISTANCE, range ? frame[13] : wacom->features.distance_max);
- input_report_key(pen_input, BTN_TOUCH, frame[0] & 0x01);
- input_report_key(pen_input, BTN_STYLUS, frame[0] & 0x02);
- input_report_key(pen_input, BTN_STYLUS2, frame[0] & 0x04);
+ if (wacom->tool[0]) {
+ input_report_abs(pen_input, ABS_PRESSURE, get_unaligned_le16(&frame[5]));
+ input_report_abs(pen_input, ABS_DISTANCE, range ? frame[13] : wacom->features.distance_max);
- input_report_key(pen_input, wacom->tool[0], prox);
- input_event(pen_input, EV_MSC, MSC_SERIAL, wacom->serial[0]);
- input_report_abs(pen_input, ABS_MISC,
- wacom_intuos_id_mangle(wacom->id[0])); /* report tool id */
+ input_report_key(pen_input, BTN_TOUCH, frame[0] & 0x01);
+ input_report_key(pen_input, BTN_STYLUS, frame[0] & 0x02);
+ input_report_key(pen_input, BTN_STYLUS2, frame[0] & 0x04);
+
+ input_report_key(pen_input, wacom->tool[0], prox);
+ input_event(pen_input, EV_MSC, MSC_SERIAL, wacom->serial[0]);
+ input_report_abs(pen_input, ABS_MISC,
+ wacom_intuos_id_mangle(wacom->id[0])); /* report tool id */
+ }
wacom->shared->stylus_in_proximity = prox;
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] HID: wacom: Don't set tool type until we're in range
From: Gerecke, Jason @ 2019-04-26 16:34 UTC (permalink / raw)
To: # v3.17+, Sasha Levin
Cc: linux-input, Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke,
Aaron Armstrong Skomra
In-Reply-To: <20190426033328.GB17719@sasha-vm>
From: Jason Gerecke <jason.gerecke@wacom.com>
The serial number and tool type information that is reported by the tablet
while a pen is merely "in prox" instead of fully "in range" can be stale
and cause us to report incorrect tool information. Serial number, tool
type, and other information is only valid once the pen comes fully in range
so we should be careful to not use this information until that point.
In particular, this issue may cause the driver to incorectly report
BTN_TOOL_RUBBER after switching from the eraser tool back to the pen.
Fixes: a48324de6d ("HID: wacom: Bluetooth IRQ for Intuos Pro should handle prox/range")
Cc: <stable@vger.kernel.org> # 4.11+
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
---
Version of patch specifically targeted to stable v4.14.113
drivers/hid/wacom_wac.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 4c72e68637c2..03b04bc742dd 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1225,13 +1225,13 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
/* Add back in missing bits of ID for non-USI pens */
wacom->id[0] |= (wacom->serial[0] >> 32) & 0xFFFFF;
}
- wacom->tool[0] = wacom_intuos_get_tool_type(wacom_intuos_id_mangle(wacom->id[0]));
for (i = 0; i < pen_frames; i++) {
unsigned char *frame = &data[i*pen_frame_len + 1];
bool valid = frame[0] & 0x80;
bool prox = frame[0] & 0x40;
bool range = frame[0] & 0x20;
+ bool invert = frame[0] & 0x10;
if (!valid)
continue;
@@ -1240,9 +1240,24 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
wacom->shared->stylus_in_proximity = false;
wacom_exit_report(wacom);
input_sync(pen_input);
+
+ wacom->tool[0] = 0;
+ wacom->id[0] = 0;
+ wacom->serial[0] = 0;
return;
}
+
if (range) {
+ if (!wacom->tool[0]) { /* first in range */
+ /* Going into range select tool */
+ if (invert)
+ wacom->tool[0] = BTN_TOOL_RUBBER;
+ else if (wacom->id[0])
+ wacom->tool[0] = wacom_intuos_get_tool_type(wacom->id[0]);
+ else
+ wacom->tool[0] = BTN_TOOL_PEN;
+ }
+
/* Fix rotation alignment: userspace expects zero at left */
int16_t rotation = (int16_t)get_unaligned_le16(&frame[9]);
rotation += 1800/4;
--
2.21.0
^ permalink raw reply related
* [PATCH][next] HID: logitech-dj: fix spelling mistake "Unexpect" -> "Unexpected"
From: Colin King @ 2019-04-26 13:16 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a hid_err error message, fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/hid/hid-logitech-dj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 64e68ac871cb..b1e894618eed 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1064,7 +1064,7 @@ static void logi_dj_recv_forward_input_report(struct hid_device *hdev,
int i;
if (report > REPORT_TYPE_RFREPORT_LAST) {
- hid_err(hdev, "Unexpect input report number %d\n", report);
+ hid_err(hdev, "Unexpected input report number %d\n", report);
return;
}
--
2.20.1
^ permalink raw reply related
* Re: [Bug 203297] Synaptics touchpad TM-3127 functionality broken by PCI runtime power management patch on 4.20.2
From: Jarkko Nikula @ 2019-04-26 12:12 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Keijo Vaara, Jean Delvare, Rafael J. Wysocki, Benjamin Tissoires,
Dmitry Torokhov, linux-pm, linux-pci, linux-kernel, linux-input
In-Reply-To: <20190422130814.GJ173520@google.com>
On 4/22/19 4:08 PM, Bjorn Helgaas wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=203297
>
> Regression, suspected but as yet unconfirmed cause:
>
> c5eb1190074c ("PCI / PM: Allow runtime PM without callback functions")
>
> backported to 4.20 stable as 39e1be324c2f.
>
With help of Keijo it was confirmed above patch broke the Synaptics
touchpad. Not bisected but touchpad works again by forcing the i2c-i801
SMBus controller always on:
"echo on >/sys/bus/pci/devices/0000\:00\:1f.3/power/control"
Above patch is a generalized fix that fixed the runtime PM regression on
i2c-i801 and re-allow the controller go to runtime suspend when idle. So
most probably Synaptics touchpad was broken by i2c-i801 runtime PM also
before but got unnoticed. Which is easy since on many platforms SMBus
controller doesn't necessarily have the PCI PM capabilities.
I would like to ask help from input subsystem experts what kind of SMBus
power state dependency Synaptics RMI4 SMBus devices have since it cease
to work if SMBus controllers idles between transfers and how this is
best to fix?
Instead of revert I think we'd need to have some method to force SMBus
controller on whenever the touchpad is active, like when there is a
userspace listening.
I'm not expert in this area so as quick proof of concept I had a
following hack which forces the I2C/SMBus adapter, and eventually the
parent PCI device of it on when the RMI4 SMBus device is probed and let
the SMBus controller to idle when removed.
According to Keijo it fixes the issue but I like to hear input experts
for better place to put these.
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index b6ccf39c6a7b..2b11d69be313 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -16,6 +16,7 @@
#include <linux/lockdep.h>
#include <linux/module.h>
#include <linux/pm.h>
+#include <linux/pm_runtime.h>
#include <linux/rmi.h>
#include <linux/slab.h>
#include "rmi_driver.h"
@@ -332,6 +333,9 @@ static int rmi_smb_probe(struct i2c_client *client,
dev_info(&client->dev, "registering SMbus-connected sensor\n");
+ /* Force SMBus adapter on while RMI4 device is connected */
+ pm_runtime_get(&client->adapter->dev);
+
error = rmi_register_transport_device(&rmi_smb->xport);
if (error) {
dev_err(&client->dev, "failed to register sensor: %d\n", error);
@@ -346,6 +350,7 @@ static int rmi_smb_remove(struct i2c_client *client)
struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
rmi_unregister_transport_device(&rmi_smb->xport);
+ pm_runtime_put(&client->adapter->dev);
return 0;
}
--
Jarkko
^ permalink raw reply related
* [RFC] tty: kb_value with flags for better Unicode support
From: Reinis Danne @ 2019-04-26 10:52 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-input, kbd
Compliment already existing kbdiacruc and kbdiacrsuc structs and
KD[GS]KBDIACRUC ioctls with Unicode equivalents for kb_value, kbentry and
KD[GS]KBENT ioctls.
```
struct kb_valueuc {
__u32 flags; /* 15 used by KTYP */
__u32 kb_valueuc; /* Unicode range: 0x0–0x10ffff */
};
struct kbentryuc {
__u32 kb_table;
__u32 kb_index;
struct kb_valueuc;
};
extern kb_valueuc *key_maps[MAX_NR_KEYMAPS];
#define KDGKBENTUC 0x???? /* get one entry in translation table */
#define KDSKBENTUC 0x???? /* set one entry in translation table */
```
Motivation
==========
Since I learned touchtyping, I want to have the same keyboard layout in VT as I
have in X. So I wrote a keymap file for Latvian (modern) keyboard layout [1]
to use with the kbd package and it works, mostly.
I have three issues:
- Compose sequences with base above Latin-1 not working (fixed).
- CapsLock not working as expected for characters above Latin-1.
- Can't use Meta key with characters above Latin-1.
There are three letters above 0xff on level 1 of this keyboard layout:
ē — U+0113 Dec:275 LATIN SMALL LETTER E WITH MACRON
ā — U+0101 Dec:257 LATIN SMALL LETTER A WITH MACRON
ī — U+012B Dec:299 LATIN SMALL LETTER I WITH MACRON
Compose
=======
I have added some extra letters in the free places to be able to type not only
Latvian and English, but also German and Finnish (e.g., there is letter ö on
level 3 of ē key) for the rare occasions I need them.
This keyboard layout uses a dead key (dead_acute) to access level 3 symbols
(the same as AltGr):
compose diacr base to result
compose '\'' U+0113 to U+00F6
But it didn't work if the base in the compose sequence was above 0xff (patch
[2] is in tty-next).
Key value and flags
===================
The other two issues could be attributed to the lack of proper flags for key
values (key type is encoded in its value).
According to keymaps manual:
```
Each keysym may be prefixed by a '+' (plus sign), in wich case this keysym
is treated as a "letter" and therefore affected by the "CapsLock" the same way
as by "Shift" (to be correct, the CapsLock inverts the Shift state). The ASCII
letters ('a'-'z' and 'A'-'Z') are made CapsLock'able by default. If
Shift+CapsLock should not produce a lower case symbol, put lines like
keycode 30 = +a A
in the map file.
```
But it doesn't work — CapsLock is ignored for codepoints above 0xff. Adding
plus signs to all four maps should make them behave the same way (like in X):
# 0 1 2 3
# Plain Shift AltGr AltGr+Shift
keycode 16 = +U+0113 +U+0112 +U+00F6 +U+00D6
| X VT
--------------------------+---------------
CapsLock ē | Ē ē
CapsLock+Shift ē | ē Ē
CapsLock+AltGr ē | Ö Ö
CapsLock+Shift+AltGr ē | ö ö
For the key to behave properly, its key type (KTYP) has to be 'letter':
include/uapi/linux/keyboard.h:
#define KT_LETTER 11 /* symbol that can be acted upon by CapsLock */
Thus it is necessary to set KTYP for characters beyond Latin-1; which is not
possible now.
Currently they are defined like this:
```
include/linux/keyboard.h:
extern unsigned short *key_maps[MAX_NR_KEYMAPS];
drivers/tty/vt/defkeymap.c_shipped:
ushort *key_maps[MAX_NR_KEYMAPS] = {
plain_map, shift_map, altgr_map, NULL,
ctrl_map, shift_ctrl_map, NULL, NULL,
alt_map, NULL, NULL, NULL,
ctrl_alt_map, NULL
};
include/uapi/linux/kd.h:
struct kbentry {
unsigned char kb_table;
unsigned char kb_index;
unsigned short kb_value; <-- Important!
};
#define KDGKBENT 0x4B46 /* gets one entry in translation table */
#define KDSKBENT 0x4B47 /* sets one entry in translation table */
include/linux/kbd_kern.h:
#define U(x) ((x) ^ 0xf000)
#define BRL_UC_ROW 0x2800
include/uapi/linux/keyboard.h:
#define K(t,v) (((t)<<8)|(v))
#define KTYP(x) ((x) >> 8)
#define KVAL(x) ((x) & 0xff)
```
The use of ``unsigned short kb_value`` in ``struct kbentry`` prevents setting
KTYP for Unicode characters beyond Latin-1 since there are only two bytes in an
``unsigned short`` and KTYP needs one, not leaving enough space for code points
beyond 0xff.
This breaks CapsLock for keyboard layouts with characters above Latin-1 [3–6].
I think those bugs are closed by mistake, since, to this day, it doesn't work.
And it can't work because of the aforementioned kernel limitations (at least as
far as CapsLock issue in Unicode mode is concerned).
To illustrate, keysym is 16 bits long:
mmmm tttt nnnn nnnn
m — mask for (non-)Unicode characters (U macro)
t — KTYP
n — KVAL
This also limits the number of Unicode characters — from 0xf000 the mask is
lost. (No Klingon input in VT [not that I want one]. I think
Documentation/admin-guide/unicode.rst talks only about the output. Or am I
missing something?)
See vt_do_kdsk_ioctl() and kbd_keycode() in drivers/tty/vt/keyboard.c for how
the mask and U macro is used.
As a side note: It seems CapsShift has never worked either. It was suggested
as a workaround to this issue in one of the kernel bugs, but it obviously
wouldn't work. First, CapsShift needs key map 256 and up (limited by
MAX_NR_KEYMAPS). Second, in struct kbentry the kb_table index is unsigned char
(0–255). So, even if one increased MAX_NR_KEYMAPS and recompiled the kernel,
they still wouldn't be able to set the key map, because the ioctl can't index
the table.
Solution
========
A possible fix could be a proper, extensible struct with flags [7] for
kb_value, used in the key_map[] and a pair of new ioctls (see the top of the
mail).
I think the increase in memory usage here is not something to worry about.
That would change key_map[] from ushort to __u64. So instead of 2 bytes per
keysym, it would use 8 bytes. The memory usage of keymaps would increase 4
times. Since there are 7 keymaps by default with 256 keys each, that would
increase memory usage by:
(8-2)*7*256=42*256=10752 B
Each additional keymap would increase memory usage by:
8*256=2048 B
Increasing the size of kb_table and kb_index might be useful in the future for
adding multiple keyboard layout support to VT [8].
---
The increase of memory usage could be cut in half if ``__u32 flags`` is dropped
and KTYP is put at the last byte of ``__u32 kb_valueuc``:
#define K(t,v) (((t)<<24)|(v))
#define KTYP(x) ((x) >> 24)
#define KVAL(x) ((x) & 0xffffff)
But in this case the future-proofing for flags [7,9] would be lost.
Also, there is possible conflict for programs built with old version of K
macros running on newer kernels. The macros would have to be renamed.
---
Affected users
==============
KTYP or KVAL are used in (they would all have to be updated):
- kernel/debug/kdb/kdb_keyboard.c
- drivers/s390/char/keyboard.c
- drivers/s390/char/tty3270.c
- drivers/staging/speakup/main.c
- drivers/tty/vt/keyboard.c
- drivers/accessibility/braille/braille_console.c
- arch/m68k/atari/atakeyb.c
In addition to those, ``key_maps`` are used in:
- drivers/s390/char/defkeymap.c
- drivers/tty/vt/defkeymap.c_shipped
- drivers/input/keyboard/amikbd.c
- include/linux/keyboard.h
- arch/m68k/amiga/config.c
Also kbd package would have to be updated to take advantage of the change.
Is anybody already working on this? Maybe somebody has done it a long time ago
already, and I just have to do some magic incantations to make it work?
Is it even worth doing?
I'm new to kernel programming, comments from people with better insights are
very much appreciated.
-Reinis
[1] https://odo.lv/xwiki/bin/download/Recipes/LatvianKeyboard/Modern.png
[2] https://lkml.org/lkml/2019/4/11/362
[3] https://bugzilla.kernel.org/show_bug.cgi?id=7063
[4] https://bugzilla.kernel.org/show_bug.cgi?id=7746
[5] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=404503
[6] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/16638
[7] https://blog.ffwll.ch/2013/11/botching-up-ioctls.html
[8] https://www.happyassassin.net/2013/11/23/keyboard-layouts-in-fedora-20-and-previously/
[9] https://lwn.net/Articles/585415/
^ permalink raw reply
* Re: [PATCH 0/5] Add of_ functions for device_link_add()
From: Benjamin GAIGNARD @ 2019-04-26 8:36 UTC (permalink / raw)
To: Rob Herring, Dmitry Torokhov
Cc: Wysocki, Rafael J, Mark Rutland, Bastien Nocera, Frank Rowand,
Marco Felsch, Guido Günther, Yannick FERTRE, Arnd Bergmann,
Linux Input, DTML, linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com, Mark Brown
In-Reply-To: <CAL_JsqJAnaJo4uLgNaqfnNMGYO+qqLaqdEn159hMTYqYE-Afhg@mail.gmail.com>
On 4/26/19 1:02 AM, Rob Herring wrote:
> On Thu, Apr 25, 2019 at 2:24 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <robh+dt@kernel.org> wrote:
>>> On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard
>>> <benjamin.gaignard@st.com> wrote:
>>>> It could happen that we need to control suspend/resume ordering between
>>>> devices without obvious consumer/supplier link. For example when touchscreens
>>>> and DSI panels share the same reset line, in this case we need to be sure
>>>> of pm_runtime operations ordering between those two devices to correctly
>>>> perform reset.
>>>> DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
>>>> I2C client and I2C bus or regulator client and regulator provider) so we need
>>>> to describe this in device-tree.
>>> Needing to know which touchscreen is attached to a panel could be
>>> important to describe if you have multiple displays.
>>>
>>> Doesn't the reset subsystem already have some support for shared
>>> resets? Seems like it could provide clients with struct device or
>>> device_node ptrs to other devices sharing a reset.
Sharing reset will not help here because we don't want to have two reset
occuring in the same line but only one when the both devices are resumed.
That why we to force suspend/resume ordering and so add a link between the
devices.
>>>
>>>> This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
>>>> helpers to find and parse 'links-add' property in a device-tree node.
>>> Going to document that property somewhere? :)
I have put a description of this property in each device bindings but if
you can tell me
where to put it, I will follow your advice.
>>>
>>> I think this is too generic and coupled to Linux. It doesn't have any
>>> information as to what is the dependency or connection nor what the
>>> direction of the dependency is.
Could something like 'link-suppliers' or 'pm-runtime-dependencies'
are more explicit property name to describe direction, goal, and connection
between consumer and supplier ?
>>>
>>> I'm not convinced we need to solve this generically vs. defining
>>> something for this specific example.
>> I am pretty sure there will be more drivers needing complex
>> dependencies. Doesn't ACPI allow defining relationship between devices
>> that goes beyond the tree structure?
> Can't speak to ACPI, but I assume you where implying ACPI supports
> this so DT should too.
>
> Almost every binding we have is defining relationships between
> devices. Interrupts, clocks, gpio, pinctrl, etc. all do this. To use a
> similar example to the one here, we already define the relationship
> between a display and a backlight with a 'backlight' property in the
> display node. Why should touchscreen be any different than backlight?
It is different because it is only about suspend/resume ordering.
There is no need for a panel to knows about touchscreen unlike
than backlight that it could drive.
I have the same need to order suspend/resume operations between
GPU and display to be sure that GPU is suspend before display and resume
after it.
>
> What really concerns me here is folks just add relationships to
> 'links-add' which are already captured in DT (such as backlight) just
> because they'll get it for free and not have to go add support to
> handle each specific binding.
It won't be for free since I don't want to put it in device core so each
driver
will have to add a call to a function to enable it and will have to
explain why doing it.
Benjamin
>
> Rob
^ permalink raw reply
* [RFC PATCH 4/4] ARM: dts: imx6dl-yapp4: Enable MPR121 touch keypad on Hydra
From: Michal Vokáč @ 2019-04-26 8:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring
Cc: Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam, linux-input,
devicetree, linux-kernel, Pengutronix Kernel Team,
Michal Vokáč
In-Reply-To: <1556267420-93219-1-git-send-email-michal.vokac@ysoft.com>
Enable the I2C connected touch keypad on Hydra board.
Use the polled binding as the interrupt line is not available.
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi | 12 ++++++++++++
arch/arm/boot/dts/imx6dl-yapp4-hydra.dts | 4 ++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi b/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
index e8d800fec637..65a670e5bd4f 100644
--- a/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
+++ b/arch/arm/boot/dts/imx6dl-yapp4-common.dtsi
@@ -4,6 +4,7 @@
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/input/input.h>
#include <dt-bindings/pwm/pwm.h>
/ {
@@ -330,6 +331,17 @@
vcc-supply = <&sw2_reg>;
status = "disabled";
};
+
+ touchkeys: keys@5a {
+ compatible = "fsl,mpr121-touchkey-polled";
+ reg = <0x5a>;
+ vdd-supply = <&sw2_reg>;
+ autorepeat;
+ linux,keycodes = <KEY_1>, <KEY_2>, <KEY_3>, <KEY_4>, <KEY_5>,
+ <KEY_6>, <KEY_7>, <KEY_8>, <KEY_9>,
+ <KEY_BACKSPACE>, <KEY_0>, <KEY_ENTER>;
+ status = "disabled";
+ };
};
&iomuxc {
diff --git a/arch/arm/boot/dts/imx6dl-yapp4-hydra.dts b/arch/arm/boot/dts/imx6dl-yapp4-hydra.dts
index f97927064750..84c275bfdd38 100644
--- a/arch/arm/boot/dts/imx6dl-yapp4-hydra.dts
+++ b/arch/arm/boot/dts/imx6dl-yapp4-hydra.dts
@@ -45,6 +45,10 @@
status = "okay";
};
+&touchkeys {
+ status = "okay";
+};
+
&usdhc3 {
status = "okay";
};
--
2.1.4
^ permalink raw reply related
* [RFC PATCH 3/4] Input: mpr121-polled: Add write-through cache to detect corrupted registers
From: Michal Vokáč @ 2019-04-26 8:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring
Cc: Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam, linux-input,
devicetree, linux-kernel, Pengutronix Kernel Team,
Michal Vokáč
In-Reply-To: <1556267420-93219-1-git-send-email-michal.vokac@ysoft.com>
The MPR121 chip (and I2C bus in general) is quite sensitive to ESD.
An electrostatic discharge can easily cause a reset of the MPR121 chip.
Even though the chip then recovers and respond to read/write commands,
it is not properly initialized.
This state can be detected using a write-through cache of the internal
registers. Each time a register is written to, its value is stored in
the cache and marked as valid. Once per MPR121_REG_CACHE_CHECK_LIMIT
polls one valid cache value is compared with its corresponding register
value. In case of difference an error counter is increased. If the error
counter limit is exceeded, the chip is re-initialized.
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
drivers/input/keyboard/mpr121_touchkey_polled.c | 100 +++++++++++++++++++++---
1 file changed, 88 insertions(+), 12 deletions(-)
diff --git a/drivers/input/keyboard/mpr121_touchkey_polled.c b/drivers/input/keyboard/mpr121_touchkey_polled.c
index e5e80530c9d8..6536d9b2eeb8 100644
--- a/drivers/input/keyboard/mpr121_touchkey_polled.c
+++ b/drivers/input/keyboard/mpr121_touchkey_polled.c
@@ -67,6 +67,19 @@
#define MPR121_POLL_INTERVAL_REINIT 500
#define MPR121_POLL_RETRY_MAX 4
+#define MPR121_REG_CACHE_MIN_ADDR 0x2b
+#define MPR121_REG_CACHE_MAX_ADDR 0x7f
+#define MPR121_REG_CACHE_SIZE \
+ (MPR121_REG_CACHE_MAX_ADDR - MPR121_REG_CACHE_MIN_ADDR + 1)
+#define MPR121_REG_CACHE_CHECK_LIMIT 8
+#define mpr121_addr_to_cache_idx(addr) (addr - MPR121_REG_CACHE_MIN_ADDR)
+#define mpr121_cache_idx_to_addr(idx) (idx + MPR121_REG_CACHE_MIN_ADDR)
+
+struct mpr121_polled_reg_cache {
+ bool valid;
+ u8 value;
+};
+
struct mpr121_polled {
struct i2c_client *client;
struct input_dev *input_dev;
@@ -76,6 +89,9 @@ struct mpr121_polled {
u32 keycodes[MPR121_MAX_KEY_COUNT];
u8 read_errors;
int vdd_uv;
+ struct mpr121_polled_reg_cache reg_cache[MPR121_REG_CACHE_SIZE];
+ u8 reg_cache_check_count;
+ u8 reg_cache_next_check_item;
};
struct mpr121_polled_init_register {
@@ -95,6 +111,29 @@ static const struct mpr121_polled_init_register init_reg_table[] = {
{ AUTO_CONFIG_CTRL_ADDR, 0x0b },
};
+static int mpr121_polled_write_reg(struct mpr121_polled *mpr121, u8 addr,
+ u8 value)
+{
+ struct i2c_client *client = mpr121->client;
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, addr, value);
+ if (ret < 0) {
+ dev_err(&client->dev, "i2c write error: %d\n", ret);
+ return ret;
+ }
+
+ if (addr >= MPR121_REG_CACHE_MIN_ADDR &&
+ addr <= MPR121_REG_CACHE_MAX_ADDR) {
+ u8 i = mpr121_addr_to_cache_idx(addr);
+
+ mpr121->reg_cache[i].valid = 1;
+ mpr121->reg_cache[i].value = value;
+ }
+
+ return 0;
+}
+
static void mpr121_polled_vdd_supply_disable(void *data)
{
struct regulator *vdd_supply = data;
@@ -140,18 +179,18 @@ static int mpr121_polled_phys_init(struct mpr121_polled *mpr121,
int i, t, vdd, ret;
/* Set stop mode prior to writing any register */
- ret = i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, 0x00);
+ ret = mpr121_polled_write_reg(mpr121, ELECTRODE_CONF_ADDR, 0x00);
if (ret < 0)
goto err_i2c_write;
/* Set up touch/release threshold for ele0-ele11 */
for (i = 0; i <= MPR121_MAX_KEY_COUNT; i++) {
t = ELE0_TOUCH_THRESHOLD_ADDR + (i * 2);
- ret = i2c_smbus_write_byte_data(client, t, TOUCH_THRESHOLD);
+ ret = mpr121_polled_write_reg(mpr121, t, TOUCH_THRESHOLD);
if (ret < 0)
goto err_i2c_write;
- ret = i2c_smbus_write_byte_data(client, t + 1,
- RELEASE_THRESHOLD);
+ ret = mpr121_polled_write_reg(mpr121, t + 1,
+ RELEASE_THRESHOLD);
if (ret < 0)
goto err_i2c_write;
}
@@ -159,7 +198,7 @@ static int mpr121_polled_phys_init(struct mpr121_polled *mpr121,
/* Set up init register */
for (i = 0; i < ARRAY_SIZE(init_reg_table); i++) {
reg = &init_reg_table[i];
- ret = i2c_smbus_write_byte_data(client, reg->addr, reg->val);
+ ret = mpr121_polled_write_reg(mpr121, reg->addr, reg->val);
if (ret < 0)
goto err_i2c_write;
}
@@ -173,9 +212,9 @@ static int mpr121_polled_phys_init(struct mpr121_polled *mpr121,
usl = ((vdd - 700) * 256) / vdd;
lsl = (usl * 65) / 100;
tl = (usl * 90) / 100;
- ret = i2c_smbus_write_byte_data(client, AUTO_CONFIG_USL_ADDR, usl);
- ret |= i2c_smbus_write_byte_data(client, AUTO_CONFIG_LSL_ADDR, lsl);
- ret |= i2c_smbus_write_byte_data(client, AUTO_CONFIG_TL_ADDR, tl);
+ ret = mpr121_polled_write_reg(mpr121, AUTO_CONFIG_USL_ADDR, usl);
+ ret |= mpr121_polled_write_reg(mpr121, AUTO_CONFIG_LSL_ADDR, lsl);
+ ret |= mpr121_polled_write_reg(mpr121, AUTO_CONFIG_TL_ADDR, tl);
/*
* Quick charge bit will let the capacitive charge to ready
@@ -183,7 +222,7 @@ static int mpr121_polled_phys_init(struct mpr121_polled *mpr121,
* boot.
*/
eleconf = mpr121->keycount | ELECTRODE_CONF_QUICK_CHARGE;
- ret |= i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR,
+ ret |= mpr121_polled_write_reg(mpr121, ELECTRODE_CONF_ADDR,
eleconf);
if (ret != 0)
goto err_i2c_write;
@@ -256,6 +295,36 @@ static int mpr121_polled_process_keys(struct mpr121_polled *mpr121)
return 0;
}
+
+static int mpr121_polled_check_regs(struct mpr121_polled *mpr121)
+{
+ struct i2c_client *client = mpr121->client;
+ int i, reg;
+
+ /* Skip registers that were never written to (have invalid cache) */
+ i = mpr121->reg_cache_next_check_item;
+ for (; i < MPR121_REG_CACHE_SIZE; i++)
+ if (mpr121->reg_cache[i].valid)
+ break;
+
+ if (i == MPR121_REG_CACHE_SIZE) {
+ mpr121->reg_cache_next_check_item = 0;
+ return 0;
+ }
+
+ reg = i2c_smbus_read_byte_data(client, mpr121_cache_idx_to_addr(i));
+ if (reg < 0) {
+ dev_err(&client->dev, "i2c read error: %d\n", reg);
+ return -1;
+ }
+
+ if (reg != mpr121->reg_cache[i].value)
+ return -1;
+
+ mpr121->reg_cache_next_check_item = i + 1;
+ return 0;
+}
+
static void mpr121_poll(struct input_polled_dev *dev)
{
struct mpr121_polled *mpr121 = dev->private;
@@ -282,6 +351,13 @@ static void mpr121_poll(struct input_polled_dev *dev)
}
mpr121->read_errors = 0;
+ mpr121->reg_cache_check_count++;
+ if (mpr121->reg_cache_check_count > MPR121_REG_CACHE_CHECK_LIMIT) {
+ mpr121->reg_cache_check_count = 0;
+ ret = mpr121_polled_check_regs(mpr121);
+ if (ret < 0)
+ mpr121->read_errors++;
+ }
}
static int mpr121_polled_probe(struct i2c_client *client,
@@ -366,8 +442,9 @@ static int mpr121_polled_probe(struct i2c_client *client,
static int __maybe_unused mpr121_polled_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
+ struct mpr121_polled *mpr121 = i2c_get_clientdata(client);
- i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, 0x00);
+ mpr121_polled_write_reg(mpr121, ELECTRODE_CONF_ADDR, 0x00);
return 0;
}
@@ -377,8 +454,7 @@ static int __maybe_unused mpr121_polled_resume(struct device *dev)
struct i2c_client *client = to_i2c_client(dev);
struct mpr121_polled *mpr121 = i2c_get_clientdata(client);
- i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR,
- mpr121->keycount);
+ mpr121_polled_write_reg(mpr121, ELECTRODE_CONF_ADDR, mpr121->keycount);
return 0;
}
--
2.1.4
^ permalink raw reply related
* [RFC PATCH 2/4] Input: mpr121-polled: Add polling variant of the MPR121 touchkey driver
From: Michal Vokáč @ 2019-04-26 8:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring
Cc: Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam, linux-input,
devicetree, linux-kernel, Pengutronix Kernel Team,
Michal Vokáč
In-Reply-To: <1556267420-93219-1-git-send-email-michal.vokac@ysoft.com>
This driver is based on the original driver with interrupts. Polling
driver may be used in cases where the MPR121 chip is connected using
only the I2C interface and the interrupt line is not available.
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
drivers/input/keyboard/Kconfig | 13 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mpr121_touchkey_polled.c | 417 ++++++++++++++++++++++++
3 files changed, 431 insertions(+)
create mode 100644 drivers/input/keyboard/mpr121_touchkey_polled.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a878351f1643..a674e72cdeef 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -418,6 +418,19 @@ config KEYBOARD_MPR121
To compile this driver as a module, choose M here: the
module will be called mpr121_touchkey.
+config KEYBOARD_MPR121_POLLED
+ tristate "Polled Freescale MPR121 Touchkey"
+ depends on I2C
+ help
+ Say Y here if you have Freescale MPR121 touchkey controller
+ chip in your system connected only using the I2C line without
+ the interrupt line.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called mpr121_touchkey_polled.
+
config KEYBOARD_SNVS_PWRKEY
tristate "IMX SNVS Power Key Driver"
depends on SOC_IMX6SX || SOC_IMX7D
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 182e92985dbf..903f50842844 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MPR121_POLLED) += mpr121_touchkey_polled.o
obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
diff --git a/drivers/input/keyboard/mpr121_touchkey_polled.c b/drivers/input/keyboard/mpr121_touchkey_polled.c
new file mode 100644
index 000000000000..e5e80530c9d8
--- /dev/null
+++ b/drivers/input/keyboard/mpr121_touchkey_polled.c
@@ -0,0 +1,417 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Touchkey driver for Freescale MPR121 Controllor
+//
+// Copyright (C) 2011 Freescale Semiconductor, Inc.
+// Author: Zhang Jiejing <jiejing.zhang@freescale.com>
+//
+// Based on mcs_touchkey.c
+//
+// Copyright (C) 2019 Y Soft Corporation, a.s.
+// Author: Pavel Staněk <pavel.stanek@ysoft.com>
+// Author: Michal Vokáč <michal.vokac@ysoft.com>
+//
+// Reworked into polled driver based on mpr121_touchkey.c
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input-polldev.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+/* Register definitions */
+#define ELE_TOUCH_STATUS_0_ADDR 0x0
+#define ELE_TOUCH_STATUS_1_ADDR 0X1
+#define MHD_RISING_ADDR 0x2b
+#define NHD_RISING_ADDR 0x2c
+#define NCL_RISING_ADDR 0x2d
+#define FDL_RISING_ADDR 0x2e
+#define MHD_FALLING_ADDR 0x2f
+#define NHD_FALLING_ADDR 0x30
+#define NCL_FALLING_ADDR 0x31
+#define FDL_FALLING_ADDR 0x32
+#define ELE0_TOUCH_THRESHOLD_ADDR 0x41
+#define ELE0_RELEASE_THRESHOLD_ADDR 0x42
+#define AFE_CONF_ADDR 0x5c
+#define FILTER_CONF_ADDR 0x5d
+
+/*
+ * ELECTRODE_CONF_ADDR: This register configures the number of
+ * enabled capacitance sensing inputs and its run/suspend mode.
+ */
+#define ELECTRODE_CONF_ADDR 0x5e
+#define ELECTRODE_CONF_QUICK_CHARGE 0x80
+#define AUTO_CONFIG_CTRL_ADDR 0x7b
+#define AUTO_CONFIG_USL_ADDR 0x7d
+#define AUTO_CONFIG_LSL_ADDR 0x7e
+#define AUTO_CONFIG_TL_ADDR 0x7f
+
+/* Threshold of touch/release trigger */
+#define TOUCH_THRESHOLD 0x08
+#define RELEASE_THRESHOLD 0x05
+/* Masks for touch and release triggers */
+#define TOUCH_STATUS_MASK 0xfff
+/* MPR121 has 12 keys */
+#define MPR121_MAX_KEY_COUNT 12
+
+#define MPR121_POLL_INTERVAL 50
+#define MPR121_POLL_INTERVAL_MIN 10
+#define MPR121_POLL_INTERVAL_MAX 200
+#define MPR121_POLL_INTERVAL_REINIT 500
+#define MPR121_POLL_RETRY_MAX 4
+
+struct mpr121_polled {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ struct input_polled_dev *poll_dev;
+ unsigned int statusbits;
+ unsigned int keycount;
+ u32 keycodes[MPR121_MAX_KEY_COUNT];
+ u8 read_errors;
+ int vdd_uv;
+};
+
+struct mpr121_polled_init_register {
+ int addr;
+ u8 val;
+};
+
+static const struct mpr121_polled_init_register init_reg_table[] = {
+ { MHD_RISING_ADDR, 0x1 },
+ { NHD_RISING_ADDR, 0x1 },
+ { MHD_FALLING_ADDR, 0x1 },
+ { NHD_FALLING_ADDR, 0x1 },
+ { NCL_FALLING_ADDR, 0xff },
+ { FDL_FALLING_ADDR, 0x02 },
+ { FILTER_CONF_ADDR, 0x04 },
+ { AFE_CONF_ADDR, 0x0b },
+ { AUTO_CONFIG_CTRL_ADDR, 0x0b },
+};
+
+static void mpr121_polled_vdd_supply_disable(void *data)
+{
+ struct regulator *vdd_supply = data;
+
+ regulator_disable(vdd_supply);
+}
+
+static struct regulator *mpr121_polled_vdd_supply_init(struct device *dev)
+{
+ struct regulator *vdd_supply;
+ int err;
+
+ vdd_supply = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(vdd_supply)) {
+ dev_err(dev, "failed to get vdd regulator: %ld\n",
+ PTR_ERR(vdd_supply));
+ return vdd_supply;
+ }
+
+ err = regulator_enable(vdd_supply);
+ if (err) {
+ dev_err(dev, "failed to enable vdd regulator: %d\n", err);
+ return ERR_PTR(err);
+ }
+
+ err = devm_add_action(dev, mpr121_polled_vdd_supply_disable,
+ vdd_supply);
+ if (err) {
+ regulator_disable(vdd_supply);
+ dev_err(dev, "failed to add disable regulator action: %d\n",
+ err);
+ return ERR_PTR(err);
+ }
+
+ return vdd_supply;
+}
+
+static int mpr121_polled_phys_init(struct mpr121_polled *mpr121,
+ struct i2c_client *client)
+{
+ const struct mpr121_polled_init_register *reg;
+ unsigned char usl, lsl, tl, eleconf;
+ int i, t, vdd, ret;
+
+ /* Set stop mode prior to writing any register */
+ ret = i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, 0x00);
+ if (ret < 0)
+ goto err_i2c_write;
+
+ /* Set up touch/release threshold for ele0-ele11 */
+ for (i = 0; i <= MPR121_MAX_KEY_COUNT; i++) {
+ t = ELE0_TOUCH_THRESHOLD_ADDR + (i * 2);
+ ret = i2c_smbus_write_byte_data(client, t, TOUCH_THRESHOLD);
+ if (ret < 0)
+ goto err_i2c_write;
+ ret = i2c_smbus_write_byte_data(client, t + 1,
+ RELEASE_THRESHOLD);
+ if (ret < 0)
+ goto err_i2c_write;
+ }
+
+ /* Set up init register */
+ for (i = 0; i < ARRAY_SIZE(init_reg_table); i++) {
+ reg = &init_reg_table[i];
+ ret = i2c_smbus_write_byte_data(client, reg->addr, reg->val);
+ if (ret < 0)
+ goto err_i2c_write;
+ }
+
+ /*
+ * Capacitance on sensing input varies and needs to be compensated.
+ * The internal MPR121-auto-configuration can do this if it's
+ * registers are set properly (based on vdd_uv).
+ */
+ vdd = mpr121->vdd_uv / 1000;
+ usl = ((vdd - 700) * 256) / vdd;
+ lsl = (usl * 65) / 100;
+ tl = (usl * 90) / 100;
+ ret = i2c_smbus_write_byte_data(client, AUTO_CONFIG_USL_ADDR, usl);
+ ret |= i2c_smbus_write_byte_data(client, AUTO_CONFIG_LSL_ADDR, lsl);
+ ret |= i2c_smbus_write_byte_data(client, AUTO_CONFIG_TL_ADDR, tl);
+
+ /*
+ * Quick charge bit will let the capacitive charge to ready
+ * state quickly, or the buttons may not function after system
+ * boot.
+ */
+ eleconf = mpr121->keycount | ELECTRODE_CONF_QUICK_CHARGE;
+ ret |= i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR,
+ eleconf);
+ if (ret != 0)
+ goto err_i2c_write;
+
+ dev_dbg(&client->dev, "set up with %x keys.\n", mpr121->keycount);
+
+ return 0;
+
+err_i2c_write:
+ dev_err(&client->dev, "i2c write error: %d\n", ret);
+ return ret;
+}
+
+static void mpr121_polled_release_keys(struct mpr121_polled *mpr121)
+{
+ struct input_dev *input = mpr121->input_dev;
+ struct i2c_client *client = mpr121->client;
+ unsigned long statusbits;
+ unsigned int key_num;
+
+ if (!mpr121->statusbits)
+ return;
+
+ statusbits = mpr121->statusbits;
+ mpr121->statusbits = 0;
+ for_each_set_bit(key_num, &statusbits, mpr121->keycount) {
+ unsigned int key_val;
+
+ key_val = mpr121->keycodes[key_num];
+
+ input_event(input, EV_MSC, MSC_SCAN, key_num);
+ input_report_key(input, key_val, 0);
+
+ dev_dbg(&client->dev, "key %d %d %s\n", key_num, key_val,
+ "released");
+ }
+ input_sync(input);
+}
+
+static int mpr121_polled_process_keys(struct mpr121_polled *mpr121)
+{
+ struct input_dev *input = mpr121->input_dev;
+ struct i2c_client *client = mpr121->client;
+ unsigned long bit_changed;
+ unsigned int key_num;
+ int reg;
+
+ reg = i2c_smbus_read_word_data(client, ELE_TOUCH_STATUS_0_ADDR);
+ if (reg < 0) {
+ dev_err(&client->dev, "i2c read error: %d\n", reg);
+ return reg;
+ }
+
+ reg &= TOUCH_STATUS_MASK;
+ bit_changed = reg ^ mpr121->statusbits;
+ mpr121->statusbits = reg;
+ for_each_set_bit(key_num, &bit_changed, mpr121->keycount) {
+ unsigned int key_val, pressed;
+
+ pressed = reg & BIT(key_num);
+ key_val = mpr121->keycodes[key_num];
+
+ input_event(input, EV_MSC, MSC_SCAN, key_num);
+ input_report_key(input, key_val, pressed);
+
+ dev_dbg(&client->dev, "key %d %d %s\n", key_num, key_val,
+ pressed ? "pressed" : "released");
+ }
+ input_sync(input);
+
+ return 0;
+}
+static void mpr121_poll(struct input_polled_dev *dev)
+{
+ struct mpr121_polled *mpr121 = dev->private;
+ struct i2c_client *client = mpr121->client;
+ int ret;
+
+ if (mpr121->read_errors > MPR121_POLL_RETRY_MAX) {
+ dev_warn(&client->dev,
+ "device does not respond, re-initializing\n");
+ mpr121_polled_release_keys(mpr121);
+ ret = mpr121_polled_phys_init(mpr121, client);
+ if (ret >= 0) {
+ mpr121->read_errors = 0;
+ dev->poll_interval = MPR121_POLL_INTERVAL;
+ } else {
+ dev->poll_interval = MPR121_POLL_INTERVAL_REINIT;
+ }
+ }
+
+ ret = mpr121_polled_process_keys(mpr121);
+ if (ret < 0) {
+ mpr121->read_errors++;
+ return;
+ }
+
+ mpr121->read_errors = 0;
+}
+
+static int mpr121_polled_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct regulator *vdd_supply;
+ struct mpr121_polled *mpr121;
+ struct input_dev *input_dev;
+ struct input_polled_dev *poll_dev;
+ int error;
+ int i;
+
+ vdd_supply = mpr121_polled_vdd_supply_init(dev);
+ if (IS_ERR(vdd_supply))
+ return PTR_ERR(vdd_supply);
+
+ mpr121 = devm_kzalloc(dev, sizeof(*mpr121), GFP_KERNEL);
+ if (!mpr121)
+ return -ENOMEM;
+
+ poll_dev = devm_input_allocate_polled_device(dev);
+ if (!poll_dev)
+ return -ENOMEM;
+
+ mpr121->vdd_uv = regulator_get_voltage(vdd_supply);
+ mpr121->client = client;
+ mpr121->input_dev = poll_dev->input;
+ mpr121->poll_dev = poll_dev;
+ mpr121->keycount = device_property_read_u32_array(dev, "linux,keycodes",
+ NULL, 0);
+ if (mpr121->keycount > MPR121_MAX_KEY_COUNT) {
+ dev_err(dev, "too many keys defined (%d)\n", mpr121->keycount);
+ return -EINVAL;
+ }
+
+ error = device_property_read_u32_array(dev, "linux,keycodes",
+ mpr121->keycodes,
+ mpr121->keycount);
+ if (error) {
+ dev_err(dev,
+ "failed to read linux,keycode property: %d\n", error);
+ return error;
+ }
+
+ poll_dev->private = mpr121;
+ poll_dev->poll = mpr121_poll;
+ poll_dev->poll_interval = MPR121_POLL_INTERVAL;
+ poll_dev->poll_interval_max = MPR121_POLL_INTERVAL_MAX;
+ poll_dev->poll_interval_min = MPR121_POLL_INTERVAL_MIN;
+
+ input_dev = poll_dev->input;
+ input_dev->name = "Freescale MPR121 Polled Touchkey";
+ input_dev->id.bustype = BUS_I2C;
+ input_dev->dev.parent = dev;
+ if (device_property_read_bool(dev, "autorepeat"))
+ __set_bit(EV_REP, input_dev->evbit);
+ input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+
+ input_dev->keycode = mpr121->keycodes;
+ input_dev->keycodesize = sizeof(mpr121->keycodes[0]);
+ input_dev->keycodemax = mpr121->keycount;
+
+ for (i = 0; i < mpr121->keycount; i++)
+ input_set_capability(input_dev, EV_KEY, mpr121->keycodes[i]);
+
+ error = mpr121_polled_phys_init(mpr121, client);
+ if (error) {
+ dev_err(dev, "Failed to init register\n");
+ return error;
+ }
+
+ error = input_register_polled_device(poll_dev);
+ if (error)
+ return error;
+
+ i2c_set_clientdata(client, mpr121);
+
+ return 0;
+}
+
+static int __maybe_unused mpr121_polled_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR, 0x00);
+
+ return 0;
+}
+
+static int __maybe_unused mpr121_polled_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct mpr121_polled *mpr121 = i2c_get_clientdata(client);
+
+ i2c_smbus_write_byte_data(client, ELECTRODE_CONF_ADDR,
+ mpr121->keycount);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mpr121_polled_pm_ops, mpr121_polled_suspend,
+ mpr121_polled_resume);
+
+static const struct i2c_device_id mpr121_polled_id[] = {
+ { "mpr121_polled", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mpr121_polled_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id mpr121_polled_dt_match_table[] = {
+ { .compatible = "fsl,mpr121-touchkey-polled" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, mpr121_polled_dt_match_table);
+#endif
+
+static struct i2c_driver mpr121_polled_driver = {
+ .driver = {
+ .name = "mpr121-polled",
+ .pm = &mpr121_polled_pm_ops,
+ .of_match_table = of_match_ptr(mpr121_polled_dt_match_table),
+ },
+ .id_table = mpr121_polled_id,
+ .probe = mpr121_polled_probe,
+};
+
+module_i2c_driver(mpr121_polled_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Michal Vokáč <michal.vokac@ysoft.com>");
+MODULE_DESCRIPTION("Polled driver for Freescale MPR121 chip");
--
2.1.4
^ permalink raw reply related
* [RFC PATCH 1/4] dt-bindings: input: Add support for the MPR121 without interrupt line
From: Michal Vokáč @ 2019-04-26 8:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring
Cc: Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam, linux-input,
devicetree, linux-kernel, Pengutronix Kernel Team,
Michal Vokáč
In-Reply-To: <1556267420-93219-1-git-send-email-michal.vokac@ysoft.com>
Normally, the MPR121 controller uses separate interrupt line to notify
the I2C host that a key was touched/released. To support platforms that
can not use the interrupt line, polling of the MPR121 registers can be
used.
Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
.../bindings/input/mpr121-touchkey-polled.txt | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mpr121-touchkey-polled.txt
diff --git a/Documentation/devicetree/bindings/input/mpr121-touchkey-polled.txt b/Documentation/devicetree/bindings/input/mpr121-touchkey-polled.txt
new file mode 100644
index 000000000000..6bb1d312614c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mpr121-touchkey-polled.txt
@@ -0,0 +1,26 @@
+* Freescale MPR121 Controller without interrupt line
+
+Required Properties:
+- compatible: Should be "fsl,mpr121-touchkey-polled"
+- reg: The I2C slave address of the device.
+- vdd-supply: Phandle to the Vdd power supply.
+- linux,keycodes: Specifies an array of numeric keycode values to
+ be used for reporting button presses. The array can
+ contain up to 12 entries.
+
+Optional Properties:
+- autorepeat: Enable autorepeat feature.
+
+Example:
+
+#include "dt-bindings/input/input.h"
+
+ touchkeys: keys@5a {
+ compatible = "fsl,mpr121-touchkey-polled";
+ reg = <0x5a>;
+ autorepeat;
+ vdd-supply = <&ldo4_reg>;
+ linux,keycodes = <KEY_0>, <KEY_1>, <KEY_2>, <KEY_3>,
+ <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
+ <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
+ };
--
2.1.4
^ permalink raw reply related
* [RFC PATCH 0/4] Input: mpr121-polled: Add polled driver for MPR121
From: Michal Vokáč @ 2019-04-26 8:30 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring
Cc: Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam, linux-input,
devicetree, linux-kernel, Pengutronix Kernel Team,
Michal Vokáč
Hi,
I have to deal with a situation where we have a custom i.MX6 based
platform in production that uses the MPR121 touchkey controller.
Unfortunately the chip is connected using only the I2C interface.
The interrupt line is not used. Back in 2015 (Linux v3.14), my
colleague modded the existing mpr121_touchkey.c driver to use polling
instead of interrupt.
For quite some time yet I am in a process of updating the product from
the ancient Freescale v3.14 kernel to the latest mainline and pushing
any needed changes upstream. The DT files for our imx6dl-yapp4 platform
already made it into v5.1-rc.
I rebased and updated our mpr121 patch to the latest mainline.
It is created as a separate driver, similarly to gpio_keys_polled.
The I2C device is quite susceptible to ESD. An ESD test quite often
causes reset of the chip or some register randomly changes its value.
The [PATCH 3/4] adds a write-through register cache. With the cache
this state can be detected and the device can be re-initialied.
The main question is: Is there any chance that such a polled driver
could be accepted? Is it correct to implement it as a separate driver
or should it be done as an option in the existing driver? I can not
really imagine how I would do that though..
There are also certain worries that the MPR121 chip may no longer be
available in nonspecifically distant future. In case of EOL I will need
to add a polled driver for an other touchkey chip. May it be already
in mainline or a completely new one.
I am also little bit confused from the dt-binding documentation.
The MPR121 is mentioned in the trivial-devices.yaml but it also has
its own binding documentation in input/mpr121_touchkey.txt.
I thought that a certain device/compatible should be documented in just
one place. The MPR121 device certainly needs more properties than
just compatible, reg and interrupt so trivial-devices.yaml does not
seem appropriate.
I will appreciate any comments. Thank you in advance,
Michal
Michal Vokáč (4):
dt-bindings: input: Add support for the MPR121 without interrupt line
Input: mpr121-polled: Add polling variant of the MPR121 touchkey
driver
Input: mpr121-polled: Add write-through cache to detect corrupted
registers
ARM: dts: imx6dl-yapp4: Enable MPR121 touch keypad on Hydra
.../bindings/input/mpr121-touchkey-polled.txt | 26 ++
arch/arm/boot/dts/imx6dl-yapp4-common.dtsi | 12 +
arch/arm/boot/dts/imx6dl-yapp4-hydra.dts | 4 +
drivers/input/keyboard/Kconfig | 13 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mpr121_touchkey_polled.c | 493 +++++++++++++++++++++
6 files changed, 549 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mpr121-touchkey-polled.txt
create mode 100644 drivers/input/keyboard/mpr121_touchkey_polled.c
--
2.1.4
^ permalink raw reply
* Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl
From: Arnd Bergmann @ 2019-04-26 7:46 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sean Young,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Daniel Vetter, linux-pci,
dri-devel, Bjorn Andersson, sparclinux, Mauro Carvalho Chehab,
driverdevel, linux-scsi, linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
y2038 Mailman List, qat-linux-ral2JQCrhuEAvxtiuMwx3w,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jason Gunthorpe,
open list:HID CORE LAYER, Darren Hart, Linux Media Mailing List,
linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, Al Viro, Jonathan Cameron
In-Reply-To: <5511420228cb38d08a67c0f6a614b7671d7d23d4.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
On Thu, Apr 25, 2019 at 11:25 PM Johannes Berg
<johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Thu, 2019-04-25 at 17:55 +0200, Arnd Bergmann wrote:
> > On Thu, Apr 25, 2019 at 5:35 PM Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org> wrote:
> > >
> > > On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote:
> > >
> > > > If I understand your patch description well, using compat_ptr_ioctl
> > > > only works if the driver is not for s390, right?
> > >
> > > No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl
> > > and be done with that; compat_ptr() is a no-op anyway" breaks. IOW,
> > > s390 is the reason for having compat_ptr_ioctl() in the first place;
> > > that thing works on all biarch architectures, as long as all stuff
> > > handled by ->ioctl() takes pointer to arch-independent object as
> > > argument. IOW,
> > > argument ignored => OK
> > > any arithmetical type => no go, compat_ptr() would bugger it
> > > pointer to int => OK
> > > pointer to string => OK
> > > pointer to u64 => OK
> > > pointer to struct {u64 addr; char s[11];} => OK
> >
> > To be extra pedantic, the 'struct {u64 addr; char s[11];} '
> > case is also broken on x86, because sizeof (obj) is smaller
> > on i386, even though the location of the members are
> > the same. i.e. you can copy_from_user() this
>
> Actually, you can't even do that because the struct might sit at the end
> of a page and then you'd erroneously fault in this case.
>
> We had this a while ago with struct ifreq, see commit 98406133dd and its
> parents.
Yes, you are right. Very rare to hit with real-life code, but easily
reproduced by intentionally hitting it and clearly a bug.
As the saying goes
| the difference between "always works" and "almost always works"
| is called data corruption
here the difference is an -EFAULT.
Arnd
^ permalink raw reply
* Re: [PATCH 2/2] HID: input: add mapping for KEY_KBD_LAYOUT_NEXT
From: Benjamin Tissoires @ 2019-04-26 6:38 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml, gwink
In-Reply-To: <20190425163846.51730-2-dmitry.torokhov@gmail.com>
On Thu, Apr 25, 2019 at 6:38 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> HUTRR56 defined a new usage code on consumer page to cycle through
> set of keyboard layouts, let's add this mapping.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
I don't think this will collide with the HID tree, so IMO, you can
take this through yours if you want.
Cheers,
Benjamin
> drivers/hid/hid-input.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index b607286a0bc8..0579b8d3f912 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1051,6 +1051,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> case 0x28b: map_key_clear(KEY_FORWARDMAIL); break;
> case 0x28c: map_key_clear(KEY_SEND); break;
>
> + case 0x29d: map_key_clear(KEY_KBD_LAYOUT_NEXT); break;
> +
> case 0x2c7: map_key_clear(KEY_KBDINPUTASSIST_PREV); break;
> case 0x2c8: map_key_clear(KEY_KBDINPUTASSIST_NEXT); break;
> case 0x2c9: map_key_clear(KEY_KBDINPUTASSIST_PREVGROUP); break;
> --
> 2.21.0.593.g511ec345e18-goog
>
^ permalink raw reply
* Re: [PATCH 1/3] mfd: apple-ibridge: Add Apple iBridge MFD driver.
From: Benjamin Tissoires @ 2019-04-26 6:26 UTC (permalink / raw)
To: Life is hard, and then you die
Cc: Jiri Kosina, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, open list:HID CORE LAYER,
linux-iio, lkml
In-Reply-To: <20190426055632.GC31266@innovation.ch>
On Fri, Apr 26, 2019 at 7:56 AM Life is hard, and then you die
<ronald@innovation.ch> wrote:
>
>
> Hi Benjamin,
>
> On Thu, Apr 25, 2019 at 11:39:12AM +0200, Benjamin Tissoires wrote:
> > On Thu, Apr 25, 2019 at 10:19 AM Life is hard, and then you die
> > <ronald@innovation.ch> wrote:
> > >
> > > Hi Benjamin,
> > >
> > > Thank you for looking at this.
> > >
> > > On Wed, Apr 24, 2019 at 04:18:23PM +0200, Benjamin Tissoires wrote:
> > > > On Mon, Apr 22, 2019 at 5:13 AM Ronald Tschalär <ronald@innovation.ch> wrote:
> > > > >
> > > > > The iBridge device provides access to several devices, including:
> > > > > - the Touch Bar
> > > > > - the iSight webcam
> > > > > - the light sensor
> > > > > - the fingerprint sensor
> > > > >
> > > > > This driver provides the core support for managing the iBridge device
> > > > > and the access to the underlying devices. In particular, since the
> > > > > functionality for the touch bar and light sensor is exposed via USB HID
> > > > > interfaces, and the same HID device is used for multiple functions, this
> > > > > driver provides a multiplexing layer that allows multiple HID drivers to
> > > > > be registered for a given HID device. This allows the touch bar and ALS
> > > > > driver to be separated out into their own modules.
> > > >
> > > > Sorry for coming late to the party, but IMO this series is far too
> > > > complex for what you need.
> > > >
> > > > As I read this and the first comment of drivers/mfd/apple-ibridge.c,
> > > > you need to have a HID driver that multiplex 2 other sub drivers
> > > > through one USB communication.
> > > > For that, you are using MFD, platform driver and you own sauce instead
> > > > of creating a bus.
> > >
> > > Basically correct. To be a bit more precise, there are currently two
> > > hid-devices and two drivers (touchbar and als) involved, with
> > > connections as follows (pardon the ugly ascii art):
> > >
> > > hdev1 --- tb-drv
> > > /
> > > /
> > > /
> > > hdev2 --- als-drv
> > >
> > > i.e. the touchbar driver talks to both hdev's, and hdev2's events
> > > (reports) are processed by both drivers (though each handles different
> > > reports).
> > >
> > > > So, how about we reuse entirely the HID subsystem which already
> > > > provides the capability you need (assuming I am correct above).
> > > > hid-logitech-dj already does the same kind of stuff and you could:
> > > > - create drivers/hid/hid-ibridge.c that handles USB_ID_PRODUCT_IBRIDGE
> > > > - hid-ibridge will then register itself to the hid subsystem with a
> > > > call to hid_hw_start(hdev, HID_CONNECT_HIDRAW) and
> > > > hid_device_io_start(hdev) to enable the events (so you don't create
> > > > useless input nodes for it)
> > > > - then you add your 2 new devices by calling hid_allocate_device() and
> > > > then hid_add_device(). You can even create a new HID group
> > > > APPLE_IBRIDGE and allocate 2 new PIDs for them to distinguish them
> > > > from the actual USB device.
> > > > - then you have 2 brand new HID devices you can create their driver as
> > > > a regular ones.
> > > >
> > > > hid-ibridge.c would just need to behave like any other hid transport
> > > > driver (see logi_dj_ll_driver in drivers/hid/hid-logitech-dj.c) and
> > > > you can get rid of at least the MFD and the platform part of your
> > > > drivers.
> > > >
> > > > Does it makes sense or am I missing something obvious in the middle?
> > >
> > > Yes, I think I understand, and I think this can work. Basically,
> > > instead of demux'ing at the hid-driver level as I am doing now (i.e.
> > > the iBridge hid-driver forwarding calls to the sub-hid-drivers), we
> > > demux at the hid-device level (events forwarded from iBridge hdev to
> > > all "virtual" sub-hdev's, and requests from sub-hdev's forwarded to
> > > the original hdev via an iBridge ll_driver attached to the
> > > sub-hdev's).
> > >
> > > So I would need to create 3 new "virtual" hid-devices (instances) as
> > > follows:
> > >
> > > hdev1 --- vhdev1 --- tb-drv
> > > /
> > > -- vhdev2 --
> > > /
> > > hdev2 --- vhdev3 --- als-drv
> > >
> > > (vhdev1 is probably not strictly necessary, but makes things more
> > > consistent).
> >
> > Oh, ok.
> >
> > How about the following:
> >
> > hdev1 and hdev2 are merged together in hid-apple-ibridge.c, and then
> > this driver creates 2 virtual hid drivers that are consistent
> >
> > like
> >
> > hdev1---ibridge-drv---vhdev1---tb-drv
> > hdev2--/ \--vhdev2---als-drv
>
> I don't think this will work. The problem is when the sub-drivers need
> to send a report or usb-command: how to they specify which hdev the
> report/command is destined for? While we could store the original hdev
> in each report (the hid_report's device field), that only works for
> hid_hw_request(), but not for things like hid_hw_raw_request() or
> hid_hw_output_report(). Now, currently I don't use the latter two; but
> I do need to send raw usb control messages in the touchbar driver
> (some commands are not proper hid reports), so it definitely breaks
> down there.
>
> Or am I missing something?
>
I'd need to have a deeper look at the protocol, but you can emulate
pure HID devices by having your ibridge handling a translation from
set/get features/input to the usb control messages. Likewise, nothing
prevents you to slightly rewrite the report descriptors you present to
the als and touchbar to have a clear separation with the report ID.
For example, if both hdev1 and hdev2 use a report ID of 0x01, you
could rewrite the report descriptor so that when you receive a report
with an id of 0x01 you send this to hdev1, but you can also translate
0x11 to a report ID 0x01 to hdev2.
Likewise, report ID 0x42 could be a raw USB control message to the USB
under hdev2.
Note that you will have to write 2 report descriptors for your new
devices, but you can take what makes sense from the original ones, and
just add a new collection with a vendor application with with an
opaque meaning (for the USB control messages).
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/3] mfd: apple-ibridge: Add Apple iBridge MFD driver.
From: Life is hard, and then you die @ 2019-04-26 5:56 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, open list:HID CORE LAYER,
linux-iio, lkml
In-Reply-To: <CAO-hwJKcuptG6X7Y0jrQzyLUzZfsXoyWxMmk=v_aPxJh9iv4Gg@mail.gmail.com>
Hi Benjamin,
On Thu, Apr 25, 2019 at 11:39:12AM +0200, Benjamin Tissoires wrote:
> On Thu, Apr 25, 2019 at 10:19 AM Life is hard, and then you die
> <ronald@innovation.ch> wrote:
> >
> > Hi Benjamin,
> >
> > Thank you for looking at this.
> >
> > On Wed, Apr 24, 2019 at 04:18:23PM +0200, Benjamin Tissoires wrote:
> > > On Mon, Apr 22, 2019 at 5:13 AM Ronald Tschalär <ronald@innovation.ch> wrote:
> > > >
> > > > The iBridge device provides access to several devices, including:
> > > > - the Touch Bar
> > > > - the iSight webcam
> > > > - the light sensor
> > > > - the fingerprint sensor
> > > >
> > > > This driver provides the core support for managing the iBridge device
> > > > and the access to the underlying devices. In particular, since the
> > > > functionality for the touch bar and light sensor is exposed via USB HID
> > > > interfaces, and the same HID device is used for multiple functions, this
> > > > driver provides a multiplexing layer that allows multiple HID drivers to
> > > > be registered for a given HID device. This allows the touch bar and ALS
> > > > driver to be separated out into their own modules.
> > >
> > > Sorry for coming late to the party, but IMO this series is far too
> > > complex for what you need.
> > >
> > > As I read this and the first comment of drivers/mfd/apple-ibridge.c,
> > > you need to have a HID driver that multiplex 2 other sub drivers
> > > through one USB communication.
> > > For that, you are using MFD, platform driver and you own sauce instead
> > > of creating a bus.
> >
> > Basically correct. To be a bit more precise, there are currently two
> > hid-devices and two drivers (touchbar and als) involved, with
> > connections as follows (pardon the ugly ascii art):
> >
> > hdev1 --- tb-drv
> > /
> > /
> > /
> > hdev2 --- als-drv
> >
> > i.e. the touchbar driver talks to both hdev's, and hdev2's events
> > (reports) are processed by both drivers (though each handles different
> > reports).
> >
> > > So, how about we reuse entirely the HID subsystem which already
> > > provides the capability you need (assuming I am correct above).
> > > hid-logitech-dj already does the same kind of stuff and you could:
> > > - create drivers/hid/hid-ibridge.c that handles USB_ID_PRODUCT_IBRIDGE
> > > - hid-ibridge will then register itself to the hid subsystem with a
> > > call to hid_hw_start(hdev, HID_CONNECT_HIDRAW) and
> > > hid_device_io_start(hdev) to enable the events (so you don't create
> > > useless input nodes for it)
> > > - then you add your 2 new devices by calling hid_allocate_device() and
> > > then hid_add_device(). You can even create a new HID group
> > > APPLE_IBRIDGE and allocate 2 new PIDs for them to distinguish them
> > > from the actual USB device.
> > > - then you have 2 brand new HID devices you can create their driver as
> > > a regular ones.
> > >
> > > hid-ibridge.c would just need to behave like any other hid transport
> > > driver (see logi_dj_ll_driver in drivers/hid/hid-logitech-dj.c) and
> > > you can get rid of at least the MFD and the platform part of your
> > > drivers.
> > >
> > > Does it makes sense or am I missing something obvious in the middle?
> >
> > Yes, I think I understand, and I think this can work. Basically,
> > instead of demux'ing at the hid-driver level as I am doing now (i.e.
> > the iBridge hid-driver forwarding calls to the sub-hid-drivers), we
> > demux at the hid-device level (events forwarded from iBridge hdev to
> > all "virtual" sub-hdev's, and requests from sub-hdev's forwarded to
> > the original hdev via an iBridge ll_driver attached to the
> > sub-hdev's).
> >
> > So I would need to create 3 new "virtual" hid-devices (instances) as
> > follows:
> >
> > hdev1 --- vhdev1 --- tb-drv
> > /
> > -- vhdev2 --
> > /
> > hdev2 --- vhdev3 --- als-drv
> >
> > (vhdev1 is probably not strictly necessary, but makes things more
> > consistent).
>
> Oh, ok.
>
> How about the following:
>
> hdev1 and hdev2 are merged together in hid-apple-ibridge.c, and then
> this driver creates 2 virtual hid drivers that are consistent
>
> like
>
> hdev1---ibridge-drv---vhdev1---tb-drv
> hdev2--/ \--vhdev2---als-drv
I don't think this will work. The problem is when the sub-drivers need
to send a report or usb-command: how to they specify which hdev the
report/command is destined for? While we could store the original hdev
in each report (the hid_report's device field), that only works for
hid_hw_request(), but not for things like hid_hw_raw_request() or
hid_hw_output_report(). Now, currently I don't use the latter two; but
I do need to send raw usb control messages in the touchbar driver
(some commands are not proper hid reports), so it definitely breaks
down there.
Or am I missing something?
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH 1/3] mfd: apple-ibridge: Add Apple iBridge MFD driver.
From: Life is hard, and then you die @ 2019-04-26 5:34 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jiri Kosina, Benjamin Tissoires, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Lee Jones,
linux-input, linux-iio, linux-kernel
In-Reply-To: <20190424201317.2a472120@archlinux>
Hi Jonathan,
On Wed, Apr 24, 2019 at 08:13:17PM +0100, Jonathan Cameron wrote:
> On Wed, 24 Apr 2019 03:47:18 -0700
> "Life is hard, and then you die" <ronald@innovation.ch> wrote:
>
> > Hi Jonathan,
> >
> > On Mon, Apr 22, 2019 at 12:34:26PM +0100, Jonathan Cameron wrote:
> > > On Sun, 21 Apr 2019 20:12:49 -0700
> > > Ronald Tschalär <ronald@innovation.ch> wrote:
> > >
> > > > The iBridge device provides access to several devices, including:
> > > > - the Touch Bar
> > > > - the iSight webcam
> > > > - the light sensor
> > > > - the fingerprint sensor
> > > >
> > > > This driver provides the core support for managing the iBridge device
> > > > and the access to the underlying devices. In particular, since the
> > > > functionality for the touch bar and light sensor is exposed via USB HID
> > > > interfaces, and the same HID device is used for multiple functions, this
> > > > driver provides a multiplexing layer that allows multiple HID drivers to
> > > > be registered for a given HID device. This allows the touch bar and ALS
> > > > driver to be separated out into their own modules.
> > > >
> > > > Signed-off-by: Ronald Tschalär <ronald@innovation.ch
> > > Hi Ronald,
> > >
> > > I've only taken a fairly superficial look at this. A few global
> > > things to note though.
> >
> > Thanks for this review.
[snip]
I've applied all your feedback in my tree, but it now looks like this
module is going to be redone differently. I'll try to keep all your
comments in mind during the rewrite, though, so they're not wasted.
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH 1/2] HID: wacom: Don't set tool type until we're in range
From: Sasha Levin @ 2019-04-26 3:33 UTC (permalink / raw)
To: Jason Gerecke; +Cc: Jason Gerecke, Linux Input, Ping Cheng, # v3.17+
In-Reply-To: <CANRwn3QVG2XBr-h53kimQkQ0W=KOuTxHppzTLcS=i7CFiqC_bA@mail.gmail.com>
On Thu, Apr 25, 2019 at 10:47:42AM -0700, Jason Gerecke wrote:
>I can produce a version of this patch specific to v4.14.113. Please
>let me know the proper process for submitting such a patch.
Just replying to this mail will do the job, thanks!
^ permalink raw reply
* Re: [PATCH 0/5] Add of_ functions for device_link_add()
From: Rob Herring @ 2019-04-25 23:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Gaignard, Wysocki, Rafael J, Mark Rutland,
Bastien Nocera, Frank Rowand, Marco Felsch, Guido Günther,
Yannick Fertre, Arnd Bergmann, Linux Input, DTML,
linux-kernel@vger.kernel.org, linux-stm32, Mark Brown
In-Reply-To: <CAKdAkRTUsJ4TU7BrSNXCKiQGZnArao9o_qk7i0xSzYJ1SGU14A@mail.gmail.com>
On Thu, Apr 25, 2019 at 2:24 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Thu, Apr 25, 2019 at 11:08 AM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Wed, Apr 24, 2019 at 5:19 AM Benjamin Gaignard
> > <benjamin.gaignard@st.com> wrote:
> > >
> > > It could happen that we need to control suspend/resume ordering between
> > > devices without obvious consumer/supplier link. For example when touchscreens
> > > and DSI panels share the same reset line, in this case we need to be sure
> > > of pm_runtime operations ordering between those two devices to correctly
> > > perform reset.
> > > DSI panel and touchscreen aren't sharing any heriachical relationship (unlike
> > > I2C client and I2C bus or regulator client and regulator provider) so we need
> > > to describe this in device-tree.
> >
> > Needing to know which touchscreen is attached to a panel could be
> > important to describe if you have multiple displays.
> >
> > Doesn't the reset subsystem already have some support for shared
> > resets? Seems like it could provide clients with struct device or
> > device_node ptrs to other devices sharing a reset.
> >
> > >
> > > This series introduce of_device_links_{add,remove} and devm_of_device_links_add()
> > > helpers to find and parse 'links-add' property in a device-tree node.
> >
> > Going to document that property somewhere? :)
> >
> > I think this is too generic and coupled to Linux. It doesn't have any
> > information as to what is the dependency or connection nor what the
> > direction of the dependency is.
> >
> > I'm not convinced we need to solve this generically vs. defining
> > something for this specific example.
>
> I am pretty sure there will be more drivers needing complex
> dependencies. Doesn't ACPI allow defining relationship between devices
> that goes beyond the tree structure?
Can't speak to ACPI, but I assume you where implying ACPI supports
this so DT should too.
Almost every binding we have is defining relationships between
devices. Interrupts, clocks, gpio, pinctrl, etc. all do this. To use a
similar example to the one here, we already define the relationship
between a display and a backlight with a 'backlight' property in the
display node. Why should touchscreen be any different than backlight?
What really concerns me here is folks just add relationships to
'links-add' which are already captured in DT (such as backlight) just
because they'll get it for free and not have to go add support to
handle each specific binding.
Rob
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox