Re: [PATCH v3 01/18] Input: synaptics-rmi4 - Move IRQ handling to rmi_driver

2016-11-08 Thread Dmitry Torokhov 
On Thu, Oct 13, 2016 at 05:50:55PM +0200, Benjamin Tissoires wrote:
> From: Bjorn Andersson 
> 
> The attn IRQ is related to the chip, rather than the transport, so move
> all handling of interrupts to the core driver. This also makes sure that
> there are no races between interrupts and availability of the resources
> used by the core driver.
> 
> Signed-off-by: Bjorn Andersson 
> Signed-off-by: Benjamin Tissoires 

Applied, thank you.

> 
> ---
> 
> new in v3
> 
> changes since Bjorn's submission:
> - call disable_irq on remove()
> ---
>  drivers/input/rmi4/rmi_driver.c | 73 +---
>  drivers/input/rmi4/rmi_i2c.c| 74 
> +++--
>  drivers/input/rmi4/rmi_spi.c| 72 +++
>  include/linux/rmi.h |  7 ++--
>  4 files changed, 83 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index c83bce8..cf780ef 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -134,7 +135,7 @@ static void process_one_interrupt(struct rmi_driver_data 
> *data,
>   }
>  }
>  
> -int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
> +static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>  {
>   struct rmi_driver_data *data = dev_get_drvdata(_dev->dev);
>   struct device *dev = _dev->dev;
> @@ -179,7 +180,42 @@ int rmi_process_interrupt_requests(struct rmi_device 
> *rmi_dev)
>  
>   return 0;
>  }
> -EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
> +
> +static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
> +{
> + struct rmi_device *rmi_dev = dev_id;
> + int ret;
> +
> + ret = rmi_process_interrupt_requests(rmi_dev);
> + if (ret)
> + rmi_dbg(RMI_DEBUG_CORE, _dev->dev,
> + "Failed to process interrupt request: %d\n", ret);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int rmi_irq_init(struct rmi_device *rmi_dev)
> +{
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq_flags = irq_get_trigger_type(pdata->irq);
> + int ret;
> +
> + if (!irq_flags)
> + irq_flags = IRQF_TRIGGER_LOW;
> +
> + ret = devm_request_threaded_irq(_dev->dev, pdata->irq, NULL,
> + rmi_irq_fn, irq_flags | IRQF_ONESHOT,
> + dev_name(rmi_dev->xport->dev),
> + rmi_dev);
> + if (ret < 0) {
> + dev_warn(_dev->dev, "Failed to register interrupt %d\n",
> +  pdata->irq);
> +
> + return ret;
> + }
> +
> + return 0;
> +}
>  
>  static int suspend_one_function(struct rmi_function *fn)
>  {
> @@ -787,8 +823,10 @@ err_put_fn:
>   return error;
>  }
>  
> -int rmi_driver_suspend(struct rmi_device *rmi_dev)
> +int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
>  {
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq = pdata->irq;
>   int retval = 0;
>  
>   retval = rmi_suspend_functions(rmi_dev);
> @@ -796,14 +834,33 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev)
>   dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
>   retval);
>  
> + disable_irq(irq);
> + if (enable_wake && device_may_wakeup(rmi_dev->xport->dev)) {
> + retval = enable_irq_wake(irq);
> + if (!retval)
> + dev_warn(_dev->dev,
> +  "Failed to enable irq for wake: %d\n",
> +  retval);
> + }
>   return retval;
>  }
>  EXPORT_SYMBOL_GPL(rmi_driver_suspend);
>  
> -int rmi_driver_resume(struct rmi_device *rmi_dev)
> +int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
>  {
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq = pdata->irq;
>   int retval;
>  
> + enable_irq(irq);
> + if (clear_wake && device_may_wakeup(rmi_dev->xport->dev)) {
> + retval = disable_irq_wake(irq);
> + if (!retval)
> + dev_warn(_dev->dev,
> +  "Failed to disable irq for wake: %d\n",
> +  retval);
> + }
> +
>   retval = rmi_resume_functions(rmi_dev);
>   if (retval)
>   dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
> @@ -816,6 +873,10 @@ EXPORT_SYMBOL_GPL(rmi_driver_resume);
>  static int rmi_driver_remove(struct device *dev)
>  {
>   struct rmi_device *rmi_dev = to_rmi_device(dev);
> + struct rmi_device_platform_data *pdata =

Re: [PATCH v3 01/18] Input: synaptics-rmi4 - Move IRQ handling to rmi_driver

2016-11-08 Thread Dmitry Torokhov 
On Thu, Oct 13, 2016 at 05:50:55PM +0200, Benjamin Tissoires wrote:
> From: Bjorn Andersson 
> 
> The attn IRQ is related to the chip, rather than the transport, so move
> all handling of interrupts to the core driver. This also makes sure that
> there are no races between interrupts and availability of the resources
> used by the core driver.
> 
> Signed-off-by: Bjorn Andersson 
> Signed-off-by: Benjamin Tissoires 

Applied, thank you.

> 
> ---
> 
> new in v3
> 
> changes since Bjorn's submission:
> - call disable_irq on remove()
> ---
>  drivers/input/rmi4/rmi_driver.c | 73 +---
>  drivers/input/rmi4/rmi_i2c.c| 74 
> +++--
>  drivers/input/rmi4/rmi_spi.c| 72 +++
>  include/linux/rmi.h |  7 ++--
>  4 files changed, 83 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index c83bce8..cf780ef 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -134,7 +135,7 @@ static void process_one_interrupt(struct rmi_driver_data 
> *data,
>   }
>  }
>  
> -int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
> +static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
>  {
>   struct rmi_driver_data *data = dev_get_drvdata(_dev->dev);
>   struct device *dev = _dev->dev;
> @@ -179,7 +180,42 @@ int rmi_process_interrupt_requests(struct rmi_device 
> *rmi_dev)
>  
>   return 0;
>  }
> -EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
> +
> +static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
> +{
> + struct rmi_device *rmi_dev = dev_id;
> + int ret;
> +
> + ret = rmi_process_interrupt_requests(rmi_dev);
> + if (ret)
> + rmi_dbg(RMI_DEBUG_CORE, _dev->dev,
> + "Failed to process interrupt request: %d\n", ret);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int rmi_irq_init(struct rmi_device *rmi_dev)
> +{
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq_flags = irq_get_trigger_type(pdata->irq);
> + int ret;
> +
> + if (!irq_flags)
> + irq_flags = IRQF_TRIGGER_LOW;
> +
> + ret = devm_request_threaded_irq(_dev->dev, pdata->irq, NULL,
> + rmi_irq_fn, irq_flags | IRQF_ONESHOT,
> + dev_name(rmi_dev->xport->dev),
> + rmi_dev);
> + if (ret < 0) {
> + dev_warn(_dev->dev, "Failed to register interrupt %d\n",
> +  pdata->irq);
> +
> + return ret;
> + }
> +
> + return 0;
> +}
>  
>  static int suspend_one_function(struct rmi_function *fn)
>  {
> @@ -787,8 +823,10 @@ err_put_fn:
>   return error;
>  }
>  
> -int rmi_driver_suspend(struct rmi_device *rmi_dev)
> +int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
>  {
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq = pdata->irq;
>   int retval = 0;
>  
>   retval = rmi_suspend_functions(rmi_dev);
> @@ -796,14 +834,33 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev)
>   dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
>   retval);
>  
> + disable_irq(irq);
> + if (enable_wake && device_may_wakeup(rmi_dev->xport->dev)) {
> + retval = enable_irq_wake(irq);
> + if (!retval)
> + dev_warn(_dev->dev,
> +  "Failed to enable irq for wake: %d\n",
> +  retval);
> + }
>   return retval;
>  }
>  EXPORT_SYMBOL_GPL(rmi_driver_suspend);
>  
> -int rmi_driver_resume(struct rmi_device *rmi_dev)
> +int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
>  {
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq = pdata->irq;
>   int retval;
>  
> + enable_irq(irq);
> + if (clear_wake && device_may_wakeup(rmi_dev->xport->dev)) {
> + retval = disable_irq_wake(irq);
> + if (!retval)
> + dev_warn(_dev->dev,
> +  "Failed to disable irq for wake: %d\n",
> +  retval);
> + }
> +
>   retval = rmi_resume_functions(rmi_dev);
>   if (retval)
>   dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
> @@ -816,6 +873,10 @@ EXPORT_SYMBOL_GPL(rmi_driver_resume);
>  static int rmi_driver_remove(struct device *dev)
>  {
>   struct rmi_device *rmi_dev = to_rmi_device(dev);
> + struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
> + int irq = pdata->irq;
> +
> + disable_irq(irq);
>  
>

[PATCH v3 01/18] Input: synaptics-rmi4 - Move IRQ handling to rmi_driver

2016-10-13 Thread Benjamin Tissoires 
From: Bjorn Andersson 

The attn IRQ is related to the chip, rather than the transport, so move
all handling of interrupts to the core driver. This also makes sure that
there are no races between interrupts and availability of the resources
used by the core driver.

Signed-off-by: Bjorn Andersson 
Signed-off-by: Benjamin Tissoires 

---

new in v3

changes since Bjorn's submission:
- call disable_irq on remove()
---
 drivers/input/rmi4/rmi_driver.c | 73 +---
 drivers/input/rmi4/rmi_i2c.c| 74 +++--
 drivers/input/rmi4/rmi_spi.c| 72 +++
 include/linux/rmi.h |  7 ++--
 4 files changed, 83 insertions(+), 143 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index c83bce8..cf780ef 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -134,7 +135,7 @@ static void process_one_interrupt(struct rmi_driver_data 
*data,
}
 }
 
-int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
+static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 {
struct rmi_driver_data *data = dev_get_drvdata(_dev->dev);
struct device *dev = _dev->dev;
@@ -179,7 +180,42 @@ int rmi_process_interrupt_requests(struct rmi_device 
*rmi_dev)
 
return 0;
 }
-EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
+
+static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
+{
+   struct rmi_device *rmi_dev = dev_id;
+   int ret;
+
+   ret = rmi_process_interrupt_requests(rmi_dev);
+   if (ret)
+   rmi_dbg(RMI_DEBUG_CORE, _dev->dev,
+   "Failed to process interrupt request: %d\n", ret);
+
+   return IRQ_HANDLED;
+}
+
+static int rmi_irq_init(struct rmi_device *rmi_dev)
+{
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq_flags = irq_get_trigger_type(pdata->irq);
+   int ret;
+
+   if (!irq_flags)
+   irq_flags = IRQF_TRIGGER_LOW;
+
+   ret = devm_request_threaded_irq(_dev->dev, pdata->irq, NULL,
+   rmi_irq_fn, irq_flags | IRQF_ONESHOT,
+   dev_name(rmi_dev->xport->dev),
+   rmi_dev);
+   if (ret < 0) {
+   dev_warn(_dev->dev, "Failed to register interrupt %d\n",
+pdata->irq);
+
+   return ret;
+   }
+
+   return 0;
+}
 
 static int suspend_one_function(struct rmi_function *fn)
 {
@@ -787,8 +823,10 @@ err_put_fn:
return error;
 }
 
-int rmi_driver_suspend(struct rmi_device *rmi_dev)
+int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
 {
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq = pdata->irq;
int retval = 0;
 
retval = rmi_suspend_functions(rmi_dev);
@@ -796,14 +834,33 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev)
dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
retval);
 
+   disable_irq(irq);
+   if (enable_wake && device_may_wakeup(rmi_dev->xport->dev)) {
+   retval = enable_irq_wake(irq);
+   if (!retval)
+   dev_warn(_dev->dev,
+"Failed to enable irq for wake: %d\n",
+retval);
+   }
return retval;
 }
 EXPORT_SYMBOL_GPL(rmi_driver_suspend);
 
-int rmi_driver_resume(struct rmi_device *rmi_dev)
+int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
 {
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq = pdata->irq;
int retval;
 
+   enable_irq(irq);
+   if (clear_wake && device_may_wakeup(rmi_dev->xport->dev)) {
+   retval = disable_irq_wake(irq);
+   if (!retval)
+   dev_warn(_dev->dev,
+"Failed to disable irq for wake: %d\n",
+retval);
+   }
+
retval = rmi_resume_functions(rmi_dev);
if (retval)
dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
@@ -816,6 +873,10 @@ EXPORT_SYMBOL_GPL(rmi_driver_resume);
 static int rmi_driver_remove(struct device *dev)
 {
struct rmi_device *rmi_dev = to_rmi_device(dev);
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq = pdata->irq;
+
+   disable_irq(irq);
 
rmi_free_function_list(rmi_dev);
 
@@ -1004,6 +1065,10 @@ static int rmi_driver_probe(struct device *dev)
}
}
 
+   retval = rmi_irq_init(rmi_dev);
+   if

[PATCH v3 01/18] Input: synaptics-rmi4 - Move IRQ handling to rmi_driver

2016-10-13 Thread Benjamin Tissoires 
From: Bjorn Andersson 

The attn IRQ is related to the chip, rather than the transport, so move
all handling of interrupts to the core driver. This also makes sure that
there are no races between interrupts and availability of the resources
used by the core driver.

Signed-off-by: Bjorn Andersson 
Signed-off-by: Benjamin Tissoires 

---

new in v3

changes since Bjorn's submission:
- call disable_irq on remove()
---
 drivers/input/rmi4/rmi_driver.c | 73 +---
 drivers/input/rmi4/rmi_i2c.c| 74 +++--
 drivers/input/rmi4/rmi_spi.c| 72 +++
 include/linux/rmi.h |  7 ++--
 4 files changed, 83 insertions(+), 143 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index c83bce8..cf780ef 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -134,7 +135,7 @@ static void process_one_interrupt(struct rmi_driver_data 
*data,
}
 }
 
-int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
+static int rmi_process_interrupt_requests(struct rmi_device *rmi_dev)
 {
struct rmi_driver_data *data = dev_get_drvdata(_dev->dev);
struct device *dev = _dev->dev;
@@ -179,7 +180,42 @@ int rmi_process_interrupt_requests(struct rmi_device 
*rmi_dev)
 
return 0;
 }
-EXPORT_SYMBOL_GPL(rmi_process_interrupt_requests);
+
+static irqreturn_t rmi_irq_fn(int irq, void *dev_id)
+{
+   struct rmi_device *rmi_dev = dev_id;
+   int ret;
+
+   ret = rmi_process_interrupt_requests(rmi_dev);
+   if (ret)
+   rmi_dbg(RMI_DEBUG_CORE, _dev->dev,
+   "Failed to process interrupt request: %d\n", ret);
+
+   return IRQ_HANDLED;
+}
+
+static int rmi_irq_init(struct rmi_device *rmi_dev)
+{
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq_flags = irq_get_trigger_type(pdata->irq);
+   int ret;
+
+   if (!irq_flags)
+   irq_flags = IRQF_TRIGGER_LOW;
+
+   ret = devm_request_threaded_irq(_dev->dev, pdata->irq, NULL,
+   rmi_irq_fn, irq_flags | IRQF_ONESHOT,
+   dev_name(rmi_dev->xport->dev),
+   rmi_dev);
+   if (ret < 0) {
+   dev_warn(_dev->dev, "Failed to register interrupt %d\n",
+pdata->irq);
+
+   return ret;
+   }
+
+   return 0;
+}
 
 static int suspend_one_function(struct rmi_function *fn)
 {
@@ -787,8 +823,10 @@ err_put_fn:
return error;
 }
 
-int rmi_driver_suspend(struct rmi_device *rmi_dev)
+int rmi_driver_suspend(struct rmi_device *rmi_dev, bool enable_wake)
 {
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq = pdata->irq;
int retval = 0;
 
retval = rmi_suspend_functions(rmi_dev);
@@ -796,14 +834,33 @@ int rmi_driver_suspend(struct rmi_device *rmi_dev)
dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
retval);
 
+   disable_irq(irq);
+   if (enable_wake && device_may_wakeup(rmi_dev->xport->dev)) {
+   retval = enable_irq_wake(irq);
+   if (!retval)
+   dev_warn(_dev->dev,
+"Failed to enable irq for wake: %d\n",
+retval);
+   }
return retval;
 }
 EXPORT_SYMBOL_GPL(rmi_driver_suspend);
 
-int rmi_driver_resume(struct rmi_device *rmi_dev)
+int rmi_driver_resume(struct rmi_device *rmi_dev, bool clear_wake)
 {
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq = pdata->irq;
int retval;
 
+   enable_irq(irq);
+   if (clear_wake && device_may_wakeup(rmi_dev->xport->dev)) {
+   retval = disable_irq_wake(irq);
+   if (!retval)
+   dev_warn(_dev->dev,
+"Failed to disable irq for wake: %d\n",
+retval);
+   }
+
retval = rmi_resume_functions(rmi_dev);
if (retval)
dev_warn(_dev->dev, "Failed to suspend functions: %d\n",
@@ -816,6 +873,10 @@ EXPORT_SYMBOL_GPL(rmi_driver_resume);
 static int rmi_driver_remove(struct device *dev)
 {
struct rmi_device *rmi_dev = to_rmi_device(dev);
+   struct rmi_device_platform_data *pdata = rmi_get_platform_data(rmi_dev);
+   int irq = pdata->irq;
+
+   disable_irq(irq);
 
rmi_free_function_list(rmi_dev);
 
@@ -1004,6 +1065,10 @@ static int rmi_driver_probe(struct device *dev)
}
}
 
+   retval = rmi_irq_init(rmi_dev);
+   if (retval < 0)
+   goto err_destroy_functions;
+
if