diff mbox series

[v2,1/4] Input: exc3000: split MT event handling from IRQ handler

Message ID 20200313143345.28565-1-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [v2,1/4] Input: exc3000: split MT event handling from IRQ handler | expand

Commit Message

Lucas Stach March 13, 2020, 2:33 p.m. UTC
Split out the multitouch event handling into it's own function to allow other
events to be handled in the IRQ handler without disturbing the MT handling.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/input/touchscreen/exc3000.c | 92 +++++++++++++++++------------
 1 file changed, 54 insertions(+), 38 deletions(-)

Comments

Lucas Stach May 18, 2020, 1:24 p.m. UTC | #1
Hi Dmitry,

any chance you could take a look at this series?

Regards,
Lucas

Am Freitag, den 13.03.2020, 15:33 +0100 schrieb Lucas Stach:
> Split out the multitouch event handling into it's own function to allow other
> events to be handled in the IRQ handler without disturbing the MT handling.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/input/touchscreen/exc3000.c | 92 +++++++++++++++++------------
>  1 file changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
> index e007e2e8f626..3458d02310dd 100644
> --- a/drivers/input/touchscreen/exc3000.c
> +++ b/drivers/input/touchscreen/exc3000.c
> @@ -58,6 +58,11 @@ static void exc3000_timer(struct timer_list *t)
>  	input_sync(data->input);
>  }
>  
> +static inline void exc3000_schedule_timer(struct exc3000_data *data)
> +{
> +	mod_timer(&data->timer, jiffies + msecs_to_jiffies(EXC3000_TIMEOUT_MS));
> +}
> +
>  static int exc3000_read_frame(struct i2c_client *client, u8 *buf)
>  {
>  	int ret;
> @@ -76,54 +81,35 @@ static int exc3000_read_frame(struct i2c_client *client, u8 *buf)
>  	if (ret != EXC3000_LEN_FRAME)
>  		return -EIO;
>  
> -	if (get_unaligned_le16(buf) != EXC3000_LEN_FRAME ||
> -			buf[2] != EXC3000_MT_EVENT)
> +	if (get_unaligned_le16(buf) != EXC3000_LEN_FRAME)
>  		return -EINVAL;
>  
>  	return 0;
>  }
>  
> -static int exc3000_read_data(struct i2c_client *client,
> -			     u8 *buf, int *n_slots)
> +static int exc3000_handle_mt_event(struct exc3000_data *data)
>  {
> -	int error;
> -
> -	error = exc3000_read_frame(client, buf);
> -	if (error)
> -		return error;
> +	struct input_dev *input = data->input;
> +	int ret, total_slots;
> +	u8 *buf = data->buf;
>  
> -	*n_slots = buf[3];
> -	if (!*n_slots || *n_slots > EXC3000_NUM_SLOTS)
> -		return -EINVAL;
> +	total_slots = buf[3];
> +	if (!total_slots || total_slots > EXC3000_NUM_SLOTS) {
> +		ret = -EINVAL;
> +		goto out_fail;
> +	}
>  
> -	if (*n_slots > EXC3000_SLOTS_PER_FRAME) {
> +	if (total_slots > EXC3000_SLOTS_PER_FRAME) {
>  		/* Read 2nd frame to get the rest of the contacts. */
> -		error = exc3000_read_frame(client, buf + EXC3000_LEN_FRAME);
> -		if (error)
> -			return error;
> +		ret = exc3000_read_frame(data->client, buf + EXC3000_LEN_FRAME);
> +		if (ret)
> +			goto out_fail;
>  
>  		/* 2nd chunk must have number of contacts set to 0. */
> -		if (buf[EXC3000_LEN_FRAME + 3] != 0)
> -			return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
> -static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
> -{
> -	struct exc3000_data *data = dev_id;
> -	struct input_dev *input = data->input;
> -	u8 *buf = data->buf;
> -	int slots, total_slots;
> -	int error;
> -
> -	error = exc3000_read_data(data->client, buf, &total_slots);
> -	if (error) {
> -		/* Schedule a timer to release "stuck" contacts */
> -		mod_timer(&data->timer,
> -			  jiffies + msecs_to_jiffies(EXC3000_TIMEOUT_MS));
> -		goto out;
> +		if (buf[EXC3000_LEN_FRAME + 3] != 0) {
> +			ret = -EINVAL;
> +			goto out_fail;
> +		}
>  	}
>  
>  	/*
> @@ -132,7 +118,7 @@ static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
>  	del_timer_sync(&data->timer);
>  
>  	while (total_slots > 0) {
> -		slots = min(total_slots, EXC3000_SLOTS_PER_FRAME);
> +		int slots = min(total_slots, EXC3000_SLOTS_PER_FRAME);
>  		exc3000_report_slots(input, &data->prop, buf + 4, slots);
>  		total_slots -= slots;
>  		buf += EXC3000_LEN_FRAME;
> @@ -141,6 +127,36 @@ static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
>  	input_mt_sync_frame(input);
>  	input_sync(input);
>  
> +	return 0;
> +
> +out_fail:
> +	/* Schedule a timer to release "stuck" contacts */
> +	exc3000_schedule_timer(data);
> +
> +	return ret;
> +}
> +
> +static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
> +{
> +	struct exc3000_data *data = dev_id;
> +	u8 *buf = data->buf;
> +	int ret;
> +
> +	ret = exc3000_read_frame(data->client, buf);
> +	if (ret) {
> +		/* Schedule a timer to release "stuck" contacts */
> +		exc3000_schedule_timer(data);
> +		goto out;
> +	}
> +
> +	switch (buf[2]) {
> +		case EXC3000_MT_EVENT:
> +			exc3000_handle_mt_event(data);
> +			break;
> +		default:
> +			break;
> +	}
> +
>  out:
>  	return IRQ_HANDLED;
>  }
Lucas Stach Aug. 10, 2020, 12:40 p.m. UTC | #2
Hi Dmitry,

Am Montag, den 18.05.2020, 15:24 +0200 schrieb Lucas Stach:
> Hi Dmitry,
> 
> any chance you could take a look at this series?

Are you able to find some time to look at this series? Should I resend
the series?

Regards,
Lucas

> Am Freitag, den 13.03.2020, 15:33 +0100 schrieb Lucas Stach:
> > Split out the multitouch event handling into it's own function to
> > allow other
> > events to be handled in the IRQ handler without disturbing the MT
> > handling.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/input/touchscreen/exc3000.c | 92 +++++++++++++++++------
> > ------
> >  1 file changed, 54 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/exc3000.c
> > b/drivers/input/touchscreen/exc3000.c
> > index e007e2e8f626..3458d02310dd 100644
> > --- a/drivers/input/touchscreen/exc3000.c
> > +++ b/drivers/input/touchscreen/exc3000.c
> > @@ -58,6 +58,11 @@ static void exc3000_timer(struct timer_list *t)
> >  	input_sync(data->input);
> >  }
> >  
> > +static inline void exc3000_schedule_timer(struct exc3000_data
> > *data)
> > +{
> > +	mod_timer(&data->timer, jiffies +
> > msecs_to_jiffies(EXC3000_TIMEOUT_MS));
> > +}
> > +
> >  static int exc3000_read_frame(struct i2c_client *client, u8 *buf)
> >  {
> >  	int ret;
> > @@ -76,54 +81,35 @@ static int exc3000_read_frame(struct i2c_client
> > *client, u8 *buf)
> >  	if (ret != EXC3000_LEN_FRAME)
> >  		return -EIO;
> >  
> > -	if (get_unaligned_le16(buf) != EXC3000_LEN_FRAME ||
> > -			buf[2] != EXC3000_MT_EVENT)
> > +	if (get_unaligned_le16(buf) != EXC3000_LEN_FRAME)
> >  		return -EINVAL;
> >  
> >  	return 0;
> >  }
> >  
> > -static int exc3000_read_data(struct i2c_client *client,
> > -			     u8 *buf, int *n_slots)
> > +static int exc3000_handle_mt_event(struct exc3000_data *data)
> >  {
> > -	int error;
> > -
> > -	error = exc3000_read_frame(client, buf);
> > -	if (error)
> > -		return error;
> > +	struct input_dev *input = data->input;
> > +	int ret, total_slots;
> > +	u8 *buf = data->buf;
> >  
> > -	*n_slots = buf[3];
> > -	if (!*n_slots || *n_slots > EXC3000_NUM_SLOTS)
> > -		return -EINVAL;
> > +	total_slots = buf[3];
> > +	if (!total_slots || total_slots > EXC3000_NUM_SLOTS) {
> > +		ret = -EINVAL;
> > +		goto out_fail;
> > +	}
> >  
> > -	if (*n_slots > EXC3000_SLOTS_PER_FRAME) {
> > +	if (total_slots > EXC3000_SLOTS_PER_FRAME) {
> >  		/* Read 2nd frame to get the rest of the contacts. */
> > -		error = exc3000_read_frame(client, buf +
> > EXC3000_LEN_FRAME);
> > -		if (error)
> > -			return error;
> > +		ret = exc3000_read_frame(data->client, buf +
> > EXC3000_LEN_FRAME);
> > +		if (ret)
> > +			goto out_fail;
> >  
> >  		/* 2nd chunk must have number of contacts set to 0. */
> > -		if (buf[EXC3000_LEN_FRAME + 3] != 0)
> > -			return -EINVAL;
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> > -static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
> > -{
> > -	struct exc3000_data *data = dev_id;
> > -	struct input_dev *input = data->input;
> > -	u8 *buf = data->buf;
> > -	int slots, total_slots;
> > -	int error;
> > -
> > -	error = exc3000_read_data(data->client, buf, &total_slots);
> > -	if (error) {
> > -		/* Schedule a timer to release "stuck" contacts */
> > -		mod_timer(&data->timer,
> > -			  jiffies +
> > msecs_to_jiffies(EXC3000_TIMEOUT_MS));
> > -		goto out;
> > +		if (buf[EXC3000_LEN_FRAME + 3] != 0) {
> > +			ret = -EINVAL;
> > +			goto out_fail;
> > +		}
> >  	}
> >  
> >  	/*
> > @@ -132,7 +118,7 @@ static irqreturn_t exc3000_interrupt(int irq,
> > void *dev_id)
> >  	del_timer_sync(&data->timer);
> >  
> >  	while (total_slots > 0) {
> > -		slots = min(total_slots, EXC3000_SLOTS_PER_FRAME);
> > +		int slots = min(total_slots, EXC3000_SLOTS_PER_FRAME);
> >  		exc3000_report_slots(input, &data->prop, buf + 4,
> > slots);
> >  		total_slots -= slots;
> >  		buf += EXC3000_LEN_FRAME;
> > @@ -141,6 +127,36 @@ static irqreturn_t exc3000_interrupt(int irq,
> > void *dev_id)
> >  	input_mt_sync_frame(input);
> >  	input_sync(input);
> >  
> > +	return 0;
> > +
> > +out_fail:
> > +	/* Schedule a timer to release "stuck" contacts */
> > +	exc3000_schedule_timer(data);
> > +
> > +	return ret;
> > +}
> > +
> > +static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
> > +{
> > +	struct exc3000_data *data = dev_id;
> > +	u8 *buf = data->buf;
> > +	int ret;
> > +
> > +	ret = exc3000_read_frame(data->client, buf);
> > +	if (ret) {
> > +		/* Schedule a timer to release "stuck" contacts */
> > +		exc3000_schedule_timer(data);
> > +		goto out;
> > +	}
> > +
> > +	switch (buf[2]) {
> > +		case EXC3000_MT_EVENT:
> > +			exc3000_handle_mt_event(data);
> > +			break;
> > +		default:
> > +			break;
> > +	}
> > +
> >  out:
> >  	return IRQ_HANDLED;
> >  }
> 
>
Dmitry Torokhov Aug. 10, 2020, 4:27 p.m. UTC | #3
Hi Lucas,

On Mon, Aug 10, 2020 at 02:40:30PM +0200, Lucas Stach wrote:
> Hi Dmitry,
> 
> Am Montag, den 18.05.2020, 15:24 +0200 schrieb Lucas Stach:
> > Hi Dmitry,
> > 
> > any chance you could take a look at this series?
> 
> Are you able to find some time to look at this series? Should I resend
> the series?

Sorry about delays. Could you please resend as I applied another se5ries
to this driver?

Thanks!
Lucas Stach Aug. 10, 2020, 4:49 p.m. UTC | #4
Am Montag, den 10.08.2020, 09:27 -0700 schrieb Dmitry Torokhov:
> Hi Lucas,
> 
> On Mon, Aug 10, 2020 at 02:40:30PM +0200, Lucas Stach wrote:
> > Hi Dmitry,
> > 
> > Am Montag, den 18.05.2020, 15:24 +0200 schrieb Lucas Stach:
> > > Hi Dmitry,
> > > 
> > > any chance you could take a look at this series?
> > 
> > Are you able to find some time to look at this series? Should I resend
> > the series?
> 
> Sorry about delays. Could you please resend as I applied another se5ries
> to this driver?

Urgh, this may take me some time, as the other series collides quite
badly with this one and adds some of the same bits (firmware version
query). So I'll need to do some rework of the series, requiring a full
round of testing. :(

Regards,
Lucas
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c
index e007e2e8f626..3458d02310dd 100644
--- a/drivers/input/touchscreen/exc3000.c
+++ b/drivers/input/touchscreen/exc3000.c
@@ -58,6 +58,11 @@  static void exc3000_timer(struct timer_list *t)
 	input_sync(data->input);
 }
 
+static inline void exc3000_schedule_timer(struct exc3000_data *data)
+{
+	mod_timer(&data->timer, jiffies + msecs_to_jiffies(EXC3000_TIMEOUT_MS));
+}
+
 static int exc3000_read_frame(struct i2c_client *client, u8 *buf)
 {
 	int ret;
@@ -76,54 +81,35 @@  static int exc3000_read_frame(struct i2c_client *client, u8 *buf)
 	if (ret != EXC3000_LEN_FRAME)
 		return -EIO;
 
-	if (get_unaligned_le16(buf) != EXC3000_LEN_FRAME ||
-			buf[2] != EXC3000_MT_EVENT)
+	if (get_unaligned_le16(buf) != EXC3000_LEN_FRAME)
 		return -EINVAL;
 
 	return 0;
 }
 
-static int exc3000_read_data(struct i2c_client *client,
-			     u8 *buf, int *n_slots)
+static int exc3000_handle_mt_event(struct exc3000_data *data)
 {
-	int error;
-
-	error = exc3000_read_frame(client, buf);
-	if (error)
-		return error;
+	struct input_dev *input = data->input;
+	int ret, total_slots;
+	u8 *buf = data->buf;
 
-	*n_slots = buf[3];
-	if (!*n_slots || *n_slots > EXC3000_NUM_SLOTS)
-		return -EINVAL;
+	total_slots = buf[3];
+	if (!total_slots || total_slots > EXC3000_NUM_SLOTS) {
+		ret = -EINVAL;
+		goto out_fail;
+	}
 
-	if (*n_slots > EXC3000_SLOTS_PER_FRAME) {
+	if (total_slots > EXC3000_SLOTS_PER_FRAME) {
 		/* Read 2nd frame to get the rest of the contacts. */
-		error = exc3000_read_frame(client, buf + EXC3000_LEN_FRAME);
-		if (error)
-			return error;
+		ret = exc3000_read_frame(data->client, buf + EXC3000_LEN_FRAME);
+		if (ret)
+			goto out_fail;
 
 		/* 2nd chunk must have number of contacts set to 0. */
-		if (buf[EXC3000_LEN_FRAME + 3] != 0)
-			return -EINVAL;
-	}
-
-	return 0;
-}
-
-static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
-{
-	struct exc3000_data *data = dev_id;
-	struct input_dev *input = data->input;
-	u8 *buf = data->buf;
-	int slots, total_slots;
-	int error;
-
-	error = exc3000_read_data(data->client, buf, &total_slots);
-	if (error) {
-		/* Schedule a timer to release "stuck" contacts */
-		mod_timer(&data->timer,
-			  jiffies + msecs_to_jiffies(EXC3000_TIMEOUT_MS));
-		goto out;
+		if (buf[EXC3000_LEN_FRAME + 3] != 0) {
+			ret = -EINVAL;
+			goto out_fail;
+		}
 	}
 
 	/*
@@ -132,7 +118,7 @@  static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
 	del_timer_sync(&data->timer);
 
 	while (total_slots > 0) {
-		slots = min(total_slots, EXC3000_SLOTS_PER_FRAME);
+		int slots = min(total_slots, EXC3000_SLOTS_PER_FRAME);
 		exc3000_report_slots(input, &data->prop, buf + 4, slots);
 		total_slots -= slots;
 		buf += EXC3000_LEN_FRAME;
@@ -141,6 +127,36 @@  static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
 	input_mt_sync_frame(input);
 	input_sync(input);
 
+	return 0;
+
+out_fail:
+	/* Schedule a timer to release "stuck" contacts */
+	exc3000_schedule_timer(data);
+
+	return ret;
+}
+
+static irqreturn_t exc3000_interrupt(int irq, void *dev_id)
+{
+	struct exc3000_data *data = dev_id;
+	u8 *buf = data->buf;
+	int ret;
+
+	ret = exc3000_read_frame(data->client, buf);
+	if (ret) {
+		/* Schedule a timer to release "stuck" contacts */
+		exc3000_schedule_timer(data);
+		goto out;
+	}
+
+	switch (buf[2]) {
+		case EXC3000_MT_EVENT:
+			exc3000_handle_mt_event(data);
+			break;
+		default:
+			break;
+	}
+
 out:
 	return IRQ_HANDLED;
 }