diff mbox

[v2] goodix: Add new I2C/ACPI ids for GPD Win 2 touch screen

Message ID 20180529184853.12798-1-flibitijibibo@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ethan Lee May 29, 2018, 6:48 p.m. UTC
From: Ethan Lee <flibitijibibo@gmail.com>

GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp

Tested on a unit from the first production run sent to Indiegogo backers

Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>
---
 drivers/input/touchscreen/goodix.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Bastien Nocera May 29, 2018, 7:52 p.m. UTC | #1
On Tue, 2018-05-29 at 14:48 -0400, flibitijibibo@gmail.com wrote:
> From: Ethan Lee <flibitijibibo@gmail.com>
> 
> GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp
> 
> Tested on a unit from the first production run sent to Indiegogo
> backers
> 
> Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>

Looks good

Signed-off-by: Bastien Nocera <hadess@hadess.net>
--
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
Dmitry Torokhov May 29, 2018, 10:44 p.m. UTC | #2
On Tue, May 29, 2018 at 11:48 AM,  <flibitijibibo@gmail.com> wrote:
> From: Ethan Lee <flibitijibibo@gmail.com>
>
> GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp
>
> Tested on a unit from the first production run sent to Indiegogo backers
>
> Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>
> ---
>  drivers/input/touchscreen/goodix.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index 9736c83dd418..6a4ffe800194 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -926,6 +926,7 @@ static SIMPLE_DEV_PM_OPS(goodix_pm_ops, goodix_suspend, goodix_resume);
>
>  static const struct i2c_device_id goodix_ts_id[] = {
>         { "GDIX1001:00", 0 },
> +       { "GDIX1002:00", 0 },

Do we really need this? And actually the first one as well?

>         { }
>  };
>  MODULE_DEVICE_TABLE(i2c, goodix_ts_id);
> @@ -933,6 +934,7 @@ MODULE_DEVICE_TABLE(i2c, goodix_ts_id);
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id goodix_acpi_match[] = {
>         { "GDIX1001", 0 },
> +       { "GDIX1002", 0 },
>         { }
>  };
>  MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
> --
> 2.13.6
>

Thanks.
Bastien Nocera May 30, 2018, 8:19 a.m. UTC | #3
On Tue, 2018-05-29 at 15:44 -0700, Dmitry Torokhov wrote:
> On Tue, May 29, 2018 at 11:48 AM,  <flibitijibibo@gmail.com> wrote:
> > From: Ethan Lee <flibitijibibo@gmail.com>
> > 
> > GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp
> > 
> > Tested on a unit from the first production run sent to Indiegogo
> > backers
> > 
> > Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>
> > ---
> >  drivers/input/touchscreen/goodix.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/input/touchscreen/goodix.c
> > b/drivers/input/touchscreen/goodix.c
> > index 9736c83dd418..6a4ffe800194 100644
> > --- a/drivers/input/touchscreen/goodix.c
> > +++ b/drivers/input/touchscreen/goodix.c
> > @@ -926,6 +926,7 @@ static SIMPLE_DEV_PM_OPS(goodix_pm_ops,
> > goodix_suspend, goodix_resume);
> > 
> >  static const struct i2c_device_id goodix_ts_id[] = {
> >         { "GDIX1001:00", 0 },
> > +       { "GDIX1002:00", 0 },
> 
> Do we really need this? And actually the first one as well?

It was in the original driver submission, which isn't that old. I don't
have a way to test this right now though.
--
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
Dmitry Torokhov May 31, 2018, 7:16 p.m. UTC | #4
On Wed, May 30, 2018 at 10:19:46AM +0200, Bastien Nocera wrote:
> On Tue, 2018-05-29 at 15:44 -0700, Dmitry Torokhov wrote:
> > On Tue, May 29, 2018 at 11:48 AM,  <flibitijibibo@gmail.com> wrote:
> > > From: Ethan Lee <flibitijibibo@gmail.com>
> > > 
> > > GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp
> > > 
> > > Tested on a unit from the first production run sent to Indiegogo
> > > backers
> > > 
> > > Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>
> > > ---
> > >  drivers/input/touchscreen/goodix.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/input/touchscreen/goodix.c
> > > b/drivers/input/touchscreen/goodix.c
> > > index 9736c83dd418..6a4ffe800194 100644
> > > --- a/drivers/input/touchscreen/goodix.c
> > > +++ b/drivers/input/touchscreen/goodix.c
> > > @@ -926,6 +926,7 @@ static SIMPLE_DEV_PM_OPS(goodix_pm_ops,
> > > goodix_suspend, goodix_resume);
> > > 
> > >  static const struct i2c_device_id goodix_ts_id[] = {
> > >         { "GDIX1001:00", 0 },
> > > +       { "GDIX1002:00", 0 },
> > 
> > Do we really need this? And actually the first one as well?
> 
> It was in the original driver submission, which isn't that old. I don't
> have a way to test this right now though.

I am pretty sure we changed the way we generate modaliases for I2C
devices (from ACPI/OF) since then.

Ethan, can you please try building the driver without the new entry in
goodix_ts_id, make it a module and make sure it gets autoloaded for
you.

Thanks.
Bastien Nocera May 31, 2018, 9:33 p.m. UTC | #5
On Thu, 2018-05-31 at 12:16 -0700, Dmitry Torokhov wrote:
> 
<snip>
> I am pretty sure we changed the way we generate modaliases for I2C
> devices (from ACPI/OF) since then.

4 years already ;)

Would you be able to fixup if it wasn't needed, or will you want me to
send a follow-up to remove the goodix_ts_id table?
--
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
Ethan Lee May 31, 2018, 10:53 p.m. UTC | #6
Just gave this a shot and can confirm that the device is still 
recognized when I load the module. I mostly had both lines Just In 
Case(TM), but I dunno if there's something important to the other array, 
so I'll let Bastien take the wheel from here... let me know if there's 
anything else I should test.

-Ethan

On 05/31/2018 03:16 PM, Dmitry Torokhov wrote:
> On Wed, May 30, 2018 at 10:19:46AM +0200, Bastien Nocera wrote:
>> On Tue, 2018-05-29 at 15:44 -0700, Dmitry Torokhov wrote:
>>> On Tue, May 29, 2018 at 11:48 AM,  <flibitijibibo@gmail.com> wrote:
>>>> From: Ethan Lee <flibitijibibo@gmail.com>
>>>>
>>>> GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp
>>>>
>>>> Tested on a unit from the first production run sent to Indiegogo
>>>> backers
>>>>
>>>> Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>
>>>> ---
>>>>   drivers/input/touchscreen/goodix.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/input/touchscreen/goodix.c
>>>> b/drivers/input/touchscreen/goodix.c
>>>> index 9736c83dd418..6a4ffe800194 100644
>>>> --- a/drivers/input/touchscreen/goodix.c
>>>> +++ b/drivers/input/touchscreen/goodix.c
>>>> @@ -926,6 +926,7 @@ static SIMPLE_DEV_PM_OPS(goodix_pm_ops,
>>>> goodix_suspend, goodix_resume);
>>>>
>>>>   static const struct i2c_device_id goodix_ts_id[] = {
>>>>          { "GDIX1001:00", 0 },
>>>> +       { "GDIX1002:00", 0 },
>>> Do we really need this? And actually the first one as well?
>> It was in the original driver submission, which isn't that old. I don't
>> have a way to test this right now though.
> I am pretty sure we changed the way we generate modaliases for I2C
> devices (from ACPI/OF) since then.
>
> Ethan, can you please try building the driver without the new entry in
> goodix_ts_id, make it a module and make sure it gets autoloaded for
> you.
>
> Thanks.
>

--
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
Dmitry Torokhov June 1, 2018, 12:04 a.m. UTC | #7
On Thu, May 31, 2018 at 06:53:55PM -0400, Ethan Lee wrote:
> Just gave this a shot and can confirm that the device is still recognized
> when I load the module. I mostly had both lines Just In Case(TM), but I
> dunno if there's something important to the other array, so I'll let Bastien
> take the wheel from here... let me know if there's anything else I should
> test.

OK, great, I'll drop the "GDIX1002:00" bit and apply.

> 
> -Ethan
> 
> On 05/31/2018 03:16 PM, Dmitry Torokhov wrote:
> > On Wed, May 30, 2018 at 10:19:46AM +0200, Bastien Nocera wrote:
> > > On Tue, 2018-05-29 at 15:44 -0700, Dmitry Torokhov wrote:
> > > > On Tue, May 29, 2018 at 11:48 AM,  <flibitijibibo@gmail.com> wrote:
> > > > > From: Ethan Lee <flibitijibibo@gmail.com>
> > > > > 
> > > > > GPD Win 2 Website: http://www.gpd.hk/gpdwin2.asp
> > > > > 
> > > > > Tested on a unit from the first production run sent to Indiegogo
> > > > > backers
> > > > > 
> > > > > Signed-off-by: Ethan Lee <flibitijibibo@gmail.com>
> > > > > ---
> > > > >   drivers/input/touchscreen/goodix.c | 2 ++
> > > > >   1 file changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/input/touchscreen/goodix.c
> > > > > b/drivers/input/touchscreen/goodix.c
> > > > > index 9736c83dd418..6a4ffe800194 100644
> > > > > --- a/drivers/input/touchscreen/goodix.c
> > > > > +++ b/drivers/input/touchscreen/goodix.c
> > > > > @@ -926,6 +926,7 @@ static SIMPLE_DEV_PM_OPS(goodix_pm_ops,
> > > > > goodix_suspend, goodix_resume);
> > > > > 
> > > > >   static const struct i2c_device_id goodix_ts_id[] = {
> > > > >          { "GDIX1001:00", 0 },
> > > > > +       { "GDIX1002:00", 0 },
> > > > Do we really need this? And actually the first one as well?
> > > It was in the original driver submission, which isn't that old. I don't
> > > have a way to test this right now though.
> > I am pretty sure we changed the way we generate modaliases for I2C
> > devices (from ACPI/OF) since then.
> > 
> > Ethan, can you please try building the driver without the new entry in
> > goodix_ts_id, make it a module and make sure it gets autoloaded for
> > you.
> > 
> > Thanks.
> > 
>
Dmitry Torokhov June 1, 2018, 12:05 a.m. UTC | #8
On Thu, May 31, 2018 at 11:33:22PM +0200, Bastien Nocera wrote:
> On Thu, 2018-05-31 at 12:16 -0700, Dmitry Torokhov wrote:
> > 
> <snip>
> > I am pretty sure we changed the way we generate modaliases for I2C
> > devices (from ACPI/OF) since then.
> 
> 4 years already ;)
> 
> Would you be able to fixup if it wasn't needed, or will you want me to
> send a follow-up to remove the goodix_ts_id table?

I'll drop the new ID from Ethan patch, but would be great if you sent me
a follow-up removing the goodix_ts_id altogether.
diff mbox

Patch

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 9736c83dd418..6a4ffe800194 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -926,6 +926,7 @@  static SIMPLE_DEV_PM_OPS(goodix_pm_ops, goodix_suspend, goodix_resume);
 
 static const struct i2c_device_id goodix_ts_id[] = {
 	{ "GDIX1001:00", 0 },
+	{ "GDIX1002:00", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, goodix_ts_id);
@@ -933,6 +934,7 @@  MODULE_DEVICE_TABLE(i2c, goodix_ts_id);
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id goodix_acpi_match[] = {
 	{ "GDIX1001", 0 },
+	{ "GDIX1002", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);