diff mbox

[14/15] Input: synaptics-rmi4 - ensure we have IRQs before reading status

Message ID 1390521623-6491-15-git-send-email-courtney.cavin@sonymobile.com (mailing list archive)
State New, archived
Headers show

Commit Message

Courtney Cavin Jan. 24, 2014, midnight UTC
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_f01.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Christopher Heiny Feb. 4, 2014, 11:10 p.m. UTC | #1
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_f01.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 22b57f2..06fc5bc 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -318,13 +318,15 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>   	ctrl_base_addr += sizeof(u8);
>
>   	data->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));
> -	if (error < 0) {
> -		dev_err(&fn->dev,
> -			"Failed to read F01 control interrupt enable register.\n");
> -		goto error_exit;
> +	if (data->num_of_irq_regs > 0) {
> +		error = rmi_read_block(rmi_dev, ctrl_base_addr,
> +					data->device_control.interrupt_enable,
> +					sizeof(u8) * (data->num_of_irq_regs));
> +		if (error < 0) {
> +			dev_err(&fn->dev,
> +				"Failed to read F01 control interrupt enable register.\n");
> +			goto error_exit;
> +		}

If we don't know the number of IRQ registers at this point, we're 
basically screwed, as most of the control register positions will be 
known correctly.  The previously submitted IRQ counting patch ensures 
that number of IRQ registers is know by this point - if they aren't, 
then we should probably fail entirely rather than just muddling along.

>   	}
>
>   	ctrl_base_addr += data->num_of_irq_regs;
>
Courtney Cavin Feb. 5, 2014, 2:40 a.m. UTC | #2
On Wed, Feb 05, 2014 at 12:10:49AM +0100, 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_f01.c | 16 +++++++++-------
> >   1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> > index 22b57f2..06fc5bc 100644
> > --- a/drivers/input/rmi4/rmi_f01.c
> > +++ b/drivers/input/rmi4/rmi_f01.c
> > @@ -318,13 +318,15 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> >   	ctrl_base_addr += sizeof(u8);
> >
> >   	data->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));
> > -	if (error < 0) {
> > -		dev_err(&fn->dev,
> > -			"Failed to read F01 control interrupt enable register.\n");
> > -		goto error_exit;
> > +	if (data->num_of_irq_regs > 0) {
> > +		error = rmi_read_block(rmi_dev, ctrl_base_addr,
> > +					data->device_control.interrupt_enable,
> > +					sizeof(u8) * (data->num_of_irq_regs));
> > +		if (error < 0) {
> > +			dev_err(&fn->dev,
> > +				"Failed to read F01 control interrupt enable register.\n");
> > +			goto error_exit;
> > +		}
> 
> If we don't know the number of IRQ registers at this point, we're 
> basically screwed, as most of the control register positions will be 
> known correctly.  The previously submitted IRQ counting patch ensures 
> that number of IRQ registers is know by this point - if they aren't, 
> then we should probably fail entirely rather than just muddling along.
> 

Sounds good.  I just needed this to prevent random failure cases during
testing.  If your patch solves this problem, then it's probably a much
better way to address this.

> >   	}
> >
> >   	ctrl_base_addr += data->num_of_irq_regs;
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 22b57f2..06fc5bc 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -318,13 +318,15 @@  static int rmi_f01_initialize(struct rmi_function *fn)
 	ctrl_base_addr += sizeof(u8);
 
 	data->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));
-	if (error < 0) {
-		dev_err(&fn->dev,
-			"Failed to read F01 control interrupt enable register.\n");
-		goto error_exit;
+	if (data->num_of_irq_regs > 0) {
+		error = rmi_read_block(rmi_dev, ctrl_base_addr,
+					data->device_control.interrupt_enable,
+					sizeof(u8) * (data->num_of_irq_regs));
+		if (error < 0) {
+			dev_err(&fn->dev,
+				"Failed to read F01 control interrupt enable register.\n");
+			goto error_exit;
+		}
 	}
 
 	ctrl_base_addr += data->num_of_irq_regs;