Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-19 Thread Barnabás Pőcze
> [...] > You have been directly contributing to this patch because several > improvements of next version are from you. I experienced a similar > situation when submitting patches for QEMU. The only difference is > that the feedbacks on the QEMU patches were also given in the format > of patch. Or

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-18 Thread Coiby Xu
On Sun, Oct 18, 2020 at 12:23:14PM +, Barnabás Pőcze wrote: [...] > > > > > +static int i2c_hid_polling_thread(void *i2c_hid) > > > > > +{ > > > > > > > > > > - struct i2c_hid *ihid = i2c_hid; > > > > > - struct i2c_client *client = ihid->client; > > > > > - unsigned int polling_interva

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-18 Thread Coiby Xu
On Sat, Oct 17, 2020 at 02:58:13PM +, Barnabás Pőcze wrote: [...] >> >> +static int get_gpio_pin_state(struct irq_desc *irq_desc) >> >> +{ >> >> + struct gpio_chip *gc = irq_data_get_irq_chip_data(&irq_desc->irq_data); >> >> + >> >> + return gc->get(gc, irq_desc->irq_data.hwirq); >>

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-18 Thread Barnabás Pőcze
> [...] > > > > > > +static int i2c_hid_polling_thread(void *i2c_hid) > > > > > > +{ > > > > > > > > > > > > - struct i2c_hid *ihid = i2c_hid; > > > > > > - struct i2c_client *client = ihid->client; > > > > > > - unsigned int polling_interval_idle; > > > > > > - > > > > > > - while (1) { >

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-17 Thread Barnabás Pőcze
> [...] > >> >> +static int get_gpio_pin_state(struct irq_desc *irq_desc) > >> >> +{ > >> >> + struct gpio_chip *gc = > >> >> irq_data_get_irq_chip_data(&irq_desc->irq_data); > >> >> + > >> >> + return gc->get(gc, irq_desc->irq_data.hwirq); > >> >> +} > >> >> + > >> >> +static bool int

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-17 Thread Coiby Xu
Hi, On Sat, Oct 17, 2020 at 01:06:14PM +, Barnabás Pőcze wrote: Hi [...] >> +static int get_gpio_pin_state(struct irq_desc *irq_desc) >> +{ >> + struct gpio_chip *gc = irq_data_get_irq_chip_data(&irq_desc->irq_data); >> + >> + return gc->get(gc, irq_desc->irq_data.hwirq); >> +}

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-17 Thread Barnabás Pőcze
Hi > [...] > >> +static int get_gpio_pin_state(struct irq_desc *irq_desc) > >> +{ > >> + struct gpio_chip *gc = irq_data_get_irq_chip_data(&irq_desc->irq_data); > >> + > >> + return gc->get(gc, irq_desc->irq_data.hwirq); > >> +} > >> + > >> +static bool interrupt_line_active(struct i2c_client *c

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-16 Thread Coiby Xu
Hi, Thank you for examine this patch in such a careful way! On Fri, Oct 16, 2020 at 03:00:49PM +, Barnabás Pőcze wrote: Hi, I still think that `i2c_hid_resume()` and `i2c_hid_suspend()` are asymmetric and that might lead to issues. Do you think this commit message is relevant to your co

Re: [PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-16 Thread Barnabás Pőcze
Hi, I still think that `i2c_hid_resume()` and `i2c_hid_suspend()` are asymmetric and that might lead to issues. > [...] > When polling mode is enabled, an I2C device can't wake up the suspended > system since enable/disable_irq_wake is invalid for polling mode. > Excuse my ignorance, but could

[PATCH v2] HID: i2c-hid: add polling mode based on connected GPIO chip's pin status

2020-10-16 Thread Coiby Xu
For a broken touchpad, it may take several months or longer to be fixed. Polling mode could be a fallback solution for enthusiastic Linux users when they have a new laptop. It also acts like a debugging feature. If polling mode works for a broken touchpad, we can almost be certain the root cause is