* Re: [PATCH 01/15] Input: synaptics-rmi4 - fix checkpatch.pl, sparse and GCC warnings
From: Dmitry Torokhov @ 2014-02-13 19:12 UTC (permalink / raw)
To: Christopher Heiny; +Cc: Courtney Cavin, linux-input
In-Reply-To: <2278620.abrl2TfnmQ@dtor-d630.eng.vmware.com>
On Thu, Feb 13, 2014 at 11:10:29AM -0800, Dmitry Torokhov wrote:
> On Thursday, February 13, 2014 10:56:25 AM Christopher Heiny wrote:
> > On 02/12/2014 10:36 PM, Dmitry Torokhov wrote:
> > > On Wed, Feb 05, 2014 at 05:36:09PM -0800, Christopher Heiny wrote:
> > >> >On 02/05/2014 05:09 PM, Dmitry Torokhov wrote:
> > >>> > >On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote:
> > >>>>> > >>>On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> > >>>>>>> > >>>> >Cc: Christopher Heiny<cheiny@synaptics.com>
> > >>>>>>> > >>>> >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com>
> > >>>>>>> > >>>> >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com>
> > >>>>>>> > >>>> >---
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> > drivers/input/rmi4/rmi_bus.c | 4 ++--
> > >>>>>>> > >>>> > drivers/input/rmi4/rmi_bus.h | 2 +-
> > >>>>>>> > >>>> > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++-----
> > >>>>>>> > >>>> > drivers/input/rmi4/rmi_f11.c | 4 +++-
> > >>>>>>> > >>>> > 4 files changed, 18 insertions(+), 9 deletions(-)
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> >diff --git a/drivers/input/rmi4/rmi_bus.c
> > >>>>>>> > >>>> >b/drivers/input/rmi4/rmi_bus.c
> > >>>>>>> > >>>> >index 96a76e7..8a939f3 100644
> > >>>>>>> > >>>> >--- a/drivers/input/rmi4/rmi_bus.c
> > >>>>>>> > >>>> >+++ b/drivers/input/rmi4/rmi_bus.c
> > >>>>>>> > >>>> >@@ -37,7 +37,7 @@ static void rmi_release_device(struct
> > >>>>>>> > >>>> >device *dev)
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> > kfree(rmi_dev);
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> > }
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> >-struct device_type rmi_device_type = {
> > >>>>>>> > >>>> >+static struct device_type rmi_device_type = {
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> > .name = "rmi_sensor",
> > >>>>>>> > >>>> > .release = rmi_release_device,
> > >>>>>>> > >>>> >
> > >>>>>>> > >>>> > };
> > >>>>> > >>>
> > >>>>> > >>>This struct is used by diagnostic modules to identify sensor
> > >>>>> > >>>devices, so it cannot be static.
> > >>> > >
> > >>> > >Then we need to declare it somewhere or provide an accessor function.
> > >> >
> > >> >Currently it's in a header not included in the patches. We'll move
> > >> >it to rmi_bus.h.
> > >
> > > Hmm, we do have rmi_is_physical_device() to identify whether it is a
> > > sensor or a function, so I believe we should mark all structures static
> > > to avoid anyone poking at them.
> >
> > I was poking around in the dependent code late last night and came to
> > the same conclusion. I'll send a micropatch later today to get it out
> > of the way.
>
> No need, I untangled relevant bits from the one Courtney sent.
>
> Thanks.
Here it is by the way.
Input: synaptics-rmi4 - make device types module-private
From: Courtney Cavin <courtney.cavin@sonymobile.com>
Nobody outside of RMI bus core should be accessing rmi_device_type,
rmi_function_type or rmi_physical_driver structures so let's limit their
scope.
We provide rmi_is_physical_device() and rmi_is_function_device() to allow
determining whether one is dealing with sensor or function device.
Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_bus.c | 4 ++--
drivers/input/rmi4/rmi_driver.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
index 7efe7ed..6e0454a 100644
--- a/drivers/input/rmi4/rmi_bus.c
+++ b/drivers/input/rmi4/rmi_bus.c
@@ -38,7 +38,7 @@ static void rmi_release_device(struct device *dev)
kfree(rmi_dev);
}
-struct device_type rmi_device_type = {
+static struct device_type rmi_device_type = {
.name = "rmi_sensor",
.release = rmi_release_device,
};
@@ -154,7 +154,7 @@ static void rmi_release_function(struct device *dev)
kfree(fn);
}
-struct device_type rmi_function_type = {
+static struct device_type rmi_function_type = {
.name = "rmi_function",
.release = rmi_release_function,
};
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 788343a..4406a7f 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -946,7 +946,7 @@ err_free_mem:
return retval < 0 ? retval : 0;
}
-struct rmi_driver rmi_physical_driver = {
+static struct rmi_driver rmi_physical_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "rmi_physical",
--
Dmitry
^ permalink raw reply related
* Re: [PATCH 03/11] Input: synaptics-rmi4 - do not update configuration in rmi_f01_probe()
From: Christopher Heiny @ 2014-02-13 19:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-3-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Do not write configuration data in probe(), we have config() for that.
Then we should call config() in rmi_function_probe() to ensure that
any platform data or device tree configuration settings get written
to the device.
Thinking about that, it looks like it's not fatal if the config
write fails in that situation. The device might not function as
intended, but you can hopefully get some use out of it (for
instance, a phone's touchscreen sensitivity might be wacky, but
the user will still be able to dial tech support).
For example:
static int rmi_function_probe(struct device *dev)
{
struct rmi_function *fn = to_rmi_function(dev);
struct rmi_function_handler *handler =
to_rmi_function_handler(dev->driver);
int error;
if (handler->probe) {
error = handler->probe(fn);
if (error)
return error;
}
if (handler->config) {
error = handler->config(fn);
if (error)
dev_warn(dev, "Driver config failed.\n");
}
return 0;
}
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 897d8ac..976aba3 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -303,12 +303,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (pdata->power_management.doze_interval) {
> data->device_control.doze_interval =
> pdata->power_management.doze_interval;
> - error = rmi_write(rmi_dev, data->doze_interval_addr,
> - data->device_control.doze_interval);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to configure F01 doze interval register.\n");
> - return error;
> - }
> } else {
> error = rmi_read(rmi_dev, data->doze_interval_addr,
> &data->device_control.doze_interval);
> @@ -324,12 +318,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (pdata->power_management.wakeup_threshold) {
> data->device_control.wakeup_threshold =
> pdata->power_management.wakeup_threshold;
> - error = rmi_write(rmi_dev, data->wakeup_threshold_addr,
> - data->device_control.wakeup_threshold);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to configure F01 wakeup threshold register.\n");
> - return error;
> - }
> } else {
> error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
> &data->device_control.wakeup_threshold);
> @@ -347,12 +335,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> if (pdata->power_management.doze_holdoff) {
> data->device_control.doze_holdoff =
> pdata->power_management.doze_holdoff;
> - error = rmi_write(rmi_dev, data->doze_holdoff_addr,
> - data->device_control.doze_holdoff);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to configure F01 doze holdoff register.\n");
> - return error;
> - }
> } else {
> error = rmi_read(rmi_dev, data->doze_holdoff_addr,
> &data->device_control.doze_holdoff);
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 01/15] Input: synaptics-rmi4 - fix checkpatch.pl, sparse and GCC warnings
From: Christopher Heiny @ 2014-02-13 19:25 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Courtney Cavin, linux-input
In-Reply-To: <20140213191216.GA3916@core.coreip.homeip.net>
On 02/13/2014 11:12 AM, Dmitry Torokhov wrote:
> On Thu, Feb 13, 2014 at 11:10:29AM -0800, Dmitry Torokhov wrote:
>> On Thursday, February 13, 2014 10:56:25 AM Christopher Heiny wrote:
>>> On 02/12/2014 10:36 PM, Dmitry Torokhov wrote:
>>>> On Wed, Feb 05, 2014 at 05:36:09PM -0800, Christopher Heiny wrote:
>>>>>> On 02/05/2014 05:09 PM, Dmitry Torokhov wrote:
>>>>>>>> On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote:
>>>>>>>>>>>> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
>>>>>>>>>>>>>>>> Cc: Christopher Heiny<cheiny@synaptics.com>
>>>>>>>>>>>>>>>> Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>>>>>>>>>>>>>>> Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com>
>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> drivers/input/rmi4/rmi_bus.c | 4 ++--
>>>>>>>>>>>>>>>> drivers/input/rmi4/rmi_bus.h | 2 +-
>>>>>>>>>>>>>>>> drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++-----
>>>>>>>>>>>>>>>> drivers/input/rmi4/rmi_f11.c | 4 +++-
>>>>>>>>>>>>>>>> 4 files changed, 18 insertions(+), 9 deletions(-)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> diff --git a/drivers/input/rmi4/rmi_bus.c
>>>>>>>>>>>>>>>> b/drivers/input/rmi4/rmi_bus.c
>>>>>>>>>>>>>>>> index 96a76e7..8a939f3 100644
>>>>>>>>>>>>>>>> --- a/drivers/input/rmi4/rmi_bus.c
>>>>>>>>>>>>>>>> +++ b/drivers/input/rmi4/rmi_bus.c
>>>>>>>>>>>>>>>> @@ -37,7 +37,7 @@ static void rmi_release_device(struct
>>>>>>>>>>>>>>>> device *dev)
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> kfree(rmi_dev);
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> -struct device_type rmi_device_type = {
>>>>>>>>>>>>>>>> +static struct device_type rmi_device_type = {
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> .name = "rmi_sensor",
>>>>>>>>>>>>>>>> .release = rmi_release_device,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>
>>>>>>>>>>>> This struct is used by diagnostic modules to identify sensor
>>>>>>>>>>>> devices, so it cannot be static.
>>>>>>>>
>>>>>>>> Then we need to declare it somewhere or provide an accessor function.
>>>>>>
>>>>>> Currently it's in a header not included in the patches. We'll move
>>>>>> it to rmi_bus.h.
>>>>
>>>> Hmm, we do have rmi_is_physical_device() to identify whether it is a
>>>> sensor or a function, so I believe we should mark all structures static
>>>> to avoid anyone poking at them.
>>>
>>> I was poking around in the dependent code late last night and came to
>>> the same conclusion. I'll send a micropatch later today to get it out
>>> of the way.
>>
>> No need, I untangled relevant bits from the one Courtney sent.
Thanks!
>>
>> Thanks.
>
> Here it is by the way.
>
> Input: synaptics-rmi4 - make device types module-private
>
> From: Courtney Cavin <courtney.cavin@sonymobile.com>
>
> Nobody outside of RMI bus core should be accessing rmi_device_type,
> rmi_function_type or rmi_physical_driver structures so let's limit their
> scope.
>
> We provide rmi_is_physical_device() and rmi_is_function_device() to allow
> determining whether one is dealing with sensor or function device.
>
> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_bus.c | 4 ++--
> drivers/input/rmi4/rmi_driver.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> index 7efe7ed..6e0454a 100644
> --- a/drivers/input/rmi4/rmi_bus.c
> +++ b/drivers/input/rmi4/rmi_bus.c
> @@ -38,7 +38,7 @@ static void rmi_release_device(struct device *dev)
> kfree(rmi_dev);
> }
>
> -struct device_type rmi_device_type = {
> +static struct device_type rmi_device_type = {
> .name = "rmi_sensor",
> .release = rmi_release_device,
> };
> @@ -154,7 +154,7 @@ static void rmi_release_function(struct device *dev)
> kfree(fn);
> }
>
> -struct device_type rmi_function_type = {
> +static struct device_type rmi_function_type = {
> .name = "rmi_function",
> .release = rmi_release_function,
> };
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 788343a..4406a7f 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -946,7 +946,7 @@ err_free_mem:
> return retval < 0 ? retval : 0;
> }
>
> -struct rmi_driver rmi_physical_driver = {
> +static struct rmi_driver rmi_physical_driver = {
> .driver = {
> .owner = THIS_MODULE,
> .name = "rmi_physical",
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 04/11] Input: synaptics-rmi4 - fix LTS handling in F01
From: Christopher Heiny @ 2014-02-13 19:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-4-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> From: Christopher Heiny <cheiny@synaptics.com>
>
> Both F01_RMI_Ctrl2 and F01_RMI_Ctrl3 (doze_interval and wakeup_threshold)
> are controlled by the has_adjustable_doze bit.
>
> Signed-off-by: Christopher Heiny<cheiny@synaptics.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Not sure if this need an Ack, but just in case.
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 976aba3..30e0b50 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -328,6 +328,9 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
> }
>
> + if (data->properties.has_lts)
> + ctrl_base_addr++;
> +
> if (data->properties.has_adjustable_doze_holdoff) {
> data->doze_holdoff_addr = ctrl_base_addr;
> ctrl_base_addr++;
> @@ -383,7 +386,7 @@ static int rmi_f01_config(struct rmi_function *fn)
> dev_err(&fn->dev, "Failed to write interrupt enable.\n");
> return retval;
> }
> - if (data->properties.has_lts) {
> + if (data->properties.has_adjustable_doze) {
> retval = rmi_write_block(fn->rmi_dev, data->doze_interval_addr,
> &data->device_control.doze_interval,
> sizeof(u8));
> @@ -391,9 +394,7 @@ static int rmi_f01_config(struct rmi_function *fn)
> dev_err(&fn->dev, "Failed to write doze interval.\n");
> return retval;
> }
> - }
>
> - if (data->properties.has_adjustable_doze) {
> retval = rmi_write_block(fn->rmi_dev,
> data->wakeup_threshold_addr,
> &data->device_control.wakeup_threshold,
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 09/11] Input: synaptics-rmi4 - consolidate memory allocations in F01
From: Christopher Heiny @ 2014-02-13 19:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-9-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Let's allocate interrupt mask together with the main structure and combine
> rmi_f01_alloc_memory, rmi_f01_initialize and rmi_f01_probe into single
> function.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 86 ++++++++++++++++----------------------------
> 1 file changed, 30 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index a2f05bc..0e21004 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -130,37 +130,15 @@ struct f01_data {
> u16 doze_interval_addr;
> u16 wakeup_threshold_addr;
> u16 doze_holdoff_addr;
> - int irq_count;
> - int num_of_irq_regs;
>
> #ifdef CONFIG_PM_SLEEP
> bool suspended;
> bool old_nosleep;
> #endif
> -};
> -
> -static int rmi_f01_alloc_memory(struct rmi_function *fn,
> - int num_of_irq_regs)
> -{
> - struct f01_data *f01;
>
> - f01 = devm_kzalloc(&fn->dev, sizeof(struct f01_data), GFP_KERNEL);
> - if (!f01) {
> - dev_err(&fn->dev, "Failed to allocate fn_01_data.\n");
> - return -ENOMEM;
> - }
> -
> - f01->device_control.interrupt_enable = devm_kzalloc(&fn->dev,
> - sizeof(u8) * (num_of_irq_regs),
> - GFP_KERNEL);
> - if (!f01->device_control.interrupt_enable) {
> - dev_err(&fn->dev, "Failed to allocate interrupt enable.\n");
> - return -ENOMEM;
> - }
> - fn->data = f01;
> -
> - return 0;
> -}
> + unsigned int num_of_irq_regs;
> + u8 interrupt_enable[];
> +};
>
> static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
> u16 query_base_addr,
> @@ -202,16 +180,28 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
> return 0;
> }
>
> -static int rmi_f01_initialize(struct rmi_function *fn)
> +static int rmi_f01_probe(struct rmi_function *fn)
> {
> - u8 temp;
> - int error;
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> - struct f01_data *f01 = fn->data;
> const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + struct f01_data *f01;
> + size_t f01_size;
> + int error;
> u16 ctrl_base_addr = fn->fd.control_base_addr;
> u8 device_status;
> + u8 temp;
> +
> + f01_size = sizeof(struct f01_data) +
> + sizeof(u8) * driver_data->num_of_irq_regs;
> + f01 = devm_kzalloc(&fn->dev, f01_size, GFP_KERNEL);
> + if (!f01) {
> + dev_err(&fn->dev, "Failed to allocate fn01_data.\n");
> + return -ENOMEM;
> + }
> +
> + f01->num_of_irq_regs = driver_data->num_of_irq_regs;
> + f01->device_control.interrupt_enable = f01->interrupt_enable;
>
> /*
> * Set the configured bit and (optionally) other important stuff
> @@ -257,12 +247,11 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return error;
> }
>
> - f01->irq_count = driver_data->irq_count;
> - f01->num_of_irq_regs = driver_data->num_of_irq_regs;
> - ctrl_base_addr += sizeof(u8);
> -
> + /* Advance to interrupt control registers */
> + ctrl_base_addr++;
> f01->interrupt_enable_addr = ctrl_base_addr;
> - error = rmi_read_block(rmi_dev, ctrl_base_addr,
> +
> + error = rmi_read_block(rmi_dev, f01->interrupt_enable_addr,
> f01->device_control.interrupt_enable,
> sizeof(u8) * (f01->num_of_irq_regs));
> if (error) {
> @@ -272,9 +261,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return error;
> }
>
> - ctrl_base_addr += f01->num_of_irq_regs;
> -
> - /* dummy read in order to clear irqs */
> + /* Dummy read in order to clear irqs */
> error = rmi_read(rmi_dev, fn->fd.data_base_addr + 1, &temp);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read Interrupt Status.\n");
> @@ -287,11 +274,13 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> dev_err(&fn->dev, "Failed to read F01 properties.\n");
> return error;
> }
> +
> dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
> - f01->properties.manufacturer_id == 1 ?
> - "Synaptics" : "unknown",
> + f01->properties.manufacturer_id == 1 ? "Synaptics" : "unknown",
> f01->properties.product_id);
>
> + ctrl_base_addr += f01->num_of_irq_regs;
> +
> /* read control register */
> if (f01->properties.has_adjustable_doze) {
> f01->doze_interval_addr = ctrl_base_addr;
> @@ -365,6 +354,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return -EINVAL;
> }
>
> + fn->data = f01;
> +
> return 0;
> }
>
> @@ -424,23 +415,6 @@ static int rmi_f01_config(struct rmi_function *fn)
> return 0;
> }
>
> -static int rmi_f01_probe(struct rmi_function *fn)
> -{
> - struct rmi_driver_data *driver_data =
> - dev_get_drvdata(&fn->rmi_dev->dev);
> - int error;
> -
> - error = rmi_f01_alloc_memory(fn, driver_data->num_of_irq_regs);
> - if (error)
> - return error;
> -
> - error = rmi_f01_initialize(fn);
> - if (error)
> - return error;
> -
> - return 0;
> -}
> -
> #ifdef CONFIG_PM_SLEEP
> static int rmi_f01_suspend(struct device *dev)
> {
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 10/11] Input: synaptics-rmi4 - make accessor for platform data return const pointer
From: Christopher Heiny @ 2014-02-13 20:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-10-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> When accessing platform data of RMI device let's make sure we do not
> accidentally change data that may be shared by returning const pointer.
> Also switch to an inline function instead of macro to ensure type safety.
That's a good idea. We'll want to look at some of our other #define
accessors as well, I think.
Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/rmi4/rmi_bus.h | 7 ++++++-
> drivers/input/rmi4/rmi_driver.c | 8 ++++----
> drivers/input/rmi4/rmi_f01.c | 2 +-
> drivers/input/rmi4/rmi_f11.c | 2 +-
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
> index 8d47f52..02a2af8 100644
> --- a/drivers/input/rmi4/rmi_bus.h
> +++ b/drivers/input/rmi4/rmi_bus.h
> @@ -224,7 +224,12 @@ struct rmi_device {
> };
>
> #define to_rmi_device(d) container_of(d, struct rmi_device, dev)
> -#define to_rmi_platform_data(d) ((d)->xport->dev->platform_data)
> +
> +static inline const struct rmi_device_platform_data *
> +rmi_get_platform_data(struct rmi_device *d)
> +{
> + return dev_get_platdata(d->xport->dev);
> +}
>
> bool rmi_is_physical_device(struct device *dev);
>
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 736cfbd..473efbc 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -598,7 +598,7 @@ static int rmi_initial_reset(struct rmi_device *rmi_dev,
> u16 cmd_addr = pdt->page_start + pdt->command_base_addr;
> u8 cmd_buf = RMI_DEVICE_RESET_CMD;
> const struct rmi_device_platform_data *pdata =
> - to_rmi_platform_data(rmi_dev);
> + rmi_get_platform_data(rmi_dev);
>
> error = rmi_write_block(rmi_dev, cmd_addr, &cmd_buf, 1);
> if (error) {
> @@ -621,7 +621,7 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> {
> struct device *dev = &rmi_dev->dev;
> struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> - struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> int *current_irq_count = ctx;
> struct rmi_function *fn;
> int i;
> @@ -745,7 +745,7 @@ static int rmi_driver_remove(struct device *dev)
> struct rmi_device *rmi_dev = to_rmi_device(dev);
> struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> const struct rmi_device_platform_data *pdata =
> - to_rmi_platform_data(rmi_dev);
> + rmi_get_platform_data(rmi_dev);
>
> disable_sensor(rmi_dev);
> rmi_free_function_list(rmi_dev);
> @@ -781,7 +781,7 @@ static int rmi_driver_probe(struct device *dev)
> rmi_driver = to_rmi_driver(dev->driver);
> rmi_dev->driver = rmi_driver;
>
> - pdata = to_rmi_platform_data(rmi_dev);
> + pdata = rmi_get_platform_data(rmi_dev);
>
> data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
> if (!data) {
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 0e21004..36fcf8d 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -184,7 +184,7 @@ static int rmi_f01_probe(struct rmi_function *fn)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> - const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> struct f01_data *f01;
> size_t f01_size;
> int error;
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 2aa7d17..5072437 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1176,7 +1176,7 @@ static int rmi_f11_initialize(struct rmi_function *fn)
> u16 control_base_addr;
> u16 max_x_pos, max_y_pos, temp;
> int rc;
> - struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> struct f11_2d_sensor *sensor;
> u8 buf;
>
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 06/11] Input: synaptics-rmi4 - remove device_status form f01_data
From: Christopher Heiny @ 2014-02-13 21:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-6-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> We do not need to persist it - we read it when signalled.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 6f90a6c..1e49ab4 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -126,8 +126,6 @@ struct f01_data {
>
> struct f01_device_control device_control;
>
> - u8 device_status;
> -
> u16 interrupt_enable_addr;
> u16 doze_interval_addr;
> u16 wakeup_threshold_addr;
> @@ -212,6 +210,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> struct f01_data *data = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + u8 device_status;
>
> /*
> * Set the configured bit and (optionally) other important stuff
> @@ -346,16 +345,16 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
>
> error = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &data->device_status, sizeof(data->device_status));
> + &device_status, sizeof(device_status));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read device status.\n");
> return error;
> }
>
> - if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
> + if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
> dev_err(&fn->dev,
> "Device was reset during configuration process, status: %#02x!\n",
> - RMI_F01_STATUS_CODE(data->device_status));
> + RMI_F01_STATUS_CODE(device_status));
> return -EINVAL;
> }
>
> @@ -497,18 +496,18 @@ static int rmi_f01_attention(struct rmi_function *fn,
> unsigned long *irq_bits)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *data = fn->data;
> int retval;
> + u8 device_status;
>
> retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &data->device_status, sizeof(data->device_status));
> + &device_status, sizeof(device_status));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to read device status, code: %d.\n",
> retval);
> return retval;
> }
>
> - if (RMI_F01_STATUS_UNCONFIGURED(data->device_status)) {
> + if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
> dev_warn(&fn->dev, "Device reset detected.\n");
> retval = rmi_dev->driver->reset_handler(rmi_dev);
> if (retval < 0)
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 07/11] Input: synaptics-rmi4 - rename instances of f01_data from data to f01
From: Christopher Heiny @ 2014-02-13 21:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-7-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> We have too many "data"s: f01_data, driver_data, pdata, etc. Let's
> untangle it a bit.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 135 ++++++++++++++++++++++---------------------
> 1 file changed, 68 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 1e49ab4..1219e0c 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -208,7 +208,7 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> u16 ctrl_base_addr;
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> u8 device_status;
>
> @@ -218,8 +218,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> */
> ctrl_base_addr = fn->fd.control_base_addr;
> error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 control.\n");
> return error;
> @@ -228,10 +228,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> case RMI_F01_NOSLEEP_DEFAULT:
> break;
> case RMI_F01_NOSLEEP_OFF:
> - data->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
> break;
> case RMI_F01_NOSLEEP_ON:
> - data->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> break;
> }
>
> @@ -240,38 +240,38 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> * reboot without power cycle. If so, clear it so the sensor
> * is certain to function.
> */
> - if ((data->device_control.ctrl0 & RMI_F01_CTRL0_SLEEP_MODE_MASK) !=
> + if ((f01->device_control.ctrl0 & RMI_F01_CTRL0_SLEEP_MODE_MASK) !=
> RMI_SLEEP_MODE_NORMAL) {
> dev_warn(&fn->dev,
> "WARNING: Non-zero sleep mode found. Clearing...\n");
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> }
>
> - data->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
>
> error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to write F01 control.\n");
> return error;
> }
>
> - data->irq_count = driver_data->irq_count;
> - data->num_of_irq_regs = driver_data->num_of_irq_regs;
> + f01->irq_count = driver_data->irq_count;
> + f01->num_of_irq_regs = driver_data->num_of_irq_regs;
> ctrl_base_addr += sizeof(u8);
>
> - data->interrupt_enable_addr = ctrl_base_addr;
> + f01->interrupt_enable_addr = ctrl_base_addr;
> error = rmi_read_block(rmi_dev, ctrl_base_addr,
> - data->device_control.interrupt_enable,
> - sizeof(u8) * (data->num_of_irq_regs));
> + f01->device_control.interrupt_enable,
> + sizeof(u8) * (f01->num_of_irq_regs));
> if (error < 0) {
> dev_err(&fn->dev,
> "Failed to read F01 control interrupt enable register.\n");
> return error;
> }
>
> - ctrl_base_addr += data->num_of_irq_regs;
> + ctrl_base_addr += f01->num_of_irq_regs;
>
> /* dummy read in order to clear irqs */
> error = rmi_read(rmi_dev, fn->fd.data_base_addr + 1, &temp);
> @@ -281,42 +281,42 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
>
> error = rmi_f01_read_properties(rmi_dev, fn->fd.query_base_addr,
> - &data->properties);
> + &f01->properties);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 properties.\n");
> return error;
> }
> dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
> - data->properties.manufacturer_id == 1 ?
> + f01->properties.manufacturer_id == 1 ?
> "Synaptics" : "unknown",
> - data->properties.product_id);
> + f01->properties.product_id);
>
> /* read control register */
> - if (data->properties.has_adjustable_doze) {
> - data->doze_interval_addr = ctrl_base_addr;
> + if (f01->properties.has_adjustable_doze) {
> + f01->doze_interval_addr = ctrl_base_addr;
> ctrl_base_addr++;
>
> if (pdata->power_management.doze_interval) {
> - data->device_control.doze_interval =
> + f01->device_control.doze_interval =
> pdata->power_management.doze_interval;
> } else {
> - error = rmi_read(rmi_dev, data->doze_interval_addr,
> - &data->device_control.doze_interval);
> + error = rmi_read(rmi_dev, f01->doze_interval_addr,
> + &f01->device_control.doze_interval);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
> return error;
> }
> }
>
> - data->wakeup_threshold_addr = ctrl_base_addr;
> + f01->wakeup_threshold_addr = ctrl_base_addr;
> ctrl_base_addr++;
>
> if (pdata->power_management.wakeup_threshold) {
> - data->device_control.wakeup_threshold =
> + f01->device_control.wakeup_threshold =
> pdata->power_management.wakeup_threshold;
> } else {
> - error = rmi_read(rmi_dev, data->wakeup_threshold_addr,
> - &data->device_control.wakeup_threshold);
> + error = rmi_read(rmi_dev, f01->wakeup_threshold_addr,
> + &f01->device_control.wakeup_threshold);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
> return error;
> @@ -324,19 +324,19 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> }
> }
>
> - if (data->properties.has_lts)
> + if (f01->properties.has_lts)
> ctrl_base_addr++;
>
> - if (data->properties.has_adjustable_doze_holdoff) {
> - data->doze_holdoff_addr = ctrl_base_addr;
> + if (f01->properties.has_adjustable_doze_holdoff) {
> + f01->doze_holdoff_addr = ctrl_base_addr;
> ctrl_base_addr++;
>
> if (pdata->power_management.doze_holdoff) {
> - data->device_control.doze_holdoff =
> + f01->device_control.doze_holdoff =
> pdata->power_management.doze_holdoff;
> } else {
> - error = rmi_read(rmi_dev, data->doze_holdoff_addr,
> - &data->device_control.doze_holdoff);
> + error = rmi_read(rmi_dev, f01->doze_holdoff_addr,
> + &f01->device_control.doze_holdoff);
> if (error < 0) {
> dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
> return error;
> @@ -363,28 +363,28 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>
> static int rmi_f01_config(struct rmi_function *fn)
> {
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> int retval;
>
> retval = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write device_control.reg.\n");
> return retval;
> }
>
> - retval = rmi_write_block(fn->rmi_dev, data->interrupt_enable_addr,
> - data->device_control.interrupt_enable,
> - sizeof(u8) * data->num_of_irq_regs);
> -
> + retval = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
> + f01->device_control.interrupt_enable,
> + sizeof(u8) * f01->num_of_irq_regs);
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write interrupt enable.\n");
> return retval;
> }
> - if (data->properties.has_adjustable_doze) {
> - retval = rmi_write_block(fn->rmi_dev, data->doze_interval_addr,
> - &data->device_control.doze_interval,
> +
> + if (f01->properties.has_adjustable_doze) {
> + retval = rmi_write_block(fn->rmi_dev, f01->doze_interval_addr,
> + &f01->device_control.doze_interval,
> sizeof(u8));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write doze interval.\n");
> @@ -392,8 +392,8 @@ static int rmi_f01_config(struct rmi_function *fn)
> }
>
> retval = rmi_write_block(fn->rmi_dev,
> - data->wakeup_threshold_addr,
> - &data->device_control.wakeup_threshold,
> + f01->wakeup_threshold_addr,
> + &f01->device_control.wakeup_threshold,
> sizeof(u8));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write wakeup threshold.\n");
> @@ -401,15 +401,16 @@ static int rmi_f01_config(struct rmi_function *fn)
> }
> }
>
> - if (data->properties.has_adjustable_doze_holdoff) {
> - retval = rmi_write_block(fn->rmi_dev, data->doze_holdoff_addr,
> - &data->device_control.doze_holdoff,
> + if (f01->properties.has_adjustable_doze_holdoff) {
> + retval = rmi_write_block(fn->rmi_dev, f01->doze_holdoff_addr,
> + &f01->device_control.doze_holdoff,
> sizeof(u8));
> if (retval < 0) {
> dev_err(&fn->dev, "Failed to write doze holdoff.\n");
> return retval;
> }
> }
> +
> return 0;
> }
>
> @@ -435,28 +436,28 @@ static int rmi_f01_suspend(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> int error;
>
> - data->old_nosleep = data->device_control.ctrl0 &
> + f01->old_nosleep = f01->device_control.ctrl0 &
> RMI_F01_CRTL0_NOSLEEP_BIT;
> - data->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
>
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> - data->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
>
> error = rmi_write_block(rmi_dev,
> fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev, "Failed to write sleep mode. Code: %d.\n",
> error);
> - if (data->old_nosleep)
> - data->device_control.ctrl0 |=
> + if (f01->old_nosleep)
> + f01->device_control.ctrl0 |=
> RMI_F01_CRTL0_NOSLEEP_BIT;
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> - data->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> return error;
> }
>
> @@ -467,18 +468,18 @@ static int rmi_f01_resume(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *data = fn->data;
> + struct f01_data *f01 = fn->data;
> int error;
>
> - if (data->old_nosleep)
> - data->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> + if (f01->old_nosleep)
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
>
> - data->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> - data->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> + f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> + f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
>
> error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &data->device_control.ctrl0,
> - sizeof(data->device_control.ctrl0));
> + &f01->device_control.ctrl0,
> + sizeof(f01->device_control.ctrl0));
> if (error < 0) {
> dev_err(&fn->dev,
> "Failed to restore normal operation. Code: %d.\n",
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 08/11] Input: synaptics-rmi4 - use rmi_read/rmi_write in F01
From: Christopher Heiny @ 2014-02-13 21:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-8-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Use rmi_read()/rmi_write() for reading/writing single-byte data. Also print
> error code when IO fails.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 170 ++++++++++++++++++++++---------------------
> 1 file changed, 88 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 1219e0c..a2f05bc 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -171,8 +171,9 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
>
> error = rmi_read_block(rmi_dev, query_base_addr,
> basic_query, sizeof(basic_query));
> - if (error < 0) {
> - dev_err(&rmi_dev->dev, "Failed to read device query registers.\n");
> + if (error) {
> + dev_err(&rmi_dev->dev,
> + "Failed to read device query registers: %d\n", error);
> return error;
> }
>
> @@ -205,25 +206,25 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> {
> u8 temp;
> int error;
> - u16 ctrl_base_addr;
> struct rmi_device *rmi_dev = fn->rmi_dev;
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> struct f01_data *f01 = fn->data;
> - struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + const struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> + u16 ctrl_base_addr = fn->fd.control_base_addr;
> u8 device_status;
>
> /*
> * Set the configured bit and (optionally) other important stuff
> * in the device control register.
> */
> - ctrl_base_addr = fn->fd.control_base_addr;
> - error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 control.\n");
> +
> + error = rmi_read(rmi_dev, fn->fd.control_base_addr,
> + &f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev, "Failed to read F01 control: %d\n", error);
> return error;
> }
> +
> switch (pdata->power_management.nosleep) {
> case RMI_F01_NOSLEEP_DEFAULT:
> break;
> @@ -249,11 +250,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>
> f01->device_control.ctrl0 |= RMI_F01_CRTL0_CONFIGURED_BIT;
>
> - error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to write F01 control.\n");
> + error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev, "Failed to write F01 control: %d\n", error);
> return error;
> }
>
> @@ -265,9 +265,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> error = rmi_read_block(rmi_dev, ctrl_base_addr,
> f01->device_control.interrupt_enable,
> sizeof(u8) * (f01->num_of_irq_regs));
> - if (error < 0) {
> + if (error) {
> dev_err(&fn->dev,
> - "Failed to read F01 control interrupt enable register.\n");
> + "Failed to read F01 control interrupt enable register: %d\n",
> + error);
> return error;
> }
>
> @@ -302,8 +303,10 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> } else {
> error = rmi_read(rmi_dev, f01->doze_interval_addr,
> &f01->device_control.doze_interval);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 doze interval register.\n");
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to read F01 doze interval register: %d\n",
> + error);
> return error;
> }
> }
> @@ -318,7 +321,9 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> error = rmi_read(rmi_dev, f01->wakeup_threshold_addr,
> &f01->device_control.wakeup_threshold);
> if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 wakeup threshold register.\n");
> + dev_err(&fn->dev,
> + "Failed to read F01 wakeup threshold register: %d\n",
> + error);
> return error;
> }
> }
> @@ -337,17 +342,19 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> } else {
> error = rmi_read(rmi_dev, f01->doze_holdoff_addr,
> &f01->device_control.doze_holdoff);
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to read F01 doze holdoff register.\n");
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to read F01 doze holdoff register: %d\n",
> + error);
> return error;
> }
> }
> }
>
> - error = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &device_status, sizeof(device_status));
> + error = rmi_read(rmi_dev, fn->fd.data_base_addr, &device_status);
> if (error < 0) {
> - dev_err(&fn->dev, "Failed to read device status.\n");
> + dev_err(&fn->dev,
> + "Failed to read device status: %d\n", error);
> return error;
> }
>
> @@ -364,50 +371,53 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> static int rmi_f01_config(struct rmi_function *fn)
> {
> struct f01_data *f01 = fn->data;
> - int retval;
> -
> - retval = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write device_control.reg.\n");
> - return retval;
> + int error;
> +
> + error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write device_control register: %d\n", error);
> + return error;
> }
>
> - retval = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
> - f01->device_control.interrupt_enable,
> - sizeof(u8) * f01->num_of_irq_regs);
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write interrupt enable.\n");
> - return retval;
> + error = rmi_write_block(fn->rmi_dev, f01->interrupt_enable_addr,
> + f01->device_control.interrupt_enable,
> + sizeof(u8) * f01->num_of_irq_regs);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write interrupt enable: %d\n", error);
> + return error;
> }
>
> if (f01->properties.has_adjustable_doze) {
> - retval = rmi_write_block(fn->rmi_dev, f01->doze_interval_addr,
> - &f01->device_control.doze_interval,
> - sizeof(u8));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write doze interval.\n");
> - return retval;
> + error = rmi_write(fn->rmi_dev, f01->doze_interval_addr,
> + f01->device_control.doze_interval);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write doze interval: %d\n", error);
> + return error;
> }
>
> - retval = rmi_write_block(fn->rmi_dev,
> + error = rmi_write_block(fn->rmi_dev,
> f01->wakeup_threshold_addr,
> &f01->device_control.wakeup_threshold,
> sizeof(u8));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write wakeup threshold.\n");
> - return retval;
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write wakeup threshold: %d\n",
> + error);
> + return error;
> }
> }
>
> if (f01->properties.has_adjustable_doze_holdoff) {
> - retval = rmi_write_block(fn->rmi_dev, f01->doze_holdoff_addr,
> - &f01->device_control.doze_holdoff,
> - sizeof(u8));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to write doze holdoff.\n");
> - return retval;
> + error = rmi_write(fn->rmi_dev, f01->doze_holdoff_addr,
> + f01->device_control.doze_holdoff);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to write doze holdoff: %d\n", error);
> + return error;
> }
> }
>
> @@ -439,23 +449,19 @@ static int rmi_f01_suspend(struct device *dev)
> struct f01_data *f01 = fn->data;
> int error;
>
> - f01->old_nosleep = f01->device_control.ctrl0 &
> - RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->old_nosleep =
> + f01->device_control.ctrl0 & RMI_F01_CRTL0_NOSLEEP_BIT;
> f01->device_control.ctrl0 &= ~RMI_F01_CRTL0_NOSLEEP_BIT;
>
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
>
> - error = rmi_write_block(rmi_dev,
> - fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> - dev_err(&fn->dev, "Failed to write sleep mode. Code: %d.\n",
> - error);
> + error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> + dev_err(&fn->dev, "Failed to write sleep mode: %d.\n", error);
> if (f01->old_nosleep)
> - f01->device_control.ctrl0 |=
> - RMI_F01_CRTL0_NOSLEEP_BIT;
> + f01->device_control.ctrl0 |= RMI_F01_CRTL0_NOSLEEP_BIT;
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
> return error;
> @@ -477,13 +483,11 @@ static int rmi_f01_resume(struct device *dev)
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
>
> - error = rmi_write_block(rmi_dev, fn->fd.control_base_addr,
> - &f01->device_control.ctrl0,
> - sizeof(f01->device_control.ctrl0));
> - if (error < 0) {
> + error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + f01->device_control.ctrl0);
> + if (error) {
> dev_err(&fn->dev,
> - "Failed to restore normal operation. Code: %d.\n",
> - error);
> + "Failed to restore normal operation: %d.\n", error);
> return error;
> }
>
> @@ -497,23 +501,25 @@ static int rmi_f01_attention(struct rmi_function *fn,
> unsigned long *irq_bits)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - int retval;
> + int error;
> u8 device_status;
>
> - retval = rmi_read_block(rmi_dev, fn->fd.data_base_addr,
> - &device_status, sizeof(device_status));
> - if (retval < 0) {
> - dev_err(&fn->dev, "Failed to read device status, code: %d.\n",
> - retval);
> - return retval;
> + error = rmi_read(rmi_dev, fn->fd.data_base_addr, &device_status);
> + if (error) {
> + dev_err(&fn->dev,
> + "Failed to read device status: %d.\n", error);
> + return error;
> }
>
> if (RMI_F01_STATUS_UNCONFIGURED(device_status)) {
> dev_warn(&fn->dev, "Device reset detected.\n");
> - retval = rmi_dev->driver->reset_handler(rmi_dev);
> - if (retval < 0)
> - return retval;
> + error = rmi_dev->driver->reset_handler(rmi_dev);
> + if (error) {
> + dev_err(&fn->dev, "Device reset failed: %d\n", error);
> + return error;
> + }
> }
> +
> return 0;
> }
>
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 11/11] Input: synaptics-rmi4 - remove data pointer from RMI fucntion structure
From: Christopher Heiny @ 2014-02-13 21:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-11-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> Device core provides way of accessing driver-private data, we should
> use it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_bus.h | 1 -
> drivers/input/rmi4/rmi_f01.c | 14 +++++------
> drivers/input/rmi4/rmi_f11.c | 57 ++++++++++++++++++++------------------------
> 3 files changed, 32 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
> index 02a2af8..5ad94c6 100644
> --- a/drivers/input/rmi4/rmi_bus.h
> +++ b/drivers/input/rmi4/rmi_bus.h
> @@ -48,7 +48,6 @@ struct rmi_function {
> int num_of_irqs;
> int irq_pos;
> unsigned long *irq_mask;
> - void *data;
> struct list_head node;
>
> #ifdef CONFIG_RMI4_DEBUG
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 36fcf8d..e646f60 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -354,14 +354,14 @@ static int rmi_f01_probe(struct rmi_function *fn)
> return -EINVAL;
> }
>
> - fn->data = f01;
> + dev_set_drvdata(&fn->dev, f01);
>
> return 0;
> }
>
> static int rmi_f01_config(struct rmi_function *fn)
> {
> - struct f01_data *f01 = fn->data;
> + struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> int error;
>
> error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> @@ -419,8 +419,7 @@ static int rmi_f01_config(struct rmi_function *fn)
> static int rmi_f01_suspend(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *f01 = fn->data;
> + struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> int error;
>
> f01->old_nosleep =
> @@ -430,7 +429,7 @@ static int rmi_f01_suspend(struct device *dev)
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_SENSOR_SLEEP;
>
> - error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> f01->device_control.ctrl0);
> if (error) {
> dev_err(&fn->dev, "Failed to write sleep mode: %d.\n", error);
> @@ -447,8 +446,7 @@ static int rmi_f01_suspend(struct device *dev)
> static int rmi_f01_resume(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f01_data *f01 = fn->data;
> + struct f01_data *f01 = dev_get_drvdata(&fn->dev);
> int error;
>
> if (f01->old_nosleep)
> @@ -457,7 +455,7 @@ static int rmi_f01_resume(struct device *dev)
> f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_SLEEP_MODE_MASK;
> f01->device_control.ctrl0 |= RMI_SLEEP_MODE_NORMAL;
>
> - error = rmi_write(rmi_dev, fn->fd.control_base_addr,
> + error = rmi_write(fn->rmi_dev, fn->fd.control_base_addr,
> f01->device_control.ctrl0);
> if (error) {
> dev_err(&fn->dev,
> diff --git a/drivers/input/rmi4/rmi_f11.c b/drivers/input/rmi4/rmi_f11.c
> index 5072437..a26b944 100644
> --- a/drivers/input/rmi4/rmi_f11.c
> +++ b/drivers/input/rmi4/rmi_f11.c
> @@ -1087,9 +1087,8 @@ static int rmi_f11_get_query_parameters(struct rmi_device *rmi_dev,
> /* This operation is done in a number of places, so we have a handy routine
> * for it.
> */
> -static void f11_set_abs_params(struct rmi_function *fn)
> +static void f11_set_abs_params(struct rmi_function *fn, struct f11_data *f11)
> {
> - struct f11_data *f11 = fn->data;
> struct f11_2d_sensor *sensor = &f11->sensor;
> struct input_dev *input = sensor->input;
> /* These two lines are not doing what we want them to. So we use
> @@ -1190,7 +1189,6 @@ static int rmi_f11_initialize(struct rmi_function *fn)
> if (!f11)
> return -ENOMEM;
>
> - fn->data = f11;
> f11->rezero_wait_ms = pdata->f11_rezero_wait;
>
> query_base_addr = fn->fd.query_base_addr;
> @@ -1280,13 +1278,16 @@ static int rmi_f11_initialize(struct rmi_function *fn)
> }
>
> mutex_init(&f11->dev_controls_mutex);
> +
> + dev_set_drvdata(&fn->dev, f11);
> +
> return 0;
> }
>
> static int rmi_f11_register_devices(struct rmi_function *fn)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f11_data *f11 = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> struct input_dev *input_dev;
> struct input_dev *input_dev_mouse;
> struct rmi_driver *driver = rmi_dev->driver;
> @@ -1304,8 +1305,8 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> rc = driver->set_input_params(rmi_dev, input_dev);
> if (rc < 0) {
> dev_err(&fn->dev,
> - "%s: Error in setting input device.\n",
> - __func__);
> + "%s: Error in setting input device.\n",
> + __func__);
> goto error_unregister;
> }
> }
> @@ -1319,7 +1320,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> set_bit(EV_ABS, input_dev->evbit);
> input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>
> - f11_set_abs_params(fn);
> + f11_set_abs_params(fn, f11);
>
> if (sensor->sens_query.has_rel) {
> set_bit(EV_REL, input_dev->evbit);
> @@ -1327,7 +1328,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> set_bit(REL_Y, input_dev->relbit);
> }
> rc = input_register_device(input_dev);
> - if (rc < 0) {
> + if (rc) {
> input_free_device(input_dev);
> sensor->input = NULL;
> goto error_unregister;
> @@ -1347,8 +1348,8 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> input_dev_mouse);
> if (rc < 0) {
> dev_err(&fn->dev,
> - "%s: Error in setting input device.\n",
> - __func__);
> + "%s: Error in setting input device.\n",
> + __func__);
> goto error_unregister;
> }
> }
> @@ -1366,7 +1367,7 @@ static int rmi_f11_register_devices(struct rmi_function *fn)
> set_bit(BTN_RIGHT, input_dev_mouse->keybit);
>
> rc = input_register_device(input_dev_mouse);
> - if (rc < 0) {
> + if (rc) {
> input_free_device(input_dev_mouse);
> sensor->mouse_input = NULL;
> goto error_unregister;
> @@ -1390,19 +1391,9 @@ error_unregister:
> return rc;
> }
>
> -static void rmi_f11_free_devices(struct rmi_function *fn)
> -{
> - struct f11_data *f11 = fn->data;
> -
> - if (f11->sensor.input)
> - input_unregister_device(f11->sensor.input);
> - if (f11->sensor.mouse_input)
> - input_unregister_device(f11->sensor.mouse_input);
> -}
> -
> static int rmi_f11_config(struct rmi_function *fn)
> {
> - struct f11_data *f11 = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> int rc;
>
> rc = f11_write_control_regs(fn, &f11->sensor.sens_query,
> @@ -1416,7 +1407,7 @@ static int rmi_f11_config(struct rmi_function *fn)
> static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
> {
> struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f11_data *f11 = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> u16 data_base_addr = fn->fd.data_base_addr;
> u16 data_base_addr_offset = 0;
> int error;
> @@ -1425,7 +1416,7 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
> data_base_addr + data_base_addr_offset,
> f11->sensor.data_pkt,
> f11->sensor.pkt_size);
> - if (error < 0)
> + if (error)
> return error;
>
> rmi_f11_finger_handler(f11, &f11->sensor);
> @@ -1438,18 +1429,17 @@ static int rmi_f11_attention(struct rmi_function *fn, unsigned long *irq_bits)
> static int rmi_f11_resume(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> - struct rmi_device *rmi_dev = fn->rmi_dev;
> - struct f11_data *data = fn->data;
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> int error;
>
> dev_dbg(&fn->dev, "Resuming...\n");
> - if (!data->rezero_wait_ms)
> + if (!f11->rezero_wait_ms)
> return 0;
>
> - mdelay(data->rezero_wait_ms);
> + mdelay(f11->rezero_wait_ms);
>
> - error = rmi_write(rmi_dev, fn->fd.command_base_addr, RMI_F11_REZERO);
> - if (error < 0) {
> + error = rmi_write(fn->rmi_dev, fn->fd.command_base_addr, RMI_F11_REZERO);
> + if (error) {
> dev_err(&fn->dev,
> "%s: failed to issue rezero command, error = %d.",
> __func__, error);
> @@ -1479,7 +1469,12 @@ static int rmi_f11_probe(struct rmi_function *fn)
>
> static void rmi_f11_remove(struct rmi_function *fn)
> {
> - rmi_f11_free_devices(fn);
> + struct f11_data *f11 = dev_get_drvdata(&fn->dev);
> +
> + if (f11->sensor.input)
> + input_unregister_device(f11->sensor.input);
> + if (f11->sensor.mouse_input)
> + input_unregister_device(f11->sensor.mouse_input);
> }
>
> static struct rmi_function_handler rmi_f11_handler = {
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: Use put_device() and device_type.release() to free storage.
From: Courtney Cavin @ 2014-02-13 21:59 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Christopher Heiny, Linux Input, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij, Benjamin Tissoires, David Herrmann, Jiri Kosina
In-Reply-To: <20140213061524.GA15260@core.coreip.homeip.net>
On Thu, Feb 13, 2014 at 07:15:24AM +0100, Dmitry Torokhov wrote:
> On Wed, Feb 12, 2014 at 06:31:04PM -0800, Christopher Heiny wrote:
> > Input: synaptics-rmi4 - Use put_device() and device_type.release()
> > to free storage.
> >
> > From: Christopher Heiny <cheiny@synaptics.com>
> >
> > For rmi_sensor and rmi_function device_types, use put_device() and
> > the assocated device_type.release() function to clean up related
> > structures and storage in the correct and safe order.
> >
> > Allocate irq_mask as part of struct rmi_function.
> >
> > Delete unused rmi_driver_irq_get_mask() function.
[...]
> Input: synaptics-rmi4 - use put_device() to free devices
>
> From: Christopher Heiny <cheiny@synaptics.com>
>
> For rmi_sensor and rmi_function device_types, use put_device() and
> the associated device_type->release() function to clean up related
> structures and storage in the correct and safe order.
>
> Allocate irq_mask as part of struct rmi_function.
>
> Suggested-by: Courtney Cavin <courtney.cavin@sonymobile.com>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/rmi4/rmi_bus.c | 30 +++++++++++++++++++++---------
> drivers/input/rmi4/rmi_bus.h | 7 ++++---
> drivers/input/rmi4/rmi_driver.c | 25 +++++++------------------
> 3 files changed, 32 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
[...]
> @@ -142,6 +150,7 @@ EXPORT_SYMBOL(rmi_unregister_transport_device);
> static void rmi_release_function(struct device *dev)
> {
> struct rmi_function *fn = to_rmi_function(dev);
> +
> kfree(fn);
> }
Ownership of this memory is a bit weird...
[...]
> void rmi_unregister_function(struct rmi_function *fn)
> {
> + device_del(&fn->dev);
> rmi_function_teardown_debugfs(fn);
> - device_unregister(&fn->dev);
> + put_device(&fn->dev);
> }
Here clearly the bus code owns it...
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
[...]
> static int rmi_create_function(struct rmi_device *rmi_dev,
> - void *ctx, const struct pdt_entry *pdt)
> + void *ctx, const struct pdt_entry *pdt)
> {
> struct device *dev = &rmi_dev->dev;
> struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> @@ -630,7 +629,9 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> dev_dbg(dev, "Initializing F%02X for %s.\n",
> pdt->function_number, pdata->sensor_name);
>
> - fn = kzalloc(sizeof(struct rmi_function), GFP_KERNEL);
> + fn = kzalloc(sizeof(struct rmi_function) +
> + BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long),
> + GFP_KERNEL);
But it's allocated in the chip driver...
> if (!fn) {
> dev_err(dev, "Failed to allocate memory for F%02X\n",
> pdt->function_number);
> @@ -646,22 +647,12 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> fn->irq_pos = *current_irq_count;
> *current_irq_count += fn->num_of_irqs;
>
> - fn->irq_mask = kzalloc(
> - BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long),
> - GFP_KERNEL);
> - if (!fn->irq_mask) {
> - dev_err(dev, "%s: Failed to create irq_mask for F%02X.\n",
> - __func__, pdt->function_number);
> - error = -ENOMEM;
> - goto err_free_mem;
> - }
> -
> for (i = 0; i < fn->num_of_irqs; i++)
> set_bit(fn->irq_pos + i, fn->irq_mask);
>
> error = rmi_register_function(fn);
> if (error)
> - goto err_free_irq_mask;
> + goto err_put_fn;
>
> if (pdt->function_number == 0x01)
> data->f01_container = fn;
> @@ -670,10 +661,8 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
>
> return RMI_SCAN_CONTINUE;
>
> -err_free_irq_mask:
> - kfree(fn->irq_mask);
> -err_free_mem:
> - kfree(fn);
> +err_put_fn:
> + put_device(&fn->dev);
And the chip driver now is expected to know it's a device, and trust
that the bus code knows how to free the memory.
> return error;
> }
>
As this clearly fixes a bug or two, I say we should take this patch
as-is and worry about proper ownership at some other time.
-Courtney
^ permalink raw reply
* Re: [PATCH 05/11] Input: synaptics-rmi4 - remove control_mutex from f01_data
From: Christopher Heiny @ 2014-02-13 23:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <1392269277-16391-5-git-send-email-dmitry.torokhov@gmail.com>
On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> It is not used by anyone.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_f01.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 30e0b50..6f90a6c 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -125,7 +125,6 @@ struct f01_data {
> struct f01_basic_properties properties;
>
> struct f01_device_control device_control;
> - struct mutex control_mutex;
>
> u8 device_status;
>
> @@ -214,8 +213,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> struct f01_data *data = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
>
> - mutex_init(&data->control_mutex);
> -
> /*
> * Set the configured bit and (optionally) other important stuff
> * in the device control register.
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* [PATCH v3] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
From: Barry Song @ 2014-02-14 3:20 UTC (permalink / raw)
To: dmitry.torokhov, linux-input
Cc: linux-arm-kernel, workgroup.linux, Xianglong Du, Rongjun Ying,
Barry Song
From: Xianglong Du <Xianglong.Du@csr.com>
this patch adds a delayed_work to detect the untouch of onkey since HW will
not generate interrupt for it.
at the same time, we move the KEY event to POWER instead of SUSPEND, which
will be suitable for both Android and Linux. Userspace PowerManager Daemon
will decide to suspend or shutdown based on how long we have touched onkey
Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
-v3: move to use custom devres action
drivers/input/misc/sirfsoc-onkey.c | 60 +++++++++++++++++++++++++++++------
1 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index e8897c3..b354da8 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -13,16 +13,44 @@
#include <linux/input.h>
#include <linux/rtc/sirfsoc_rtciobrg.h>
#include <linux/of.h>
+#include <linux/workqueue.h>
struct sirfsoc_pwrc_drvdata {
u32 pwrc_base;
struct input_dev *input;
+ struct delayed_work work;
};
#define PWRC_ON_KEY_BIT (1 << 0)
#define PWRC_INT_STATUS 0xc
#define PWRC_INT_MASK 0x10
+#define PWRC_PIN_STATUS 0x14
+#define PWRC_KEY_DETECT_UP_TIME 20 /* ms*/
+
+static inline int sirfsoc_pwrc_is_on_key_down(
+ struct sirfsoc_pwrc_drvdata *pwrcdrv)
+{
+ int state = sirfsoc_rtc_iobrg_readl(
+ pwrcdrv->pwrc_base + PWRC_PIN_STATUS)
+ & PWRC_ON_KEY_BIT;
+ return !state; /* ON_KEY is active low */
+}
+
+static void sirfsoc_pwrc_report_event(struct work_struct *work)
+{
+ struct sirfsoc_pwrc_drvdata *pwrcdrv =
+ container_of((struct delayed_work *)work,
+ struct sirfsoc_pwrc_drvdata, work);
+
+ if (!sirfsoc_pwrc_is_on_key_down(pwrcdrv)) {
+ input_event(pwrcdrv->input, EV_KEY, KEY_POWER, 0);
+ input_sync(pwrcdrv->input);
+ } else {
+ schedule_delayed_work(&pwrcdrv->work,
+ msecs_to_jiffies(PWRC_KEY_DETECT_UP_TIME));
+ }
+}
static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
{
@@ -34,21 +62,22 @@ static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
sirfsoc_rtc_iobrg_writel(int_status & ~PWRC_ON_KEY_BIT,
pwrcdrv->pwrc_base + PWRC_INT_STATUS);
- /*
- * For a typical Linux system, we report KEY_SUSPEND to trigger apm-power.c
- * to queue a SUSPEND APM event
- */
- input_event(pwrcdrv->input, EV_PWR, KEY_SUSPEND, 1);
- input_sync(pwrcdrv->input);
- /*
- * Todo: report KEY_POWER event for Android platforms, Android PowerManager
- * will handle the suspend and powerdown/hibernation
- */
+ input_event(pwrcdrv->input, EV_KEY, KEY_POWER, 1);
+ input_sync(pwrcdrv->input);
+ schedule_delayed_work(&pwrcdrv->work,
+ msecs_to_jiffies(PWRC_KEY_DETECT_UP_TIME));
return IRQ_HANDLED;
}
+static void sirfsoc_pwrc_stop_updetect(void *data)
+{
+ struct sirfsoc_pwrc_drvdata *pwrcdrv = data;
+
+ cancel_delayed_work_sync(&pwrcdrv->work);
+}
+
static const struct of_device_id sirfsoc_pwrc_of_match[] = {
{ .compatible = "sirf,prima2-pwrc" },
{},
@@ -86,7 +115,9 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
pwrcdrv->input->name = "sirfsoc pwrckey";
pwrcdrv->input->phys = "pwrc/input0";
- pwrcdrv->input->evbit[0] = BIT_MASK(EV_PWR);
+ pwrcdrv->input->evbit[0] = BIT_MASK(EV_KEY);
+
+ INIT_DELAYED_WORK(&pwrcdrv->work, sirfsoc_pwrc_report_event);
irq = platform_get_irq(pdev, 0);
error = devm_request_irq(&pdev->dev, irq,
@@ -98,6 +129,13 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
return error;
}
+ error = devm_add_action(&pdev->dev, sirfsoc_pwrc_stop_updetect, pwrcdrv);
+ if (error) {
+ dev_err(&pdev->dev, "failed to add stop untouch detection action, %d\n",
+ error);
+ return error;
+ }
+
sirfsoc_rtc_iobrg_writel(
sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK) |
PWRC_ON_KEY_BIT,
--
1.7.5.4
^ permalink raw reply related
* [PATCH 1/1] Input: s3c2410 - Trivial cleanup in header file
From: Sachin Kamat @ 2014-02-14 7:05 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, trivial, sachin.kamat
Commit 436d42c61c3e ("ARM: samsung: move platform_data definitions")
moved the files to the current location but forgot to remove the pointer
to its previous location. Clean it up. While at it also change the header
file protection macros appropriately and remove whitespace errors.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
include/linux/platform_data/touchscreen-s3c2410.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/include/linux/platform_data/touchscreen-s3c2410.h b/include/linux/platform_data/touchscreen-s3c2410.h
index 26fdb22e0fc2..58dc7c5ae63b 100644
--- a/include/linux/platform_data/touchscreen-s3c2410.h
+++ b/include/linux/platform_data/touchscreen-s3c2410.h
@@ -1,5 +1,4 @@
-/* arch/arm/plat-samsung/include/plat/ts.h
- *
+/*
* Copyright (c) 2005 Arnaud Patard <arnaud.patard@rtp-net.org>
*
* This program is free software; you can redistribute it and/or modify
@@ -7,14 +6,14 @@
* published by the Free Software Foundation.
*/
-#ifndef __ASM_ARM_TS_H
-#define __ASM_ARM_TS_H
+#ifndef __TOUCHSCREEN_S3C2410_H
+#define __TOUCHSCREEN_S3C2410_H
struct s3c2410_ts_mach_info {
- int delay;
- int presc;
- int oversampling_shift;
- void (*cfg_gpio)(struct platform_device *dev);
+ int delay;
+ int presc;
+ int oversampling_shift;
+ void (*cfg_gpio)(struct platform_device *dev);
};
extern void s3c24xx_ts_set_platdata(struct s3c2410_ts_mach_info *);
@@ -22,4 +21,4 @@ extern void s3c24xx_ts_set_platdata(struct s3c2410_ts_mach_info *);
/* defined by architecture to configure gpio */
extern void s3c24xx_ts_cfg_gpio(struct platform_device *dev);
-#endif /* __ASM_ARM_TS_H */
+#endif /*__TOUCHSCREEN_S3C2410_H */
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 04/11] Input: synaptics-rmi4 - fix LTS handling in F01
From: Dmitry Torokhov @ 2014-02-14 8:05 UTC (permalink / raw)
To: Christopher Heiny
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <52FD1DCE.2040005@synaptics.com>
On Thu, Feb 13, 2014 at 11:32:30AM -0800, Christopher Heiny wrote:
> On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> >From: Christopher Heiny <cheiny@synaptics.com>
> >
> >Both F01_RMI_Ctrl2 and F01_RMI_Ctrl3 (doze_interval and wakeup_threshold)
> >are controlled by the has_adjustable_doze bit.
> >
> >Signed-off-by: Christopher Heiny<cheiny@synaptics.com>
> >Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Not sure if this need an Ack, but just in case.
>
> Acked-by: Christopher Heiny <cheiny@synaptics.com>
Not the formal ack but I wanted to make sure I split it off correctly.
Thanks for verifying it!
--
Dmitry
^ permalink raw reply
* Re: [PATCH 03/11] Input: synaptics-rmi4 - do not update configuration in rmi_f01_probe()
From: Dmitry Torokhov @ 2014-02-13 21:54 UTC (permalink / raw)
To: Christopher Heiny
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <52FD1BC0.1050009@synaptics.com>
On Thu, Feb 13, 2014 at 11:23:44AM -0800, Christopher Heiny wrote:
> On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
> >Do not write configuration data in probe(), we have config() for that.
>
> Then we should call config() in rmi_function_probe() to ensure that
> any platform data or device tree configuration settings get written
> to the device.
Well, yes, we may elect to update device configuration in probe, but
then we should not be doing that 2nd time in ->config(). We shoudl pick
either one or another.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4: Use put_device() and device_type.release() to free storage.
From: Dmitry Torokhov @ 2014-02-13 22:10 UTC (permalink / raw)
To: Courtney Cavin
Cc: Christopher Heiny, Linux Input, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij, Benjamin Tissoires, David Herrmann, Jiri Kosina
In-Reply-To: <20140213215930.GH1706@sonymobile.com>
On Thu, Feb 13, 2014 at 01:59:31PM -0800, Courtney Cavin wrote:
> On Thu, Feb 13, 2014 at 07:15:24AM +0100, Dmitry Torokhov wrote:
> > On Wed, Feb 12, 2014 at 06:31:04PM -0800, Christopher Heiny wrote:
> > > Input: synaptics-rmi4 - Use put_device() and device_type.release()
> > > to free storage.
> > >
> > > From: Christopher Heiny <cheiny@synaptics.com>
> > >
> > > For rmi_sensor and rmi_function device_types, use put_device() and
> > > the assocated device_type.release() function to clean up related
> > > structures and storage in the correct and safe order.
> > >
> > > Allocate irq_mask as part of struct rmi_function.
> > >
> > > Delete unused rmi_driver_irq_get_mask() function.
> [...]
> > Input: synaptics-rmi4 - use put_device() to free devices
> >
> > From: Christopher Heiny <cheiny@synaptics.com>
> >
> > For rmi_sensor and rmi_function device_types, use put_device() and
> > the associated device_type->release() function to clean up related
> > structures and storage in the correct and safe order.
> >
> > Allocate irq_mask as part of struct rmi_function.
> >
> > Suggested-by: Courtney Cavin <courtney.cavin@sonymobile.com>
> > Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/rmi4/rmi_bus.c | 30 +++++++++++++++++++++---------
> > drivers/input/rmi4/rmi_bus.h | 7 ++++---
> > drivers/input/rmi4/rmi_driver.c | 25 +++++++------------------
> > 3 files changed, 32 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> [...]
> > @@ -142,6 +150,7 @@ EXPORT_SYMBOL(rmi_unregister_transport_device);
> > static void rmi_release_function(struct device *dev)
> > {
> > struct rmi_function *fn = to_rmi_function(dev);
> > +
> > kfree(fn);
> > }
>
> Ownership of this memory is a bit weird...
>
> [...]
> > void rmi_unregister_function(struct rmi_function *fn)
> > {
> > + device_del(&fn->dev);
> > rmi_function_teardown_debugfs(fn);
> > - device_unregister(&fn->dev);
> > + put_device(&fn->dev);
> > }
>
> Here clearly the bus code owns it...
>
> > diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> [...]
> > static int rmi_create_function(struct rmi_device *rmi_dev,
> > - void *ctx, const struct pdt_entry *pdt)
> > + void *ctx, const struct pdt_entry *pdt)
> > {
> > struct device *dev = &rmi_dev->dev;
> > struct rmi_driver_data *data = dev_get_drvdata(&rmi_dev->dev);
> > @@ -630,7 +629,9 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> > dev_dbg(dev, "Initializing F%02X for %s.\n",
> > pdt->function_number, pdata->sensor_name);
> >
> > - fn = kzalloc(sizeof(struct rmi_function), GFP_KERNEL);
> > + fn = kzalloc(sizeof(struct rmi_function) +
> > + BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long),
> > + GFP_KERNEL);
>
> But it's allocated in the chip driver...
>
> > if (!fn) {
> > dev_err(dev, "Failed to allocate memory for F%02X\n",
> > pdt->function_number);
> > @@ -646,22 +647,12 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> > fn->irq_pos = *current_irq_count;
> > *current_irq_count += fn->num_of_irqs;
> >
> > - fn->irq_mask = kzalloc(
> > - BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long),
> > - GFP_KERNEL);
> > - if (!fn->irq_mask) {
> > - dev_err(dev, "%s: Failed to create irq_mask for F%02X.\n",
> > - __func__, pdt->function_number);
> > - error = -ENOMEM;
> > - goto err_free_mem;
> > - }
> > -
> > for (i = 0; i < fn->num_of_irqs; i++)
> > set_bit(fn->irq_pos + i, fn->irq_mask);
> >
> > error = rmi_register_function(fn);
> > if (error)
> > - goto err_free_irq_mask;
> > + goto err_put_fn;
> >
> > if (pdt->function_number == 0x01)
> > data->f01_container = fn;
> > @@ -670,10 +661,8 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
> >
> > return RMI_SCAN_CONTINUE;
> >
> > -err_free_irq_mask:
> > - kfree(fn->irq_mask);
> > -err_free_mem:
> > - kfree(fn);
> > +err_put_fn:
> > + put_device(&fn->dev);
>
> And the chip driver now is expected to know it's a device, and trust
> that the bus code knows how to free the memory.
Yeah. That is why for input devices I have a separate
input_allocate_device and input_free_device... But given that RMI is
pretty-much self-contained I think we can live with this.
>
> As this clearly fixes a bug or two, I say we should take this patch
> as-is and worry about proper ownership at some other time.
>
> -Courtney
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
From: Dmitry Torokhov @ 2014-02-14 7:57 UTC (permalink / raw)
To: Barry Song
Cc: linux-input, linux-arm-kernel, workgroup.linux, Xianglong Du,
Rongjun Ying, Barry Song
In-Reply-To: <1392348001-4950-1-git-send-email-21cnbao@gmail.com>
Hi Barry,
On Fri, Feb 14, 2014 at 11:20:01AM +0800, Barry Song wrote:
> From: Xianglong Du <Xianglong.Du@csr.com>
>
> this patch adds a delayed_work to detect the untouch of onkey since HW will
> not generate interrupt for it.
>
> at the same time, we move the KEY event to POWER instead of SUSPEND, which
> will be suitable for both Android and Linux. Userspace PowerManager Daemon
> will decide to suspend or shutdown based on how long we have touched onkey
>
> Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
> Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
> -v3: move to use custom devres action
Thank you for making the changes, however it seems that we can control
whether the device generates interrupts or not and so we can implement
open and close methods. If patch below works then your
cancel_delayed_work() call should go into sirfosc_pwrc_close() and we do
not need to use devm_free_irq() not custom action.
Thanks.
--
Dmitry
Input: sirfsoc-onkey - implement open and close methods
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
We can control whetehr device generates interrupts or not so let's
implement open and close methods of input device so that we do not do any
processing until there are users.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/sirfsoc-onkey.c | 50 +++++++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
index e8897c3..dc7db65 100644
--- a/drivers/input/misc/sirfsoc-onkey.c
+++ b/drivers/input/misc/sirfsoc-onkey.c
@@ -49,6 +49,35 @@ static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static void sirfsoc_pwrc_toggle_interrupts(struct sirfsoc_pwrc_drvdata *pwrcdrv,
+ bool enable)
+{
+ u32 int_mask;
+
+ int_mask = sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK);
+ if (enable)
+ int_mask |= PWRC_ON_KEY_BIT;
+ else
+ int_mask &= ~PWRC_ON_KEY_BIT;
+ sirfsoc_rtc_iobrg_writel(int_mask, pwrcdrv->pwrc_base + PWRC_INT_MASK);
+}
+
+static int sirfsoc_pwrc_open(struct input_dev *input)
+{
+ struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
+
+ sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
+
+ return 0;
+}
+
+static void sirfsoc_pwrc_close(struct input_dev *input)
+{
+ struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
+
+ sirfsoc_pwrc_toggle_interrupts(pwrcdrv, false);
+}
+
static const struct of_device_id sirfsoc_pwrc_of_match[] = {
{ .compatible = "sirf,prima2-pwrc" },
{},
@@ -70,7 +99,7 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
}
/*
- * we can't use of_iomap because pwrc is not mapped in memory,
+ * We can't use of_iomap because pwrc is not mapped in memory,
* the so-called base address is only offset in rtciobrg
*/
error = of_property_read_u32(np, "reg", &pwrcdrv->pwrc_base);
@@ -88,6 +117,11 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
pwrcdrv->input->phys = "pwrc/input0";
pwrcdrv->input->evbit[0] = BIT_MASK(EV_PWR);
+ pwrcdrv->input->open = sirfsoc_pwrc_open;
+ pwrcdrv->input->close = sirfsoc_pwrc_close;
+
+ input_set_drvdata(pwrcdrv->input, pwrcdrv);
+
irq = platform_get_irq(pdev, 0);
error = devm_request_irq(&pdev->dev, irq,
sirfsoc_pwrc_isr, IRQF_SHARED,
@@ -98,11 +132,6 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
return error;
}
- sirfsoc_rtc_iobrg_writel(
- sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK) |
- PWRC_ON_KEY_BIT,
- pwrcdrv->pwrc_base + PWRC_INT_MASK);
-
error = input_register_device(pwrcdrv->input);
if (error) {
dev_err(&pdev->dev,
@@ -129,15 +158,16 @@ static int pwrc_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
+ struct input_dev *input = pwrcdrv->input;
/*
* Do not mask pwrc interrupt as we want pwrc work as a wakeup source
* if users touch X_ONKEY_B, see arch/arm/mach-prima2/pm.c
*/
- sirfsoc_rtc_iobrg_writel(
- sirfsoc_rtc_iobrg_readl(
- pwrcdrv->pwrc_base + PWRC_INT_MASK) | PWRC_ON_KEY_BIT,
- pwrcdrv->pwrc_base + PWRC_INT_MASK);
+ mutex_lock(&input->mutex);
+ if (input->users)
+ sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
+ mutex_unlock(&input->mutex);
return 0;
}
^ permalink raw reply related
* Re: [PATCH 1/1] Input: s3c2410 - Trivial cleanup in header file
From: Dmitry Torokhov @ 2014-02-14 7:57 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-input, trivial
In-Reply-To: <1392361539-24397-1-git-send-email-sachin.kamat@linaro.org>
On Fri, Feb 14, 2014 at 12:35:39PM +0530, Sachin Kamat wrote:
> Commit 436d42c61c3e ("ARM: samsung: move platform_data definitions")
> moved the files to the current location but forgot to remove the pointer
> to its previous location. Clean it up. While at it also change the header
> file protection macros appropriately and remove whitespace errors.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Applied, thank you.
> ---
> include/linux/platform_data/touchscreen-s3c2410.h | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/platform_data/touchscreen-s3c2410.h b/include/linux/platform_data/touchscreen-s3c2410.h
> index 26fdb22e0fc2..58dc7c5ae63b 100644
> --- a/include/linux/platform_data/touchscreen-s3c2410.h
> +++ b/include/linux/platform_data/touchscreen-s3c2410.h
> @@ -1,5 +1,4 @@
> -/* arch/arm/plat-samsung/include/plat/ts.h
> - *
> +/*
> * Copyright (c) 2005 Arnaud Patard <arnaud.patard@rtp-net.org>
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -7,14 +6,14 @@
> * published by the Free Software Foundation.
> */
>
> -#ifndef __ASM_ARM_TS_H
> -#define __ASM_ARM_TS_H
> +#ifndef __TOUCHSCREEN_S3C2410_H
> +#define __TOUCHSCREEN_S3C2410_H
>
> struct s3c2410_ts_mach_info {
> - int delay;
> - int presc;
> - int oversampling_shift;
> - void (*cfg_gpio)(struct platform_device *dev);
> + int delay;
> + int presc;
> + int oversampling_shift;
> + void (*cfg_gpio)(struct platform_device *dev);
> };
>
> extern void s3c24xx_ts_set_platdata(struct s3c2410_ts_mach_info *);
> @@ -22,4 +21,4 @@ extern void s3c24xx_ts_set_platdata(struct s3c2410_ts_mach_info *);
> /* defined by architecture to configure gpio */
> extern void s3c24xx_ts_cfg_gpio(struct platform_device *dev);
>
> -#endif /* __ASM_ARM_TS_H */
> +#endif /*__TOUCHSCREEN_S3C2410_H */
> --
> 1.7.9.5
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
From: Barry Song @ 2014-02-14 8:49 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
DL-SHA-WorkGroupLinux, Xianglong Du, Rongjun Ying, Barry Song
In-Reply-To: <20140214075717.GA14682@core.coreip.homeip.net>
2014-02-14 15:57 GMT+08:00 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> Hi Barry,
>
> On Fri, Feb 14, 2014 at 11:20:01AM +0800, Barry Song wrote:
>> From: Xianglong Du <Xianglong.Du@csr.com>
>>
>> this patch adds a delayed_work to detect the untouch of onkey since HW will
>> not generate interrupt for it.
>>
>> at the same time, we move the KEY event to POWER instead of SUSPEND, which
>> will be suitable for both Android and Linux. Userspace PowerManager Daemon
>> will decide to suspend or shutdown based on how long we have touched onkey
>>
>> Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
>> Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> -v3: move to use custom devres action
>
> Thank you for making the changes, however it seems that we can control
> whether the device generates interrupts or not and so we can implement
> open and close methods. If patch below works then your
> cancel_delayed_work() call should go into sirfosc_pwrc_close() and we do
> not need to use devm_free_irq() not custom action.
this one looks making lots of senses. it makes sure HW will not
trigger any SW behaviour before probe() finishes, and also makes sure
HW will not trigger any SW behaviour in remove(). i'd like xianglong
to give a quick test on it.
>
> Thanks.
>
> --
> Dmitry
>
>
> Input: sirfsoc-onkey - implement open and close methods
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> We can control whetehr device generates interrupts or not so let's
> implement open and close methods of input device so that we do not do any
> processing until there are users.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/sirfsoc-onkey.c | 50 +++++++++++++++++++++++++++++-------
> 1 file changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
> index e8897c3..dc7db65 100644
> --- a/drivers/input/misc/sirfsoc-onkey.c
> +++ b/drivers/input/misc/sirfsoc-onkey.c
> @@ -49,6 +49,35 @@ static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static void sirfsoc_pwrc_toggle_interrupts(struct sirfsoc_pwrc_drvdata *pwrcdrv,
> + bool enable)
> +{
> + u32 int_mask;
> +
> + int_mask = sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK);
> + if (enable)
> + int_mask |= PWRC_ON_KEY_BIT;
> + else
> + int_mask &= ~PWRC_ON_KEY_BIT;
> + sirfsoc_rtc_iobrg_writel(int_mask, pwrcdrv->pwrc_base + PWRC_INT_MASK);
> +}
> +
> +static int sirfsoc_pwrc_open(struct input_dev *input)
> +{
> + struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
> +
> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
> +
> + return 0;
> +}
> +
> +static void sirfsoc_pwrc_close(struct input_dev *input)
> +{
> + struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
> +
> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, false);
> +}
> +
> static const struct of_device_id sirfsoc_pwrc_of_match[] = {
> { .compatible = "sirf,prima2-pwrc" },
> {},
> @@ -70,7 +99,7 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> }
>
> /*
> - * we can't use of_iomap because pwrc is not mapped in memory,
> + * We can't use of_iomap because pwrc is not mapped in memory,
> * the so-called base address is only offset in rtciobrg
> */
> error = of_property_read_u32(np, "reg", &pwrcdrv->pwrc_base);
> @@ -88,6 +117,11 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> pwrcdrv->input->phys = "pwrc/input0";
> pwrcdrv->input->evbit[0] = BIT_MASK(EV_PWR);
>
> + pwrcdrv->input->open = sirfsoc_pwrc_open;
> + pwrcdrv->input->close = sirfsoc_pwrc_close;
> +
> + input_set_drvdata(pwrcdrv->input, pwrcdrv);
> +
> irq = platform_get_irq(pdev, 0);
> error = devm_request_irq(&pdev->dev, irq,
> sirfsoc_pwrc_isr, IRQF_SHARED,
> @@ -98,11 +132,6 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> return error;
> }
>
> - sirfsoc_rtc_iobrg_writel(
> - sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK) |
> - PWRC_ON_KEY_BIT,
> - pwrcdrv->pwrc_base + PWRC_INT_MASK);
> -
> error = input_register_device(pwrcdrv->input);
> if (error) {
> dev_err(&pdev->dev,
> @@ -129,15 +158,16 @@ static int pwrc_resume(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
> + struct input_dev *input = pwrcdrv->input;
>
> /*
> * Do not mask pwrc interrupt as we want pwrc work as a wakeup source
> * if users touch X_ONKEY_B, see arch/arm/mach-prima2/pm.c
> */
> - sirfsoc_rtc_iobrg_writel(
> - sirfsoc_rtc_iobrg_readl(
> - pwrcdrv->pwrc_base + PWRC_INT_MASK) | PWRC_ON_KEY_BIT,
> - pwrcdrv->pwrc_base + PWRC_INT_MASK);
> + mutex_lock(&input->mutex);
> + if (input->users)
> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
> + mutex_unlock(&input->mutex);
>
> return 0;
> }
-barry
^ permalink raw reply
* RE: [PATCH v3] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
From: Xianglong Du @ 2014-02-14 10:32 UTC (permalink / raw)
To: Barry Song, Dmitry Torokhov
Cc: Rongjun Ying, DL-SHA-WorkGroupLinux, Barry Song,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org
In-Reply-To: <CAGsJ_4xqnZAO6vpkgTEHmfp9+nbWa1qwQ4pda6B2sSrEtbV3ZA@mail.gmail.com>
Tested-by: Xianglong Du <Xianglong.Du@csr.com>
________________________________________
From: Barry Song [21cnbao@gmail.com]
Sent: Friday, February 14, 2014 16:49
To: Dmitry Torokhov
Cc: linux-input@vger.kernel.org; linux-arm-kernel@lists.infradead.org; DL-SHA-WorkGroupLinux; Xianglong Du; Rongjun Ying; Barry Song
Subject: Re: [PATCH v3] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
2014-02-14 15:57 GMT+08:00 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> Hi Barry,
>
> On Fri, Feb 14, 2014 at 11:20:01AM +0800, Barry Song wrote:
>> From: Xianglong Du <Xianglong.Du@csr.com>
>>
>> this patch adds a delayed_work to detect the untouch of onkey since HW will
>> not generate interrupt for it.
>>
>> at the same time, we move the KEY event to POWER instead of SUSPEND, which
>> will be suitable for both Android and Linux. Userspace PowerManager Daemon
>> will decide to suspend or shutdown based on how long we have touched onkey
>>
>> Signed-off-by: Xianglong Du <Xianglong.Du@csr.com>
>> Signed-off-by: Rongjun Ying <Rongjun.Ying@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> -v3: move to use custom devres action
>
> Thank you for making the changes, however it seems that we can control
> whether the device generates interrupts or not and so we can implement
> open and close methods. If patch below works then your
> cancel_delayed_work() call should go into sirfosc_pwrc_close() and we do
> not need to use devm_free_irq() not custom action.
this one looks making lots of senses. it makes sure HW will not
trigger any SW behaviour before probe() finishes, and also makes sure
HW will not trigger any SW behaviour in remove(). i'd like xianglong
to give a quick test on it.
>
> Thanks.
>
> --
> Dmitry
>
>
> Input: sirfsoc-onkey - implement open and close methods
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> We can control whetehr device generates interrupts or not so let's
> implement open and close methods of input device so that we do not do any
> processing until there are users.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/sirfsoc-onkey.c | 50 +++++++++++++++++++++++++++++-------
> 1 file changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
> index e8897c3..dc7db65 100644
> --- a/drivers/input/misc/sirfsoc-onkey.c
> +++ b/drivers/input/misc/sirfsoc-onkey.c
> @@ -49,6 +49,35 @@ static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static void sirfsoc_pwrc_toggle_interrupts(struct sirfsoc_pwrc_drvdata *pwrcdrv,
> + bool enable)
> +{
> + u32 int_mask;
> +
> + int_mask = sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK);
> + if (enable)
> + int_mask |= PWRC_ON_KEY_BIT;
> + else
> + int_mask &= ~PWRC_ON_KEY_BIT;
> + sirfsoc_rtc_iobrg_writel(int_mask, pwrcdrv->pwrc_base + PWRC_INT_MASK);
> +}
> +
> +static int sirfsoc_pwrc_open(struct input_dev *input)
> +{
> + struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
> +
> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
> +
> + return 0;
> +}
> +
> +static void sirfsoc_pwrc_close(struct input_dev *input)
> +{
> + struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
> +
> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, false);
> +}
> +
> static const struct of_device_id sirfsoc_pwrc_of_match[] = {
> { .compatible = "sirf,prima2-pwrc" },
> {},
> @@ -70,7 +99,7 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> }
>
> /*
> - * we can't use of_iomap because pwrc is not mapped in memory,
> + * We can't use of_iomap because pwrc is not mapped in memory,
> * the so-called base address is only offset in rtciobrg
> */
> error = of_property_read_u32(np, "reg", &pwrcdrv->pwrc_base);
> @@ -88,6 +117,11 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> pwrcdrv->input->phys = "pwrc/input0";
> pwrcdrv->input->evbit[0] = BIT_MASK(EV_PWR);
>
> + pwrcdrv->input->open = sirfsoc_pwrc_open;
> + pwrcdrv->input->close = sirfsoc_pwrc_close;
> +
> + input_set_drvdata(pwrcdrv->input, pwrcdrv);
> +
> irq = platform_get_irq(pdev, 0);
> error = devm_request_irq(&pdev->dev, irq,
> sirfsoc_pwrc_isr, IRQF_SHARED,
> @@ -98,11 +132,6 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
> return error;
> }
>
> - sirfsoc_rtc_iobrg_writel(
> - sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK) |
> - PWRC_ON_KEY_BIT,
> - pwrcdrv->pwrc_base + PWRC_INT_MASK);
> -
> error = input_register_device(pwrcdrv->input);
> if (error) {
> dev_err(&pdev->dev,
> @@ -129,15 +158,16 @@ static int pwrc_resume(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
> + struct input_dev *input = pwrcdrv->input;
>
> /*
> * Do not mask pwrc interrupt as we want pwrc work as a wakeup source
> * if users touch X_ONKEY_B, see arch/arm/mach-prima2/pm.c
> */
> - sirfsoc_rtc_iobrg_writel(
> - sirfsoc_rtc_iobrg_readl(
> - pwrcdrv->pwrc_base + PWRC_INT_MASK) | PWRC_ON_KEY_BIT,
> - pwrcdrv->pwrc_base + PWRC_INT_MASK);
> + mutex_lock(&input->mutex);
> + if (input->users)
> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
> + mutex_unlock(&input->mutex);
>
> return 0;
> }
-barry
To report this email as spam click https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.
^ permalink raw reply
* Re: [PATCH 1/5] HID: logitech-dj: Fix USB 3.0 issue
From: Nestor Lopez Casado @ 2014-02-14 13:35 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, Benjamin Tissoires, Andrew de los Reyes,
open list:HID CORE LAYER, linux-kernel@vger.kernel.org
In-Reply-To: <CAE7qMrpEnF3y-dVP14190y33YNidCdjtJeO9PcMVcBBJDJsYuw@mail.gmail.com>
We discussed this issue with one of the receiver firmware developers today.
We do see a clear issue when more than one DJ command (those with
report id 0x20, 0x21) are sent to the receiver in quick sequence. The
second command can be trashed if the first is still under processing,
and there is no way for the host to know when the receiver has
actually finished processing the first DJ command.
Both the first and the second commands are ACKed at the USB level.
For hidpp commands (those with report id 0x10, 0x11) there is a
response sent from the receiver to the host, and the host should wait
for the response before sending a new command.
For some DJ commands (those with report id 0x20, 0x21) there is no
response at all, as is the case for the 0x80 (switch to dj mode with
keep alive)
So the ugly sleep accounts to workaround this issue on the receiver.
There may be a cleaner workaround that could be tested (I did not test
myself, but looking at the receiver code it does seem as a plausible
solution)
Invert the order in which the commands are sent to the receiver:
First send the CMD_GET_PAIRED_DEVICES (0x81) to the receiver, and wait
to get all the device paired notifications, (the last notification is
specially tagged), then send the CMD_SWITCH (0x80).
Using this mechanism we guarantee that the receiver never sees a
second command while still processing the previous one and no explicit
sleep will be required.
For me, the ugly sleep suffices, as long as we understand the root
cause of the problem.
Cheers,
-nestor
On Fri, Feb 14, 2014 at 2:32 PM, Nestor Lopez Casado
<nlopezcasad@logitech.com> wrote:
> We discussed this issue with one of the receiver firmware developers today.
>
> We do see a clear issue when more than one DJ command (those with report id
> 0x20, 0x21) are sent to the receiver in quick sequence. The second command
> can be trashed if the first is still under processing, and there is no way
> for the host to know when the receiver has actually finished processing the
> first DJ command.
>
> Both the first and the second commands are ACKed at the USB level.
>
> For hidpp commands (those with report id 0x10, 0x11) there is a response
> sent from the receiver to the host, and the host should wait for the
> response before sending a new command.
>
> For some DJ commands (those with report id 0x20, 0x21) there is no response
> at all, as is the case for the 0x80 (switch to dj mode with keep alive)
>
> So the ugly sleep accounts to workaround this issue on the receiver.
>
> There may be a cleaner workaround that could be tested (I did not test
> myself, but looking at the receiver code it does seem as a plausible
> solution)
>
> Invert the order in which the commands are sent to the receiver:
>
> First send the CMD_GET_PAIRED_DEVICES (0x81) to the receiver, and wait to
> get all the device paired notifications, (the last notification is specially
> tagged), then send the CMD_SWITCH (0x80).
> Using this mechanism we guarantee that the receiver never sees a second
> command while still processing the previous one and no explicit sleep will
> be required.
>
> For me, the ugly sleep suffices, as long as we understand the root cause of
> the problem.
>
> Cheers,
> -nestor
>
>
> On Fri, Jan 17, 2014 at 9:48 AM, Nestor Lopez Casado
> <nlopezcasad@logitech.com> wrote:
>>
>> On Thu, Jan 16, 2014 at 10:50 PM, Jiri Kosina <jkosina@suse.cz> wrote:
>> > On Wed, 8 Jan 2014, Benjamin Tissoires wrote:
>> >
>> >> From: Benjamin Tisssoires <benjamin.tissoires@redhat.com>
>> >>
>> >> This fix (not very clean though) should fix the long time USB3
>> >> issue that was spotted last year. The rational has been given by
>> >> Hans de Goede:
>> >
>> > Man this is so ugly ... makes one wonder how come that other OSes don't
>> > suffer from this. Are they really *that much* slower? :)
>> >
>> > I have applied this (and just this) one for 3.14.
>>
>> This is clearly a workaround for an underlying bug.
>> Most probably the issue is in the receiver firmware, and the bug
>> happens to become apparent easier when it is plugged in a usb 3.0
>> port.
>> We are experiencing some issues when the receiver is plugged to usb
>> 3.0 ports in other OSes as well, these could also be explained by the
>> same phenomenon of vendor requests being missed by the receiver. But
>> we dont have a clear understanding of the problem yet.
>>
>> >
>> > Thanks,
>> >
>> > --
>> > Jiri Kosina
>> > SUSE Labs
>> Cheers,
>> -nestor
>
>
^ permalink raw reply
* Re: [PATCH v3] input: sirfsoc-onkey - report onkey untouch event by detecting pin status
From: Barry Song @ 2014-02-14 13:41 UTC (permalink / raw)
To: Xianglong Du
Cc: Dmitry Torokhov, linux-input@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, DL-SHA-WorkGroupLinux,
Rongjun Ying, Barry Song
In-Reply-To: <847BC5012124A94BB5CA1B26E022CCA3012BAFDC27@SHAASIEXM01.ASIA.ROOT.PRI>
>> Input: sirfsoc-onkey - implement open and close methods
>>
>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> We can control whetehr device generates interrupts or not so let's
>> implement open and close methods of input device so that we do not do any
>> processing until there are users.
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Tested-by: Xianglong Du <Xianglong.Du@csr.com>
Dmitry, will you push this one to your tree so that i can rebase others?
>> ---
>> drivers/input/misc/sirfsoc-onkey.c | 50 +++++++++++++++++++++++++++++-------
>> 1 file changed, 40 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/input/misc/sirfsoc-onkey.c b/drivers/input/misc/sirfsoc-onkey.c
>> index e8897c3..dc7db65 100644
>> --- a/drivers/input/misc/sirfsoc-onkey.c
>> +++ b/drivers/input/misc/sirfsoc-onkey.c
>> @@ -49,6 +49,35 @@ static irqreturn_t sirfsoc_pwrc_isr(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> +static void sirfsoc_pwrc_toggle_interrupts(struct sirfsoc_pwrc_drvdata *pwrcdrv,
>> + bool enable)
>> +{
>> + u32 int_mask;
>> +
>> + int_mask = sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK);
>> + if (enable)
>> + int_mask |= PWRC_ON_KEY_BIT;
>> + else
>> + int_mask &= ~PWRC_ON_KEY_BIT;
>> + sirfsoc_rtc_iobrg_writel(int_mask, pwrcdrv->pwrc_base + PWRC_INT_MASK);
>> +}
>> +
>> +static int sirfsoc_pwrc_open(struct input_dev *input)
>> +{
>> + struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
>> +
>> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
>> +
>> + return 0;
>> +}
>> +
>> +static void sirfsoc_pwrc_close(struct input_dev *input)
>> +{
>> + struct sirfsoc_pwrc_drvdata *pwrcdrv = input_get_drvdata(input);
>> +
>> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, false);
>> +}
>> +
>> static const struct of_device_id sirfsoc_pwrc_of_match[] = {
>> { .compatible = "sirf,prima2-pwrc" },
>> {},
>> @@ -70,7 +99,7 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
>> }
>>
>> /*
>> - * we can't use of_iomap because pwrc is not mapped in memory,
>> + * We can't use of_iomap because pwrc is not mapped in memory,
>> * the so-called base address is only offset in rtciobrg
>> */
>> error = of_property_read_u32(np, "reg", &pwrcdrv->pwrc_base);
>> @@ -88,6 +117,11 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
>> pwrcdrv->input->phys = "pwrc/input0";
>> pwrcdrv->input->evbit[0] = BIT_MASK(EV_PWR);
>>
>> + pwrcdrv->input->open = sirfsoc_pwrc_open;
>> + pwrcdrv->input->close = sirfsoc_pwrc_close;
>> +
>> + input_set_drvdata(pwrcdrv->input, pwrcdrv);
>> +
>> irq = platform_get_irq(pdev, 0);
>> error = devm_request_irq(&pdev->dev, irq,
>> sirfsoc_pwrc_isr, IRQF_SHARED,
>> @@ -98,11 +132,6 @@ static int sirfsoc_pwrc_probe(struct platform_device *pdev)
>> return error;
>> }
>>
>> - sirfsoc_rtc_iobrg_writel(
>> - sirfsoc_rtc_iobrg_readl(pwrcdrv->pwrc_base + PWRC_INT_MASK) |
>> - PWRC_ON_KEY_BIT,
>> - pwrcdrv->pwrc_base + PWRC_INT_MASK);
>> -
>> error = input_register_device(pwrcdrv->input);
>> if (error) {
>> dev_err(&pdev->dev,
>> @@ -129,15 +158,16 @@ static int pwrc_resume(struct device *dev)
>> {
>> struct platform_device *pdev = to_platform_device(dev);
>> struct sirfsoc_pwrc_drvdata *pwrcdrv = platform_get_drvdata(pdev);
>> + struct input_dev *input = pwrcdrv->input;
>>
>> /*
>> * Do not mask pwrc interrupt as we want pwrc work as a wakeup source
>> * if users touch X_ONKEY_B, see arch/arm/mach-prima2/pm.c
>> */
>> - sirfsoc_rtc_iobrg_writel(
>> - sirfsoc_rtc_iobrg_readl(
>> - pwrcdrv->pwrc_base + PWRC_INT_MASK) | PWRC_ON_KEY_BIT,
>> - pwrcdrv->pwrc_base + PWRC_INT_MASK);
>> + mutex_lock(&input->mutex);
>> + if (input->users)
>> + sirfsoc_pwrc_toggle_interrupts(pwrcdrv, true);
>> + mutex_unlock(&input->mutex);
>>
>> return 0;
>> }
>
> -barry
-barry
^ permalink raw reply
* [PATCH] Input: Support in the elantech driver of the trackpoint present on for instance Lenovo L530
From: Ulrik De Bie @ 2014-02-14 20:51 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, Dmitry Torokhov, David Herrmann, Jonathan Aquilina,
Ulrik De Bie
Sorry it took me some time to have an email addres that would allow clean
mails. Below you can find the updated patch previously sent on linux-input
mailinglist. David Herrman asked to resent this patch as a proper git patch so
here it is now.
It also available on https://github.com/ulrikdb/linux.git elantech_trackpoint
from commit 38dbfb59d1175ef458d006556061adeaa8751b72 (Linus 3.14-rc1) up to
d69e103a35c944721966105790d14adf79098a4c
Please when responding please send either to linux-input or put Ulrik De Bie <ulrik.debie-os@e2big.org> in CC:, Thanks.
The Lenovo L530 trackpoint does not work out of the box. It gives sync errors
as shown below when the trackpoint or trackpoint mouse buttons are pressed and
no input is received by userspace:
[ 29.010641] psmouse serio1: Touchpad at isa0060/serio1/input0 lost sync at byte 6
The touchpad does work.
The alternative is to do a downgrade to generic ps/2 mouse (modprobe psmouse proto=bare)
but this has the disadvantage that touchpad can't be disabled (I want trackpoint, not touchpad).
With this patch, the trackpoint is provided as another input device; currently called 'TPPS/2 IBM TrackPoint'
The trackpoint now succesfully works and I can disable the touchpad with synclient TouchPadOff=1
The patch will also output messages that do not follow the expected pattern.
In the mean time I've seen 2 unknown packets occasionally:
0x04 , 0x00 , 0x00 , 0x00 , 0x00 , 0x00
0x00 , 0x00 , 0x00 , 0x02 , 0x00 , 0x00
I don't know what those are for, but they can be safely ignored.
Currently all packets that are not known to v3 touchpad and where packet[3] (the fourth byte) lowest
nibble is 6 are now recognized as PACKET_TRACKPOINT and processed by the new elantech_report_trackpoint.
This has been verified to work on a laptop where the touchpad/trackpoint combined identify themselves as:
psmouse serio1: elantech: assuming hardware version 3 (with firmware version 0x350f02)
psmouse serio1: elantech: Synaptics capabilities query result 0xb9, 0x15, 0x0c.
Since I can't send clean email from yahoo, I've switched to a different email address: ulrik.debie-os@e2big.org
Signed-off-by: Ulrik De Bie <ulrik_opensource-kernel@yahoo.com>
Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
Signed-off-by: Ulrik De Bie <ulrik.debie-os@e2big.org>
---
drivers/input/mouse/elantech.c | 78 +++++++++++++++++++++++++++++++++++++++++-
drivers/input/mouse/elantech.h | 3 ++
2 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index ef1cf52..21d693b 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -402,6 +402,54 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
input_sync(dev);
}
+static void elantech_report_trackpoint(struct psmouse *psmouse,
+ int packet_type)
+{
+ /* byte 0: 0 0 ~sx ~sy 0 M R L */
+ /* byte 1: sx 0 0 0 0 0 0 0 */
+ /* byte 2: sy 0 0 0 0 0 0 0 */
+ /* byte 3: 0 0 sy sx 0 1 1 0 */
+ /* byte 4: x7 x6 x5 x4 x3 x2 x1 x0 */
+ /* byte 5: y7 y6 y5 y4 y3 y2 y1 y0 */
+
+ /*
+ * x and y are written in two's complement spread
+ * over 9 bits with sx/sy the relative top bit and
+ * x7..x0 and y7..y0 the lower bits.
+ * The sign of y is opposite to what the input driver
+ * expects for a relative movement
+ */
+
+ struct elantech_data *etd = psmouse->private;
+ struct input_dev *dev2 = etd->dev2;
+ unsigned char *packet = psmouse->packet;
+ int x, y;
+ input_report_key(dev2, BTN_LEFT, packet[0] & 0x01);
+ input_report_key(dev2, BTN_RIGHT, packet[0] & 0x02);
+ input_report_key(dev2, BTN_MIDDLE, packet[0] & 0x04);
+ x = (s32) ((u32) ((packet[1] & 0x80) ? 0UL : 0xFFFFFF00UL) | (u32)
+ packet[4]);
+ y = -(s32) ((u32) ((packet[2] & 0x80) ? 0UL : 0xFFFFFF00UL) | (u32)
+ packet[5]);
+ input_report_rel(dev2, REL_X, x);
+ input_report_rel(dev2, REL_Y, y);
+ switch ((((u32) packet[0] & 0xF8) << 24) | ((u32) packet[1] << 16)
+ | (u32) packet[2] << 8 | (u32) packet[3]) {
+ case 0x00808036UL:
+ case 0x10008026UL:
+ case 0x20800016UL:
+ case 0x30000006UL:
+ break;
+ default:
+ /* Dump unexpected packet sequences if debug=1 (default) */
+ if (etd->debug == 1)
+ elantech_packet_dump(psmouse);
+ break;
+ }
+
+ input_sync(dev2);
+}
+
/*
* Interpret complete data packets and report absolute mode input events for
* hardware version 3. (12 byte packets for two fingers)
@@ -700,8 +748,11 @@ static int elantech_packet_check_v3(struct psmouse *psmouse)
if ((packet[0] & 0x0c) == 0x0c && (packet[3] & 0xce) == 0x0c)
return PACKET_V3_TAIL;
+ if ((packet[3]&0x0f) == 0x06)
+ return PACKET_TRACKPOINT;
}
+
return PACKET_UNKNOWN;
}
@@ -783,7 +834,10 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
if (packet_type == PACKET_UNKNOWN)
return PSMOUSE_BAD_DATA;
- elantech_report_absolute_v3(psmouse, packet_type);
+ if (packet_type == PACKET_TRACKPOINT)
+ elantech_report_trackpoint(psmouse, packet_type);
+ else
+ elantech_report_absolute_v3(psmouse, packet_type);
break;
case 4:
@@ -1400,10 +1454,15 @@ int elantech_init(struct psmouse *psmouse)
struct elantech_data *etd;
int i, error;
unsigned char param[3];
+ struct input_dev *dev2;
psmouse->private = etd = kzalloc(sizeof(struct elantech_data), GFP_KERNEL);
if (!etd)
return -ENOMEM;
+ dev2 = input_allocate_device();
+ if (!dev2)
+ goto init_fail;
+ etd->dev2 = dev2;
psmouse_reset(psmouse);
@@ -1463,9 +1522,26 @@ int elantech_init(struct psmouse *psmouse)
psmouse->reconnect = elantech_reconnect;
psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
+ snprintf(etd->phys, sizeof(etd->phys), "%s/input1",
+ psmouse->ps2dev.serio->phys);
+ dev2->phys = etd->phys;
+ dev2->name = "TPPS/2 IBM TrackPoint";
+ dev2->id.bustype = BUS_I8042;
+ dev2->id.vendor = 0x0002;
+ dev2->id.product = PSMOUSE_ELANTECH;
+ dev2->id.version = 0x0000;
+ dev2->dev.parent = &psmouse->ps2dev.serio->dev;
+ dev2->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+ dev2->relbit[BIT_WORD(REL_X)] = BIT_MASK(REL_X) | BIT_MASK(REL_Y);
+ dev2->keybit[BIT_WORD(BTN_LEFT)] =
+ BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_MIDDLE) | BIT_MASK(BTN_RIGHT);
+
+ if (input_register_device(etd->dev2))
+ goto init_fail;
return 0;
init_fail:
+ input_free_device(dev2);
kfree(etd);
return -1;
}
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 036a04a..27cf191 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -94,6 +94,7 @@
#define PACKET_V4_HEAD 0x05
#define PACKET_V4_MOTION 0x06
#define PACKET_V4_STATUS 0x07
+#define PACKET_TRACKPOINT 0x08
/*
* track up to 5 fingers for v4 hardware
@@ -114,6 +115,8 @@ struct finger_pos {
};
struct elantech_data {
+ struct input_dev *dev2; /* Relative device */
+ char phys[32];
unsigned char reg_07;
unsigned char reg_10;
unsigned char reg_11;
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH 03/11] Input: synaptics-rmi4 - do not update configuration in rmi_f01_probe()
From: Christopher Heiny @ 2014-02-14 23:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
Linus Walleij, Benjamin Tissoires, Courtney Cavin, Linux Input,
Linux Kernel
In-Reply-To: <20140213215432.GA6225@core.coreip.homeip.net>
On 02/13/2014 01:54 PM, Dmitry Torokhov wrote:
> On Thu, Feb 13, 2014 at 11:23:44AM -0800, Christopher Heiny wrote:
>> >On 02/12/2014 09:27 PM, Dmitry Torokhov wrote:
>>> > >Do not write configuration data in probe(), we have config() for that.
>> >
>> >Then we should call config() in rmi_function_probe() to ensure that
>> >any platform data or device tree configuration settings get written
>> >to the device.
>
> Well, yes, we may elect to update device configuration in probe, but
> then we should not be doing that 2nd time in ->config(). We shoudl pick
> either one or another.
But as the code currently stands, config() is only called when a device
reset is detected, not during initialization. So if there's platform
specific configuration data that needs to be written to a function, it
won't get written until after a device reset occurs, which might never
happen. So either we need to write that data (or call config()) in each
function's probe(), or in rmi_function_probe().
^ 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