* [PATCH 2/2] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled @ 2011-03-17 14:43 Iiro Valkonen 2011-03-21 11:56 ` Joonyoung Shim 0 siblings, 1 reply; 7+ messages in thread From: Iiro Valkonen @ 2011-03-17 14:43 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure we don't miss the falling edge. Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> --- drivers/input/touchscreen/atmel_mxt_ts.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index f8cd478..ca2062a 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) if (error) return error; - error = mxt_make_highchg(data); - if (error) - return error; - mxt_handle_pdata(data); /* Backup to memory */ @@ -1098,6 +1094,10 @@ static int __devinit mxt_probe(struct i2c_client *client, if (error) goto err_unregister_device; + error = mxt_make_highchg(data); + if (error) + goto err_unregister_device; + return 0; err_unregister_device: -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled 2011-03-17 14:43 [PATCH 2/2] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled Iiro Valkonen @ 2011-03-21 11:56 ` Joonyoung Shim 2011-04-01 14:04 ` [PATCH 2/2 v2] " Iiro Valkonen 0 siblings, 1 reply; 7+ messages in thread From: Joonyoung Shim @ 2011-03-21 11:56 UTC (permalink / raw) To: Iiro Valkonen; +Cc: Dmitry Torokhov, linux-input Hi, Iiro. 2011/3/17 Iiro Valkonen <iiro.valkonen@atmel.com>: > Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure we don't miss the falling edge. > > Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> > --- > drivers/input/touchscreen/atmel_mxt_ts.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index f8cd478..ca2062a 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) > if (error) > return error; > > - error = mxt_make_highchg(data); > - if (error) > - return error; > - > mxt_handle_pdata(data); > > /* Backup to memory */ > @@ -1098,6 +1094,10 @@ static int __devinit mxt_probe(struct i2c_client *client, > if (error) > goto err_unregister_device; > > + error = mxt_make_highchg(data); > + if (error) > + goto err_unregister_device; > + You are missing one error handling about sysfs_create_group() and please consider firmware update flow also. Thanks. > return 0; > > err_unregister_device: > -- > 1.7.0.4 > -- > 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 > -- - Joonyoung Shim -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled 2011-03-21 11:56 ` Joonyoung Shim @ 2011-04-01 14:04 ` Iiro Valkonen 2011-04-08 4:56 ` Joonyoung Shim 0 siblings, 1 reply; 7+ messages in thread From: Iiro Valkonen @ 2011-04-01 14:04 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Joonyoung Shim, linux-input This is version 2 of the patch, where sysfs group is correctly removed in case of error, and the firmware update is also considered (mxt_make_highchg is also called from mxt_initialize). Original description: Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure we don't miss the falling edge. Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> --- drivers/input/touchscreen/atmel_mxt_ts.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 4012436..08ea846 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) if (error) return error; - error = mxt_make_highchg(data); - if (error) - return error; - mxt_handle_pdata(data); /* Backup to memory */ @@ -832,6 +828,10 @@ static int mxt_initialize(struct mxt_data *data) return error; info->matrix_ysize = val; + error = mxt_make_highchg(data); + if (error) + return error; + dev_info(&client->dev, "Family ID: %d Variant ID: %d Version: %d Build: %d\n", info->family_id, info->variant_id, info->version, @@ -1098,8 +1098,14 @@ static int __devinit mxt_probe(struct i2c_client *client, if (error) goto err_unregister_device; + error = mxt_make_highchg(data); + if (error) + goto err_remove_sysfs_group; + return 0; +err_remove_sysfs_group: + sysfs_remove_group(&client->dev.kobj, &mxt_attr_group); err_unregister_device: input_unregister_device(input_dev); input_dev = NULL; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 v2] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled 2011-04-01 14:04 ` [PATCH 2/2 v2] " Iiro Valkonen @ 2011-04-08 4:56 ` Joonyoung Shim 2011-04-08 12:58 ` [PATCH 2/2 v3] " Iiro Valkonen 2011-04-08 13:16 ` Iiro Valkonen 0 siblings, 2 replies; 7+ messages in thread From: Joonyoung Shim @ 2011-04-08 4:56 UTC (permalink / raw) To: Iiro Valkonen; +Cc: Dmitry Torokhov, linux-input Hi, 2011/4/1 Iiro Valkonen <iiro.valkonen@atmel.com>: > This is version 2 of the patch, where sysfs group is correctly removed in case of error, and the firmware update is also considered (mxt_make_highchg is also called from mxt_initialize). > > Original description: > > Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure we don't miss the falling edge. > > Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> > --- > drivers/input/touchscreen/atmel_mxt_ts.c | 14 ++++++++++---- > 1 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index 4012436..08ea846 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) > if (error) > return error; > > - error = mxt_make_highchg(data); > - if (error) > - return error; > - > mxt_handle_pdata(data); > > /* Backup to memory */ > @@ -832,6 +828,10 @@ static int mxt_initialize(struct mxt_data *data) > return error; > info->matrix_ysize = val; > > + error = mxt_make_highchg(data); > + if (error) > + return error; > + This is unnecessary patch. If you add this for firmware update part, how about move to mxt_update_fw_store()? > dev_info(&client->dev, > "Family ID: %d Variant ID: %d Version: %d Build: %d\n", > info->family_id, info->variant_id, info->version, > @@ -1098,8 +1098,14 @@ static int __devinit mxt_probe(struct i2c_client *client, > if (error) > goto err_unregister_device; > > + error = mxt_make_highchg(data); > + if (error) > + goto err_remove_sysfs_group; > + This is ok, but how about move this to the below request_threaded_irq() instead of adding error handling? > return 0; > > +err_remove_sysfs_group: > + sysfs_remove_group(&client->dev.kobj, &mxt_attr_group); > err_unregister_device: > input_unregister_device(input_dev); > input_dev = NULL; > -- > 1.7.0.4 > Thanks. -- - Joonyoung Shim -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 v3] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled 2011-04-08 4:56 ` Joonyoung Shim @ 2011-04-08 12:58 ` Iiro Valkonen 2011-04-08 13:16 ` Iiro Valkonen 1 sibling, 0 replies; 7+ messages in thread From: Iiro Valkonen @ 2011-04-08 12:58 UTC (permalink / raw) To: Joonyoung Shim; +Cc: Dmitry Torokhov, linux-input Hello, On 04/08/2011 07:56 AM, Joonyoung Shim wrote: > Hi, > > 2011/4/1 Iiro Valkonen <iiro.valkonen@atmel.com>: >> This is version 2 of the patch, where sysfs group is correctly removed in case of error, and the firmware update is also considered (mxt_make_highchg is also called from mxt_initialize). >> >> Original description: >> >> Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure we don't miss the falling edge. >> >> Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> >> --- >> drivers/input/touchscreen/atmel_mxt_ts.c | 14 ++++++++++---- >> 1 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c >> index 4012436..08ea846 100644 >> --- a/drivers/input/touchscreen/atmel_mxt_ts.c >> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c >> @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) >> if (error) >> return error; >> >> - error = mxt_make_highchg(data); >> - if (error) >> - return error; >> - >> mxt_handle_pdata(data); >> >> /* Backup to memory */ >> @@ -832,6 +828,10 @@ static int mxt_initialize(struct mxt_data *data) >> return error; >> info->matrix_ysize = val; >> >> + error = mxt_make_highchg(data); >> + if (error) >> + return error; >> + > > This is unnecessary patch. If you add this for firmware update part, > how about move to mxt_update_fw_store()? > Good call. >> dev_info(&client->dev, >> "Family ID: %d Variant ID: %d Version: %d Build: %d\n", >> info->family_id, info->variant_id, info->version, >> @@ -1098,8 +1098,14 @@ static int __devinit mxt_probe(struct i2c_client *client, >> if (error) >> goto err_unregister_device; >> >> + error = mxt_make_highchg(data); >> + if (error) >> + goto err_remove_sysfs_group; >> + > > This is ok, but how about move this to the below request_threaded_irq() > instead of adding error handling? > Makes sense, will move it. Thanks for the feedback. BR, -- Iiro ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 v3] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled 2011-04-08 4:56 ` Joonyoung Shim 2011-04-08 12:58 ` [PATCH 2/2 v3] " Iiro Valkonen @ 2011-04-08 13:16 ` Iiro Valkonen 2011-04-13 5:17 ` Joonyoung Shim 1 sibling, 1 reply; 7+ messages in thread From: Iiro Valkonen @ 2011-04-08 13:16 UTC (permalink / raw) To: Joonyoung Shim; +Cc: Dmitry Torokhov, linux-input Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure we don't miss the falling edge. Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> --- v3: Change the locations where this is done to ensure correct operation at FW update, and to get rid of some error handling drivers/input/touchscreen/atmel_mxt_ts.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 4012436..5f09f57 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) if (error) return error; - error = mxt_make_highchg(data); - if (error) - return error; - mxt_handle_pdata(data); /* Backup to memory */ @@ -981,6 +977,10 @@ static ssize_t mxt_update_fw_store(struct device *dev, enable_irq(data->irq); + error = mxt_make_highchg(data); + if (error) + return error; + return count; } @@ -1090,6 +1090,10 @@ static int __devinit mxt_probe(struct i2c_client *client, goto err_free_object; } + error = mxt_make_highchg(data); + if (error) + goto err_free_irq; + error = input_register_device(input_dev); if (error) goto err_free_irq; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 v3] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled 2011-04-08 13:16 ` Iiro Valkonen @ 2011-04-13 5:17 ` Joonyoung Shim 0 siblings, 0 replies; 7+ messages in thread From: Joonyoung Shim @ 2011-04-13 5:17 UTC (permalink / raw) To: Iiro Valkonen; +Cc: Dmitry Torokhov, linux-input 2011/4/8 Iiro Valkonen <iiro.valkonen@atmel.com>: > Make the CHG line (interrupt line) go high after the interrupts have been enabled to make sure > we don't miss the falling edge. > > Signed-off-by: Iiro Valkonen <iiro.valkonen@atmel.com> > --- > v3: Change the locations where this is done to ensure correct operation at FW update, > and to get rid of some error handling > > drivers/input/touchscreen/atmel_mxt_ts.c | 12 ++++++++---- > 1 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index 4012436..5f09f57 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -804,10 +804,6 @@ static int mxt_initialize(struct mxt_data *data) > if (error) > return error; > > - error = mxt_make_highchg(data); > - if (error) > - return error; > - > mxt_handle_pdata(data); > > /* Backup to memory */ > @@ -981,6 +977,10 @@ static ssize_t mxt_update_fw_store(struct device *dev, > > enable_irq(data->irq); > > + error = mxt_make_highchg(data); > + if (error) > + return error; > + > return count; > } > > @@ -1090,6 +1090,10 @@ static int __devinit mxt_probe(struct i2c_client *client, > goto err_free_object; > } > > + error = mxt_make_highchg(data); > + if (error) > + goto err_free_irq; > + > error = input_register_device(input_dev); > if (error) > goto err_free_irq; > -- > 1.7.0.4 > > Acked-by: Joonyoung Shim <jy0922.shim@samsung.com> Thanks. -- - Joonyoung Shim -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-04-13 5:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-17 14:43 [PATCH 2/2] Input: atmel_mxt_ts - Make CHG line high after the interrupts are enabled Iiro Valkonen 2011-03-21 11:56 ` Joonyoung Shim 2011-04-01 14:04 ` [PATCH 2/2 v2] " Iiro Valkonen 2011-04-08 4:56 ` Joonyoung Shim 2011-04-08 12:58 ` [PATCH 2/2 v3] " Iiro Valkonen 2011-04-08 13:16 ` Iiro Valkonen 2011-04-13 5:17 ` Joonyoung Shim
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.