* Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
From: Ivan T. Ivanov @ 2014-10-08 9:30 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Stephen Boyd, linux-input, linux-kernel, linux-arm-msm
In-Reply-To: <1412759584.13569.0.camel@iivanov-dev>
On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote:
> On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote:
> > Hi Ivan,
> >
> > On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote:
> > > @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
> > >
>
> > > +
> > > + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap,
> > > + info->row_hold);
> > > + if (IS_ERR(kp->row_hold))
> > > + return PTR_ERR(kp->row_hold);
> >
> > Why do we have to allocate all regmap fields separately instead of
> > embedding them into keypad structure?
> >
>
> No particular reason. Will rework it.
>
Oops. struct regmap_field is opaque. It seems that the allocation
is the only way that I could have instance of it.
Regards,
Ivan
^ permalink raw reply
* Re: [PATCH 0/2] Qualcomm PM8941 power key driver
From: Ivan T. Ivanov @ 2014-10-08 9:50 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Grant Likely, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
linux-arm-msm@vger.kernel.org
In-Reply-To: <20141007184640.GB31125@sonymobile.com>
On Tue, 2014-10-07 at 11:46 -0700, Bjorn Andersson wrote:
> On Tue 07 Oct 02:01 PDT 2014, Ivan T. Ivanov wrote:
>
> >
> > Hi Bjorn,
> >
> > On Mon, 2014-10-06 at 18:11 -0700, Bjorn Andersson wrote:
> > > These patches add dt bindings and a device driver for the power key block in
> > > the Qualcomm PM8941 pmic.
> > >
> > > Courtney Cavin (2):
> > > input: Add Qualcomm PM8941 power key driver
> > > input: pm8941-pwrkey: Add DT binding documentation
> > >
> > > .../bindings/input/qcom,pm8941-pwrkey.txt | 43 +++++
> > > drivers/input/misc/Kconfig | 12 ++
> > > drivers/input/misc/Makefile | 1 +
> > > drivers/input/misc/pm8941-pwrkey.c | 196 ++++++++++++++++++++
> > > 4 files changed, 252 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > > create mode 100644 drivers/input/misc/pm8941-pwrkey.c
> >
> > Any reason why we cannot reuse pm8xxx-pwrkey driver? It have been
> > converted to regmap already.
> >
>
> The boilerplate code is the same,
The boilerplate code is almost 100% :-)
> but configuration registers have different
> layout and values written in them are different.
We talk about 3 registers and 2 bit defines. struct regmap_field
should be able to help here.
> The pm8xxx block have separate
> interrupts for press and release events while pm8941 have one interrupt for
> both, so the pm8941 must read out the irq status bits to figure out which event
> it was.
Optional interrupt property? If both are defined hook old ISR, if its
only one hook pm8941 ISR?
>
> Maybe if we introduce some vagueness related to interrupts in the dt binding
> documentation for pm8xxx we could simply reuse that binding.
>
I would not say vagueness, we just can say that pm8941 did not have
second interrupt?
Regards,
Ivan
> Regards,
> Bjorn
^ permalink raw reply
* Re: [PATCH] Input: adp5588-keys: cancel workqueue in failure path
From: Pramod Gurav @ 2014-10-08 10:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pramod Gurav, linux-kernel@vger.kernel.org, Michael Hennerich,
linux-input
In-Reply-To: <20141007163856.GE16469@dtor-ws>
Hi Dmitry,
On Tue, Oct 7, 2014 at 10:08 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Oct 07, 2014 at 01:00:49PM +0530, Pramod Gurav wrote:
>> This change introduces a label to call cancel_delayed_work_sync in
>> failure path.
>>
>> Cc: Michael Hennerich <michael.hennerich@analog.com>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: linux-input@vger.kernel.org
>> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>> ---
>> drivers/input/keyboard/adp5588-keys.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
>> index 5ef7fcf..b494062 100644
>> --- a/drivers/input/keyboard/adp5588-keys.c
>> +++ b/drivers/input/keyboard/adp5588-keys.c
>> @@ -559,7 +559,7 @@ static int adp5588_probe(struct i2c_client *client,
>> error = input_register_device(input);
>> if (error) {
>> dev_err(&client->dev, "unable to register input device\n");
>> - goto err_free_mem;
>> + goto err_delayed_work;
>> }
>>
>> error = request_irq(client->irq, adp5588_irq,
>> @@ -592,6 +592,8 @@ static int adp5588_probe(struct i2c_client *client,
>> err_unreg_dev:
>> input_unregister_device(input);
>> input = NULL;
>> + err_delayed_work:
>> + cancel_delayed_work_sync(&kpad->work);
>
> We do not need to have a separate label, just need to cancel workqueue
> after freeing interrupt. I adjusted the patch and applied.
>
Thanks. I saw the change in your tree. Shouldn't
cancel_delayed_work_sync(&kpad->work) be called under label
err_free_mem so that it will executed in case adp5588_read,
input_register_device, request_irq etc.
Or else we will still have workqueue instance hanging around.
> Thanks.
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Thanks and Regards
Pramod
^ permalink raw reply
* Re: [PATCH] Input: adp5588-keys: cancel workqueue in failure path
From: Pramod Gurav @ 2014-10-08 10:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pramod Gurav, linux-kernel@vger.kernel.org, Michael Hennerich,
linux-input
In-Reply-To: <CAMf-jS=jvo2hzULv_MtvbeAq_A2z0aJS2F=1Zx5tPonKsJTrNg@mail.gmail.com>
On Wed, Oct 8, 2014 at 3:48 PM, Pramod Gurav <pramod.gurav.etc@gmail.com> wrote:
> Hi Dmitry,
>
> On Tue, Oct 7, 2014 at 10:08 PM, Dmitry Torokhov
>
..
> Thanks. I saw the change in your tree. Shouldn't
> cancel_delayed_work_sync(&kpad->work) be called under label
> err_free_mem so that it will executed in case adp5588_read,
> input_register_device, request_irq etc.
> Or else we will still have workqueue instance hanging around.
>
Sorry, Should have read Michael's reply more carefully. I get the
reason behind cancelling workquequq after freeing irq.
--
Thanks and Regards
Pramod
^ permalink raw reply
* [PATCH] Input: pxa27x_keypad: Switch to using managed resources
From: Pramod Gurav @ 2014-10-08 10:50 UTC (permalink / raw)
To: linux-kernel; +Cc: Pramod Gurav, Dmitry Torokhov, linux-input
This change switches to using devm_* APIs to allocate resources.
This helps to simplify failure path in probe function and module
unloading and does away with remove function.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
drivers/input/keyboard/pxa27x_keypad.c | 69 +++++++++-----------------------
1 file changed, 18 insertions(+), 51 deletions(-)
diff --git a/drivers/input/keyboard/pxa27x_keypad.c b/drivers/input/keyboard/pxa27x_keypad.c
index a15063b..719e44e 100644
--- a/drivers/input/keyboard/pxa27x_keypad.c
+++ b/drivers/input/keyboard/pxa27x_keypad.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/interrupt.h>
+#include <linux/io.h>
#include <linux/input.h>
#include <linux/device.h>
#include <linux/platform_device.h>
@@ -741,37 +742,36 @@ static int pxa27x_keypad_probe(struct platform_device *pdev)
return -ENXIO;
}
- keypad = kzalloc(sizeof(struct pxa27x_keypad), GFP_KERNEL);
- input_dev = input_allocate_device();
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad),
+ GFP_KERNEL);
+ input_dev = devm_input_allocate_device(&pdev->dev);
if (!keypad || !input_dev) {
dev_err(&pdev->dev, "failed to allocate memory\n");
- error = -ENOMEM;
- goto failed_free;
+ return -ENOMEM;
}
keypad->pdata = pdata;
keypad->input_dev = input_dev;
keypad->irq = irq;
- res = request_mem_region(res->start, resource_size(res), pdev->name);
+ res = devm_request_mem_region(&pdev->dev, res->start,
+ resource_size(res), pdev->name);
if (res == NULL) {
dev_err(&pdev->dev, "failed to request I/O memory\n");
- error = -EBUSY;
- goto failed_free;
+ return -EBUSY;
}
- keypad->mmio_base = ioremap(res->start, resource_size(res));
+ keypad->mmio_base = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
if (keypad->mmio_base == NULL) {
dev_err(&pdev->dev, "failed to remap I/O memory\n");
- error = -ENXIO;
- goto failed_free_mem;
+ return -ENXIO;
}
- keypad->clk = clk_get(&pdev->dev, NULL);
+ keypad->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(keypad->clk)) {
dev_err(&pdev->dev, "failed to get keypad clock\n");
- error = PTR_ERR(keypad->clk);
- goto failed_free_io;
+ return PTR_ERR(keypad->clk);
}
input_dev->name = pdev->name;
@@ -802,7 +802,7 @@ static int pxa27x_keypad_probe(struct platform_device *pdev)
}
if (error) {
dev_err(&pdev->dev, "failed to build keycode\n");
- goto failed_put_clk;
+ return error;
}
keypad->row_shift = get_count_order(pdata->matrix_key_cols);
@@ -812,56 +812,24 @@ static int pxa27x_keypad_probe(struct platform_device *pdev)
input_dev->evbit[0] |= BIT_MASK(EV_REL);
}
- error = request_irq(irq, pxa27x_keypad_irq_handler, 0,
- pdev->name, keypad);
+ error = devm_request_irq(&pdev->dev, irq, pxa27x_keypad_irq_handler,
+ 0, pdev->name, keypad);
if (error) {
dev_err(&pdev->dev, "failed to request IRQ\n");
- goto failed_put_clk;
+ return error;
}
/* Register the input device */
error = input_register_device(input_dev);
if (error) {
dev_err(&pdev->dev, "failed to register input device\n");
- goto failed_free_irq;
+ return error;
}
platform_set_drvdata(pdev, keypad);
device_init_wakeup(&pdev->dev, 1);
return 0;
-
-failed_free_irq:
- free_irq(irq, keypad);
-failed_put_clk:
- clk_put(keypad->clk);
-failed_free_io:
- iounmap(keypad->mmio_base);
-failed_free_mem:
- release_mem_region(res->start, resource_size(res));
-failed_free:
- input_free_device(input_dev);
- kfree(keypad);
- return error;
-}
-
-static int pxa27x_keypad_remove(struct platform_device *pdev)
-{
- struct pxa27x_keypad *keypad = platform_get_drvdata(pdev);
- struct resource *res;
-
- free_irq(keypad->irq, keypad);
- clk_put(keypad->clk);
-
- input_unregister_device(keypad->input_dev);
- iounmap(keypad->mmio_base);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
- kfree(keypad);
-
- return 0;
}
/* work with hotplug and coldplug */
@@ -877,7 +845,6 @@ MODULE_DEVICE_TABLE(of, pxa27x_keypad_dt_match);
static struct platform_driver pxa27x_keypad_driver = {
.probe = pxa27x_keypad_probe,
- .remove = pxa27x_keypad_remove,
.driver = {
.name = "pxa27x-keypad",
.of_match_table = of_match_ptr(pxa27x_keypad_dt_match),
--
1.7.9.5
^ permalink raw reply related
* [PATCH] Input: nomadik-ske-keypad: Switch to using managed resources
From: Pramod Gurav @ 2014-10-08 10:50 UTC (permalink / raw)
To: linux-kernel; +Cc: Pramod Gurav, Dmitry Torokhov, linux-input
This change switches to using devm_* APIs to allocate resources.
This helps to simplify failure path in probe function as well as
remove function.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
drivers/input/keyboard/nomadik-ske-keypad.c | 63 +++++++++------------------
1 file changed, 20 insertions(+), 43 deletions(-)
diff --git a/drivers/input/keyboard/nomadik-ske-keypad.c b/drivers/input/keyboard/nomadik-ske-keypad.c
index 63332e2..95ac317 100644
--- a/drivers/input/keyboard/nomadik-ske-keypad.c
+++ b/drivers/input/keyboard/nomadik-ske-keypad.c
@@ -247,12 +247,11 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
return -EINVAL;
}
- keypad = kzalloc(sizeof(struct ske_keypad), GFP_KERNEL);
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
input = input_allocate_device();
if (!keypad || !input) {
dev_err(&pdev->dev, "failed to allocate keypad memory\n");
- error = -ENOMEM;
- goto err_free_mem;
+ return -ENOMEM;
}
keypad->irq = irq;
@@ -260,31 +259,29 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
keypad->input = input;
spin_lock_init(&keypad->ske_keypad_lock);
- if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
+ if (!devm_request_mem_region(&pdev->dev, res->start,
+ resource_size(res), pdev->name)) {
dev_err(&pdev->dev, "failed to request I/O memory\n");
- error = -EBUSY;
- goto err_free_mem;
+ return -EBUSY;
}
- keypad->reg_base = ioremap(res->start, resource_size(res));
+ keypad->reg_base = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
if (!keypad->reg_base) {
dev_err(&pdev->dev, "failed to remap I/O memory\n");
- error = -ENXIO;
- goto err_free_mem_region;
+ return -ENXIO;
}
- keypad->pclk = clk_get(&pdev->dev, "apb_pclk");
+ keypad->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
if (IS_ERR(keypad->pclk)) {
dev_err(&pdev->dev, "failed to get pclk\n");
- error = PTR_ERR(keypad->pclk);
- goto err_iounmap;
+ return PTR_ERR(keypad->pclk);
}
- keypad->clk = clk_get(&pdev->dev, NULL);
+ keypad->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(keypad->clk)) {
dev_err(&pdev->dev, "failed to get clk\n");
- error = PTR_ERR(keypad->clk);
- goto err_pclk;
+ return PTR_ERR(keypad->clk);
}
input->id.bustype = BUS_HOST;
@@ -296,7 +293,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
keypad->keymap, input);
if (error) {
dev_err(&pdev->dev, "Failed to build keymap\n");
- goto err_clk;
+ return error;
}
input_set_capability(input, EV_MSC, MSC_SCAN);
@@ -306,7 +303,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
error = clk_prepare_enable(keypad->pclk);
if (error) {
dev_err(&pdev->dev, "Failed to prepare/enable pclk\n");
- goto err_clk;
+ return error;
}
error = clk_prepare_enable(keypad->clk);
@@ -326,8 +323,9 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
goto err_clk_disable;
}
- error = request_threaded_irq(keypad->irq, NULL, ske_keypad_irq,
- IRQF_ONESHOT, "ske-keypad", keypad);
+ error = devm_request_threaded_irq(&pdev->dev, keypad->irq, NULL,
+ ske_keypad_irq, IRQF_ONESHOT,
+ "ske-keypad", keypad);
if (error) {
dev_err(&pdev->dev, "allocate irq %d failed\n", keypad->irq);
goto err_clk_disable;
@@ -337,7 +335,7 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
if (error) {
dev_err(&pdev->dev,
"unable to register input device: %d\n", error);
- goto err_free_irq;
+ goto err_clk_disable;
}
if (plat->wakeup_enable)
@@ -347,45 +345,24 @@ static int __init ske_keypad_probe(struct platform_device *pdev)
return 0;
-err_free_irq:
- free_irq(keypad->irq, keypad);
err_clk_disable:
clk_disable_unprepare(keypad->clk);
err_pclk_disable:
clk_disable_unprepare(keypad->pclk);
-err_clk:
- clk_put(keypad->clk);
-err_pclk:
- clk_put(keypad->pclk);
-err_iounmap:
- iounmap(keypad->reg_base);
-err_free_mem_region:
- release_mem_region(res->start, resource_size(res));
-err_free_mem:
- input_free_device(input);
- kfree(keypad);
+
return error;
}
static int ske_keypad_remove(struct platform_device *pdev)
{
struct ske_keypad *keypad = platform_get_drvdata(pdev);
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
- free_irq(keypad->irq, keypad);
-
- input_unregister_device(keypad->input);
clk_disable_unprepare(keypad->clk);
- clk_put(keypad->clk);
+ clk_disable_unprepare(keypad->pclk);
if (keypad->board->exit)
keypad->board->exit();
- iounmap(keypad->reg_base);
- release_mem_region(res->start, resource_size(res));
- kfree(keypad);
-
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/2] input: remove use of gpiochip_remove() retval
From: Linus Walleij @ 2014-10-08 12:31 UTC (permalink / raw)
To: Pramod Gurav
Cc: linux-kernel@vger.kernel.org, Alexandre Courbot, Dmitry Torokhov,
Linux Input, linux-gpio@vger.kernel.org
In-Reply-To: <1412087693-8277-1-git-send-email-pramod.gurav@smartplayin.com>
On Tue, Sep 30, 2014 at 4:34 PM, Pramod Gurav
<pramod.gurav@smartplayin.com> wrote:
> Get rid of using return value from gpiochip_remove() as it returns
> void.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: linux-gpio@vger.kernel.org
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
A similar patch is already ACKed by Dmitry and merged in the GPIO tree.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] Input: Add Microchip AR1021 i2c touchscreen
From: Christian Gmeiner @ 2014-10-08 14:45 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, Christian Gmeiner
This patch adds support for the ar1021 i2c based touchscreen.
The driver is quite simple and only supports the Touch
Reporting Protocol.
This is the final version for an RFC patch send a while ago.
http://www.spinics.net/lists/linux-input/msg29419.html
Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
drivers/input/touchscreen/Kconfig | 12 +++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/ar1021_i2c.c | 183 +++++++++++++++++++++++++++++++++
3 files changed, 196 insertions(+)
create mode 100644 drivers/input/touchscreen/ar1021_i2c.c
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 6bb9a7d..e1d8003 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -90,6 +90,18 @@ config TOUCHSCREEN_AD7879_SPI
To compile this driver as a module, choose M here: the
module will be called ad7879-spi.
+config TOUCHSCREEN_AR1021_I2C
+ tristate "Microchip AR1021 i2c touchscreen"
+ depends on I2C && OF
+ help
+ Say Y here if you have the Microchip AR1021 touchscreen controller
+ chip in your system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ar1021_i2c.
+
config TOUCHSCREEN_ATMEL_MXT
tristate "Atmel mXT I2C Touchscreen"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 4be94fc..090e61c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_TOUCHSCREEN_AD7879) += ad7879.o
obj-$(CONFIG_TOUCHSCREEN_AD7879_I2C) += ad7879-i2c.o
obj-$(CONFIG_TOUCHSCREEN_AD7879_SPI) += ad7879-spi.o
obj-$(CONFIG_TOUCHSCREEN_ADS7846) += ads7846.o
+obj-$(CONFIG_TOUCHSCREEN_AR1021_I2C) += ar1021_i2c.o
obj-$(CONFIG_TOUCHSCREEN_ATMEL_MXT) += atmel_mxt_ts.o
obj-$(CONFIG_TOUCHSCREEN_AUO_PIXCIR) += auo-pixcir-ts.o
obj-$(CONFIG_TOUCHSCREEN_BU21013) += bu21013_ts.o
diff --git a/drivers/input/touchscreen/ar1021_i2c.c b/drivers/input/touchscreen/ar1021_i2c.c
new file mode 100644
index 0000000..566014a
--- /dev/null
+++ b/drivers/input/touchscreen/ar1021_i2c.c
@@ -0,0 +1,183 @@
+/*
+ * Microchip AR1021 driver for I2C
+ *
+ * Author: Christian Gmeiner <christian.gmeiner@gmail.com>
+ *
+ * License: GPLv2 as published by the FSF.
+ */
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+
+#define AR1021_TOCUH_PKG_SIZE 5
+
+const int MAX_X = 4095;
+const int MAX_Y = 4095;
+
+struct ar1021_i2c {
+ struct i2c_client *client;
+ struct input_dev *input;
+ u8 data[AR1021_TOCUH_PKG_SIZE];
+};
+
+static irqreturn_t ar1021_i2c_irq(int irq, void *dev_id)
+{
+ struct ar1021_i2c *ar1021 = dev_id;
+ struct input_dev *input = ar1021->input;
+ u8 *data = ar1021->data;
+ unsigned int x, y, button;
+ int error;
+
+ error = i2c_master_recv(ar1021->client,
+ ar1021->data, sizeof(ar1021->data));
+ if (error < 0)
+ goto out;
+
+ /* sync bit set ? */
+ if ((data[0] & 0x80) == 0)
+ goto out;
+
+ button = data[0] & BIT(0);
+ x = ((data[2] & 0x1f) << 7) | (data[1] & 0x7f);
+ y = ((data[4] & 0x1f) << 7) | (data[3] & 0x7f);
+
+ input_report_abs(input, ABS_X, x);
+ input_report_abs(input, ABS_Y, y);
+ input_report_key(input, BTN_TOUCH, button);
+ input_sync(input);
+
+out:
+ return IRQ_HANDLED;
+}
+
+static int ar1021_i2c_open(struct input_dev *dev)
+{
+ struct ar1021_i2c *ar1021 = input_get_drvdata(dev);
+ struct i2c_client *client = ar1021->client;
+
+ enable_irq(client->irq);
+
+ return 0;
+}
+
+static void ar1021_i2c_close(struct input_dev *dev)
+{
+ struct ar1021_i2c *ar1021 = input_get_drvdata(dev);
+ struct i2c_client *client = ar1021->client;
+
+ disable_irq(client->irq);
+}
+
+static int ar1021_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct ar1021_i2c *ar1021;
+ struct input_dev *input;
+ int error;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(&client->dev, "i2c_check_functionality error\n");
+ return -EIO;
+ }
+
+ ar1021 = devm_kzalloc(&client->dev, sizeof(*ar1021), GFP_KERNEL);
+
+ input = devm_input_allocate_device(&client->dev);
+ if (!ar1021 || !input)
+ return -ENOMEM;
+
+ ar1021->client = client;
+ ar1021->input = input;
+
+ input->name = "ar1021 I2C Touchscreen";
+ input->id.bustype = BUS_I2C;
+ input->dev.parent = &client->dev;
+ input->open = ar1021_i2c_open;
+ input->close = ar1021_i2c_close;
+
+ input->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+
+ __set_bit(BTN_TOUCH, input->keybit);
+
+ input_set_abs_params(input, ABS_X, 0, MAX_X, 0, 0);
+ input_set_abs_params(input, ABS_Y, 0, MAX_Y, 0, 0);
+
+ input_set_drvdata(input, ar1021);
+
+ error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ ar1021_i2c_irq, IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "ar1021_i2c", ar1021);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to enable IRQ, error: %d\n", error);
+ return error;
+ }
+
+ /* Disable the IRQ, we'll enable it in ar1021_i2c_open() */
+ disable_irq(client->irq);
+
+ error = input_register_device(ar1021->input);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to register input device, error: %d\n", error);
+ return error;
+ }
+
+ i2c_set_clientdata(client, ar1021);
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ar1021_i2c_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ disable_irq(client->irq);
+
+ return 0;
+}
+
+static int ar1021_i2c_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ enable_irq(client->irq);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(ar1021_i2c_pm, ar1021_i2c_suspend, ar1021_i2c_resume);
+
+static const struct i2c_device_id ar1021_i2c_id[] = {
+ { "MICROCHIP_AR1021_I2C", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, ar1021_i2c_id);
+
+static struct of_device_id ar1021_i2c_of_match[] = {
+ { .compatible = "microchip,ar1021-i2c", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ar1021_i2c_of_match);
+
+static struct i2c_driver ar1021_i2c_driver = {
+ .driver = {
+ .name = "ar1021_i2c",
+ .owner = THIS_MODULE,
+ .pm = &ar1021_i2c_pm,
+ .of_match_table = ar1021_i2c_of_match,
+ },
+
+ .probe = ar1021_i2c_probe,
+ .id_table = ar1021_i2c_id,
+};
+module_i2c_driver(ar1021_i2c_driver);
+
+MODULE_AUTHOR("Christian Gmeiner <christian.gmeiner@gmail.com>");
+MODULE_DESCRIPTION("Microchip AR1021 I2C Driver");
+MODULE_LICENSE("GPL");
--
1.9.3
^ permalink raw reply related
* Re: How to indicate hover touch when exact distance unknown?
From: Henrik Rydberg @ 2014-10-08 15:00 UTC (permalink / raw)
To: Dmitry Torokhov, Benjamin Tissoires
Cc: Peter Hutterer, Andrew de los Reyes, Hans de Goede, jikos,
David Herrmann, Linux Input
In-Reply-To: <20141001162252.GA23115@dtor-glaptop>
Hi Dmitry,
> OK, so should we then say that we will continue reporting binary hovering
> with ABS_MT_DISTANCE 0/1 and such devices must report resolution of 0;
> devices reporting wider ABS_MT_DISTANCE ranges should also report
> appropriate resolution, same as X/Y coordinates?
Yes, I think this makes sense, after all.
Thanks,
Henrik
^ permalink raw reply
* [PATCH] Input: automatically set EV_ABS bit in input_set_abs_params
From: Dmitry Torokhov @ 2014-10-08 16:54 UTC (permalink / raw)
To: linux-input
Cc: Henrik Rydberg, Hans de Goede, David Herrmann, Benjamin Tissoires,
jikos, linux-kernel
Let's automatically set EV_ABS bit in device's event type list when calling
input_set_abs_params() so that drivers do not have to do it explicitly.
These calls are never in a hot paths so we won't lose much time by setting
the same bit several times.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/input.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 29ca0bb..d2e06cc 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -498,7 +498,8 @@ void input_set_abs_params(struct input_dev *dev, unsigned int axis,
absinfo->fuzz = fuzz;
absinfo->flat = flat;
- dev->absbit[BIT_WORD(axis)] |= BIT_MASK(axis);
+ __set_bit(EV_ABS, dev->evbit);
+ __set_bit(axis, dev->absbit);
}
EXPORT_SYMBOL(input_set_abs_params);
--
Dmitry
^ permalink raw reply related
* Re: [PATCH] Input: automatically set EV_ABS bit in input_set_abs_params
From: David Herrmann @ 2014-10-08 17:09 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: open list:HID CORE LAYER, Henrik Rydberg, Hans de Goede,
Benjamin Tissoires, jikos, linux-kernel
In-Reply-To: <20141008165425.GA23281@dtor-ws>
Hi
On Wed, Oct 8, 2014 at 6:54 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Let's automatically set EV_ABS bit in device's event type list when calling
> input_set_abs_params() so that drivers do not have to do it explicitly.
>
> These calls are never in a hot paths so we won't lose much time by setting
> the same bit several times.
Looks good to me. I mean we already do the absbit-allocation here, so
this looks sane to do.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/input.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 29ca0bb..d2e06cc 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -498,7 +498,8 @@ void input_set_abs_params(struct input_dev *dev, unsigned int axis,
> absinfo->fuzz = fuzz;
> absinfo->flat = flat;
>
> - dev->absbit[BIT_WORD(axis)] |= BIT_MASK(axis);
> + __set_bit(EV_ABS, dev->evbit);
> + __set_bit(axis, dev->absbit);
> }
> EXPORT_SYMBOL(input_set_abs_params);
>
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH] Input: Add Microchip AR1021 i2c touchscreen
From: Dmitry Torokhov @ 2014-10-08 17:56 UTC (permalink / raw)
To: Christian Gmeiner; +Cc: linux-input
In-Reply-To: <1412779518-16712-1-git-send-email-christian.gmeiner@gmail.com>
Hi Christian,
On Wed, Oct 08, 2014 at 04:45:18PM +0200, Christian Gmeiner wrote:
> +static irqreturn_t ar1021_i2c_irq(int irq, void *dev_id)
> +{
> + struct ar1021_i2c *ar1021 = dev_id;
> + struct input_dev *input = ar1021->input;
> + u8 *data = ar1021->data;
> + unsigned int x, y, button;
> + int error;
> +
> + error = i2c_master_recv(ar1021->client,
> + ar1021->data, sizeof(ar1021->data));
> + if (error < 0)
I think this check should be "if (retval != sizeof(ar1021->data))" to
avoid using garbage data in case of short read. I'll change it locally
and apply the patch.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
From: Stephen Boyd @ 2014-10-08 18:00 UTC (permalink / raw)
To: Ivan T. Ivanov; +Cc: Dmitry Torokhov, linux-input, linux-kernel, linux-arm-msm
In-Reply-To: <1412760633.13569.4.camel@iivanov-dev>
On 10/08/2014 02:30 AM, Ivan T. Ivanov wrote:
> On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote:
>> On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote:
>>> Hi Ivan,
>>>
>>> On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote:
>>>> @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
>>>>
>>>> +
>>>> + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap,
>>>> + info->row_hold);
>>>> + if (IS_ERR(kp->row_hold))
>>>> + return PTR_ERR(kp->row_hold);
>>> Why do we have to allocate all regmap fields separately instead of
>>> embedding them into keypad structure?
>>>
>> No particular reason. Will rework it.
>>
> Oops. struct regmap_field is opaque. It seems that the allocation
> is the only way that I could have instance of it.
>
Maybe we can add an API to allocate an array of fields?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
From: Dmitry Torokhov @ 2014-10-08 18:13 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Ivan T. Ivanov, linux-input, linux-kernel, linux-arm-msm
In-Reply-To: <54357BA7.3010609@codeaurora.org>
On Wed, Oct 08, 2014 at 11:00:07AM -0700, Stephen Boyd wrote:
> On 10/08/2014 02:30 AM, Ivan T. Ivanov wrote:
> >On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote:
> >>On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote:
> >>>Hi Ivan,
> >>>
> >>>On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote:
> >>>>@@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
> >>>>+
> >>>>+ kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap,
> >>>>+ info->row_hold);
> >>>>+ if (IS_ERR(kp->row_hold))
> >>>>+ return PTR_ERR(kp->row_hold);
> >>>Why do we have to allocate all regmap fields separately instead of
> >>>embedding them into keypad structure?
> >>>
> >>No particular reason. Will rework it.
> >>
> >Oops. struct regmap_field is opaque. It seems that the allocation
> >is the only way that I could have instance of it.
> >
>
> Maybe we can add an API to allocate an array of fields?
Maybe we could make the structure public instead? I do not see any
reason for allocating something separately that has exactly the same
lifetime as owning structure.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: mpr121: Switch to using managed resources
From: Dmitry Torokhov @ 2014-10-08 18:17 UTC (permalink / raw)
To: Pramod Gurav; +Cc: linux-kernel, linux-input
In-Reply-To: <1412750911-16759-1-git-send-email-pramod.gurav@smartplayin.com>
On Wed, Oct 08, 2014 at 12:18:31PM +0530, Pramod Gurav wrote:
> This change switches to using devm_* managed resources APIs to
> request the resources in probe to simplify probe error path and
> module unloading and does away with remove function.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
I'll queue it for 3.19.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
From: Stephen Boyd @ 2014-10-08 18:20 UTC (permalink / raw)
To: Dmitry Torokhov, Srinivas Kandagatla
Cc: Ivan T. Ivanov, linux-input, linux-kernel, linux-arm-msm,
Mark Brown
In-Reply-To: <20141008181314.GC14423@dtor-ws>
Adding Mark and Srini
On 10/08/2014 11:13 AM, Dmitry Torokhov wrote:
> On Wed, Oct 08, 2014 at 11:00:07AM -0700, Stephen Boyd wrote:
>> On 10/08/2014 02:30 AM, Ivan T. Ivanov wrote:
>>> On Wed, 2014-10-08 at 12:13 +0300, Ivan T. Ivanov wrote:
>>>> On Tue, 2014-10-07 at 10:26 -0700, Dmitry Torokhov wrote:
>>>>> Hi Ivan,
>>>>>
>>>>> On Tue, Oct 07, 2014 at 12:50:46PM +0300, Ivan T. Ivanov wrote:
>>>>>> @@ -527,10 +538,55 @@ static int pmic8xxx_kp_probe(struct platform_device *pdev)
>>>>>> +
>>>>>> + kp->row_hold = devm_regmap_field_alloc(kp->dev, kp->regmap,
>>>>>> + info->row_hold);
>>>>>> + if (IS_ERR(kp->row_hold))
>>>>>> + return PTR_ERR(kp->row_hold);
>>>>> Why do we have to allocate all regmap fields separately instead of
>>>>> embedding them into keypad structure?
>>>>>
>>>> No particular reason. Will rework it.
>>>>
>>> Oops. struct regmap_field is opaque. It seems that the allocation
>>> is the only way that I could have instance of it.
>>>
>> Maybe we can add an API to allocate an array of fields?
> Maybe we could make the structure public instead? I do not see any
> reason for allocating something separately that has exactly the same
> lifetime as owning structure.
>
Srini/Mark, any reason why the regmap_field structure is opaque?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH] Input: pxa27x_keypad: Switch to using managed resources
From: Dmitry Torokhov @ 2014-10-08 18:25 UTC (permalink / raw)
To: Pramod Gurav; +Cc: linux-kernel, linux-input
In-Reply-To: <1412765411-6480-1-git-send-email-pramod.gurav@smartplayin.com>
On Wed, Oct 08, 2014 at 04:20:11PM +0530, Pramod Gurav wrote:
> This change switches to using devm_* APIs to allocate resources.
> This helps to simplify failure path in probe function and module
> unloading and does away with remove function.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
I'll queue it for 3.19.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] HID: wacom: Prevent potential null dereference after disconnect
From: Jason Gerecke @ 2014-10-08 18:25 UTC (permalink / raw)
To: jkosina, benjamin.tissoires, pinglinux
Cc: linux-input, linux-kernel, Jason Gerecke
In-Reply-To: <1412704473-6708-1-git-send-email-killertofu@gmail.com>
Repeated connect/disconnect cycles under GNOME can trigger an occasional
OOPS from within e.g. wacom_led_select_store, presumably due to a timing
issue where userspace begins setting a value immediately before the
device disconnects and our shared data is whisked away.
Signed-off-by: Jason Gerecke <killertofu@gmail.com>
---
Changes in v2:
* Added in missing escape character
drivers/hid/wacom_sys.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 8593047..265429b 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -641,6 +641,9 @@ static ssize_t wacom_led_select_store(struct device *dev, int set_id,
unsigned int id;
int err;
+ if (!wacom)
+ return -ENODEV;
+
err = kstrtouint(buf, 10, &id);
if (err)
return err;
@@ -666,6 +669,8 @@ static ssize_t wacom_led##SET_ID##_select_show(struct device *dev, \
{ \
struct hid_device *hdev = container_of(dev, struct hid_device, dev);\
struct wacom *wacom = hid_get_drvdata(hdev); \
+ if (!wacom) \
+ return -ENODEV; \
return scnprintf(buf, PAGE_SIZE, "%d\n", \
wacom->led.select[SET_ID]); \
} \
@@ -702,7 +707,8 @@ static ssize_t wacom_##name##_luminance_store(struct device *dev, \
{ \
struct hid_device *hdev = container_of(dev, struct hid_device, dev);\
struct wacom *wacom = hid_get_drvdata(hdev); \
- \
+ if (!wacom) \
+ return -ENODEV; \
return wacom_luminance_store(wacom, &wacom->led.field, \
buf, count); \
} \
@@ -710,6 +716,8 @@ static ssize_t wacom_##name##_luminance_show(struct device *dev, \
struct device_attribute *attr, char *buf) \
{ \
struct wacom *wacom = dev_get_drvdata(dev); \
+ if (!wacom) \
+ return -ENODEV; \
return scnprintf(buf, PAGE_SIZE, "%d\n", wacom->led.field); \
} \
static DEVICE_ATTR(name##_luminance, DEV_ATTR_RW_PERM, \
@@ -729,6 +737,9 @@ static ssize_t wacom_button_image_store(struct device *dev, int button_id,
unsigned len;
u8 xfer_id;
+ if (!wacom)
+ return -ENODEV;
+
if (hdev->bus == BUS_BLUETOOTH) {
len = 256;
xfer_id = WAC_CMD_ICON_BT_XFER;
--
2.1.2
^ permalink raw reply related
* Re: [PATCH] Input: lpc32xx-keys: Switch to using managed resources
From: Dmitry Torokhov @ 2014-10-08 18:34 UTC (permalink / raw)
To: Pramod Gurav; +Cc: linux-kernel, linux-input
In-Reply-To: <1412750081-11205-1-git-send-email-pramod.gurav@smartplayin.com>
On Wed, Oct 08, 2014 at 12:04:41PM +0530, Pramod Gurav wrote:
> This change switches to using devm_* managed resources APIs to
> request the resources in probe to simplify probe error path and
> module unloading.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
I'll queue it for 3.19.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] input: cros_ec_keyb: Add of match table
From: Dmitry Torokhov @ 2014-10-08 18:39 UTC (permalink / raw)
To: Sjoerd Simons
Cc: Lee Jones, Wolfram Sang, Doug Anderson, linux-arm-kernel,
linux-samsung-soc, Javier Martinez Canillas, linux-i2c,
linux-input
In-Reply-To: <1411114093-3505-3-git-send-email-sjoerd.simons@collabora.co.uk>
On Fri, Sep 19, 2014 at 10:08:13AM +0200, Sjoerd Simons wrote:
> To enable the cros_ec_keyb driver to be auto-loaded when build as
> module add an of match table (and export it) to match the modalias
> information passed on to userspace as the Cros EC MFD driver registers
> the MFD subdevices with an of_compatibility string.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Applied, thank you.
> ---
> Changes in v2: Fixed some indentation issues
>
> drivers/input/keyboard/cros_ec_keyb.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 791781a..e1903ec 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -342,10 +342,19 @@ static int cros_ec_keyb_resume(struct device *dev)
>
> static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id cros_ec_keyb_of_match[] = {
> + { .compatible = "google,cros-ec-keyb" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match);
> +#endif
> +
> static struct platform_driver cros_ec_keyb_driver = {
> .probe = cros_ec_keyb_probe,
> .driver = {
> .name = "cros-ec-keyb",
> + .of_match_table = of_match_ptr(cros_ec_keyb_of_match),
> .pm = &cros_ec_keyb_pm_ops,
> },
> };
> --
> 2.1.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
From: Mark Brown @ 2014-10-08 20:04 UTC (permalink / raw)
To: Stephen Boyd
Cc: Dmitry Torokhov, Srinivas Kandagatla, Ivan T. Ivanov, linux-input,
linux-kernel, linux-arm-msm
In-Reply-To: <5435808A.7070203@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 786 bytes --]
On Wed, Oct 08, 2014 at 11:20:58AM -0700, Stephen Boyd wrote:
> On 10/08/2014 11:13 AM, Dmitry Torokhov wrote:
> >>>Oops. struct regmap_field is opaque. It seems that the allocation
> >>>is the only way that I could have instance of it.
> >>Maybe we can add an API to allocate an array of fields?
> >Maybe we could make the structure public instead? I do not see any
> >reason for allocating something separately that has exactly the same
> >lifetime as owning structure.
The lifetime may be different to that of the register map it references,
consider MFD function devices for example.
> Srini/Mark, any reason why the regmap_field structure is opaque?
So you can't peer into it and rely on the contents. I can see it being
useful to add a bulk allocator.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] Input: pmic8xxx-keypad - use regmap_field for register access
From: Dmitry Torokhov @ 2014-10-08 20:32 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Boyd, Srinivas Kandagatla, Ivan T. Ivanov, linux-input,
linux-kernel, linux-arm-msm
In-Reply-To: <20141008200426.GJ4609@sirena.org.uk>
On Wed, Oct 08, 2014 at 09:04:26PM +0100, Mark Brown wrote:
> On Wed, Oct 08, 2014 at 11:20:58AM -0700, Stephen Boyd wrote:
> > On 10/08/2014 11:13 AM, Dmitry Torokhov wrote:
>
> > >>>Oops. struct regmap_field is opaque. It seems that the allocation
> > >>>is the only way that I could have instance of it.
>
> > >>Maybe we can add an API to allocate an array of fields?
>
> > >Maybe we could make the structure public instead? I do not see any
> > >reason for allocating something separately that has exactly the same
> > >lifetime as owning structure.
>
> The lifetime may be different to that of the register map it references,
> consider MFD function devices for example.
Exactly, it is different than lifetime of the register map, but the same
as device structure of the driver instantiating it. And so instead of
doing 10+ separate allocations one could do just one.
>
> > Srini/Mark, any reason why the regmap_field structure is opaque?
>
> So you can't peer into it and rely on the contents. I can see it being
> useful to add a bulk allocator.
And then one have to define offsets in an array and use awkward syntax
to access individual fields. Can we just reply on reviews/documentation
for users to not do wrong thing?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] input: avoid negative input device numbers
From: Richard Leitner @ 2014-10-08 20:42 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Fix the format string for input device name generation to avoid negative
device numbers when the id exceeds the maximum signed integer value.
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/input/input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 236bc56..6be6982 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1791,7 +1791,7 @@ struct input_dev *input_allocate_device(void)
INIT_LIST_HEAD(&dev->h_list);
INIT_LIST_HEAD(&dev->node);
- dev_set_name(&dev->dev, "input%ld",
+ dev_set_name(&dev->dev, "input%lu",
(unsigned long) atomic_inc_return(&input_no) - 1);
__module_get(THIS_MODULE);
--
2.1.1
^ permalink raw reply related
* [RFC] avoid (theoretical) conflicts of input device file names
From: Richard Leitner @ 2014-10-08 20:49 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, richard.leitner
In-Reply-To: <20141008224245.601a1339@frodo>
Hi,
currently I discovered the possibility that device file numbers of the input
subsystem could go negative when the signed int "border" is passed. To fix
this behaviour I sent a patch a few minutes ago.
But as the subject says there is currently the (theoretical) possibility that
the same input device file name is given out twice. This can happen if the
"input_no" variable had an overflow (due to the fact this is at least at 2^32
I call the issue theoretical). If such a case occurs a -EEXISTS is returned at
the creation of the file.
IMHO it would be a good idea to check if the chosen input device file name
is valid at the point it is created (which is currently input_allocate_device).
So you can just increment and check it again until there's a valid number/name
found for it.
I'm pretty new to the input subsystem, so what do you think about it?
Any comments/ideas? Would there be a better place to do such checking?
regards,
richard
^ permalink raw reply
* Re: [PATCH] input: avoid negative input device numbers
From: Dmitry Torokhov @ 2014-10-08 21:25 UTC (permalink / raw)
To: Richard Leitner; +Cc: linux-input, linux-kernel, richard.leitner
In-Reply-To: <20141008224245.601a1339@frodo>
On Wed, Oct 08, 2014 at 10:42:45PM +0200, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Fix the format string for input device name generation to avoid negative
> device numbers when the id exceeds the maximum signed integer value.
Well, it is going to take us a while to get there :)
Applied, thank you.
>
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
> drivers/input/input.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 236bc56..6be6982 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1791,7 +1791,7 @@ struct input_dev *input_allocate_device(void)
> INIT_LIST_HEAD(&dev->h_list);
> INIT_LIST_HEAD(&dev->node);
>
> - dev_set_name(&dev->dev, "input%ld",
> + dev_set_name(&dev->dev, "input%lu",
> (unsigned long) atomic_inc_return(&input_no) - 1);
>
> __module_get(THIS_MODULE);
> --
> 2.1.1
>
--
Dmitry
^ 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