[resend,2/3] Input: synaptics-rmi4 - add dummy F54 attention handler
diff mbox series

Message ID 20191104114454.10500-2-l.stach@pengutronix.de
State Under Review
Headers show
Series
  • [resend,1/3] Input: synaptics-rmi4 - fix video buffer size
Related show

Commit Message

Lucas Stach Nov. 4, 2019, 11:44 a.m. UTC
F54 is IRQ capable, even if it is not used in the current driver
implementation. The common driver code in rmi_create_function_irq always
installs a irq handler for functions that are IRQ capable. Without a
assigned attention handler, this means a NULL pointer being passed as
the nested IRQ handler. This seems to work with some architecture
implementations, but crashes on others like ARM64.

Don't rely on implementation defined behavior and actually install
a proper attention handler.

Fixes: 24d28e4f1271 ("Input: synaptics-rmi4 - convert irq distribution
                      to irq_domain")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/input/rmi4/rmi_f54.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Dmitry Torokhov Nov. 5, 2019, 12:03 a.m. UTC | #1
On Mon, Nov 04, 2019 at 12:44:53PM +0100, Lucas Stach wrote:
> F54 is IRQ capable, even if it is not used in the current driver
> implementation. The common driver code in rmi_create_function_irq always
> installs a irq handler for functions that are IRQ capable. Without a
> assigned attention handler, this means a NULL pointer being passed as
> the nested IRQ handler. This seems to work with some architecture
> implementations, but crashes on others like ARM64.
> 
> Don't rely on implementation defined behavior and actually install
> a proper attention handler.

Instead of supplying dummy IRQ handler, can't we simply disable relevant
interrupts bits?

> 
> Fixes: 24d28e4f1271 ("Input: synaptics-rmi4 - convert irq distribution
>                       to irq_domain")
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/input/rmi4/rmi_f54.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 4841354af0d7..22390e89c680 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -732,6 +732,11 @@ static void rmi_f54_remove(struct rmi_function *fn)
>  	v4l2_device_unregister(&f54->v4l2);
>  }
>  
> +static irqreturn_t rmi_f54_attention(int irq, void *ctx)
> +{
> +	return IRQ_HANDLED;
> +}
> +
>  struct rmi_function_handler rmi_f54_handler = {
>  	.driver = {
>  		.name = F54_NAME,
> @@ -740,4 +745,5 @@ struct rmi_function_handler rmi_f54_handler = {
>  	.probe = rmi_f54_probe,
>  	.config = rmi_f54_config,
>  	.remove = rmi_f54_remove,
> +	.attention = rmi_f54_attention,
>  };
> -- 
> 2.20.1
> 

Thanks.
Lucas Stach Nov. 5, 2019, 11:46 a.m. UTC | #2
Hi Dmitry,

On Mo, 2019-11-04 at 16:03 -0800, Dmitry Torokhov wrote:
> On Mon, Nov 04, 2019 at 12:44:53PM +0100, Lucas Stach wrote:
> > F54 is IRQ capable, even if it is not used in the current driver
> > implementation. The common driver code in rmi_create_function_irq always
> > installs a irq handler for functions that are IRQ capable. Without a
> > assigned attention handler, this means a NULL pointer being passed as
> > the nested IRQ handler. This seems to work with some architecture
> > implementations, but crashes on others like ARM64.
> > 
> > Don't rely on implementation defined behavior and actually install
> > a proper attention handler.
> 
> Instead of supplying dummy IRQ handler, can't we simply disable relevant
> interrupts bits?

Don't know why I didn't try this last time. I vaguely remember that not
enabling the IRQs didn't help.
I just retested and it seems the IRQs are enabled by default, so we
need to actively disable them. I just sent out a patch which does
exactly this.

Regards,
Lucas

Patch
diff mbox series

diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 4841354af0d7..22390e89c680 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -732,6 +732,11 @@  static void rmi_f54_remove(struct rmi_function *fn)
 	v4l2_device_unregister(&f54->v4l2);
 }
 
+static irqreturn_t rmi_f54_attention(int irq, void *ctx)
+{
+	return IRQ_HANDLED;
+}
+
 struct rmi_function_handler rmi_f54_handler = {
 	.driver = {
 		.name = F54_NAME,
@@ -740,4 +745,5 @@  struct rmi_function_handler rmi_f54_handler = {
 	.probe = rmi_f54_probe,
 	.config = rmi_f54_config,
 	.remove = rmi_f54_remove,
+	.attention = rmi_f54_attention,
 };