| Message ID | 20170221181254.14748-1-javier@osg.samsung.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi, On 21-02-17 19:12, Javier Martinez Canillas wrote: > The driver doesn't have a struct of_device_id table but supported devices > are registered via Device Trees. This is working on the assumption that a > I2C device registered via OF will always match a legacy I2C device ID and > that the MODALIAS reported will always be of the form i2c:<device>. > > But this could change in the future so the correct approach is to have an > OF device ID table if the devices are registered via OF. > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > --- > > drivers/input/touchscreen/silead.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c > index 404830a4a366..aae3ba1c3e02 100644 > --- a/drivers/input/touchscreen/silead.c > +++ b/drivers/input/touchscreen/silead.c > @@ -580,12 +580,26 @@ static const struct acpi_device_id silead_ts_acpi_match[] = { > MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match); > #endif > > +#ifdef CONFIG_OF > +static const struct of_device_id silead_ts_of_match[] = { > + { .compatible = "silead,gsl1680" }, > + { .compatible = "silead,gsl1688" }, > + { .compatible = "silead,gsl3670" }, > + { .compatible = "silead,gsl3675" }, > + { .compatible = "silead,gsl3692" }, > + { .compatible = "silead,mssl1680" }, > + { }, > +}; Please drop the mssl1680 compatible, that id an ACPI ugliness which we don't need for devicetree. Otherwise looks good to me. Regards, Hans > +MODULE_DEVICE_TABLE(of, silead_ts_of_match); > +#endif > + > static struct i2c_driver silead_ts_driver = { > .probe = silead_ts_probe, > .id_table = silead_ts_id, > .driver = { > .name = SILEAD_TS_NAME, > .acpi_match_table = ACPI_PTR(silead_ts_acpi_match), > + .of_match_table = of_match_ptr(silead_ts_of_match), > .pm = &silead_ts_pm, > }, > }; >
Hello Hans, Thanks for your feedback. On 02/22/2017 05:29 AM, Hans de Goede wrote: > Hi, > > On 21-02-17 19:12, Javier Martinez Canillas wrote: >> The driver doesn't have a struct of_device_id table but supported devices >> are registered via Device Trees. This is working on the assumption that a >> I2C device registered via OF will always match a legacy I2C device ID and >> that the MODALIAS reported will always be of the form i2c:<device>. >> >> But this could change in the future so the correct approach is to have an >> OF device ID table if the devices are registered via OF. >> >> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> >> --- >> >> drivers/input/touchscreen/silead.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c >> index 404830a4a366..aae3ba1c3e02 100644 >> --- a/drivers/input/touchscreen/silead.c >> +++ b/drivers/input/touchscreen/silead.c >> @@ -580,12 +580,26 @@ static const struct acpi_device_id silead_ts_acpi_match[] = { >> MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match); >> #endif >> >> +#ifdef CONFIG_OF >> +static const struct of_device_id silead_ts_of_match[] = { >> + { .compatible = "silead,gsl1680" }, >> + { .compatible = "silead,gsl1688" }, >> + { .compatible = "silead,gsl3670" }, >> + { .compatible = "silead,gsl3675" }, >> + { .compatible = "silead,gsl3692" }, >> + { .compatible = "silead,mssl1680" }, >> + { }, >> +}; > > Please drop the mssl1680 compatible, that id an ACPI ugliness Ok, I'll drop that compatible if isn't needed for Device Tree. > which we don't need for devicetree. > I'm not sure I understood your ACPI comment, > Otherwise looks good to me. > > Regards, > > Hans > > Best regards,
HI, On 22-02-17 13:45, Javier Martinez Canillas wrote: > Hello Hans, > > Thanks for your feedback. > > On 02/22/2017 05:29 AM, Hans de Goede wrote: >> Hi, >> >> On 21-02-17 19:12, Javier Martinez Canillas wrote: >>> The driver doesn't have a struct of_device_id table but supported devices >>> are registered via Device Trees. This is working on the assumption that a >>> I2C device registered via OF will always match a legacy I2C device ID and >>> that the MODALIAS reported will always be of the form i2c:<device>. >>> >>> But this could change in the future so the correct approach is to have an >>> OF device ID table if the devices are registered via OF. >>> >>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> >>> --- >>> >>> drivers/input/touchscreen/silead.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c >>> index 404830a4a366..aae3ba1c3e02 100644 >>> --- a/drivers/input/touchscreen/silead.c >>> +++ b/drivers/input/touchscreen/silead.c >>> @@ -580,12 +580,26 @@ static const struct acpi_device_id silead_ts_acpi_match[] = { >>> MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match); >>> #endif >>> >>> +#ifdef CONFIG_OF >>> +static const struct of_device_id silead_ts_of_match[] = { >>> + { .compatible = "silead,gsl1680" }, >>> + { .compatible = "silead,gsl1688" }, >>> + { .compatible = "silead,gsl3670" }, >>> + { .compatible = "silead,gsl3675" }, >>> + { .compatible = "silead,gsl3692" }, >>> + { .compatible = "silead,mssl1680" }, >>> + { }, >>> +}; >> >> Please drop the mssl1680 compatible, that id an ACPI ugliness > > Ok, I'll drop that compatible if isn't needed for Device Tree. > >> which we don't need for devicetree. >> > > I'm not sure I understood your ACPI comment, There is no silead chip named mssl1680, the mssl stands for microsoft silead (or so I believe) and it is used to identify the gsl1680 in some ACPI tables. Regards, Hans
Hello Hans, On 02/22/2017 11:23 AM, Hans de Goede wrote: > HI, > > On 22-02-17 13:45, Javier Martinez Canillas wrote: >> Hello Hans, >> >> Thanks for your feedback. >> >> On 02/22/2017 05:29 AM, Hans de Goede wrote: >>> Hi, >>> >>> On 21-02-17 19:12, Javier Martinez Canillas wrote: >>>> The driver doesn't have a struct of_device_id table but supported devices >>>> are registered via Device Trees. This is working on the assumption that a >>>> I2C device registered via OF will always match a legacy I2C device ID and >>>> that the MODALIAS reported will always be of the form i2c:<device>. >>>> >>>> But this could change in the future so the correct approach is to have an >>>> OF device ID table if the devices are registered via OF. >>>> >>>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> >>>> --- >>>> >>>> drivers/input/touchscreen/silead.c | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c >>>> index 404830a4a366..aae3ba1c3e02 100644 >>>> --- a/drivers/input/touchscreen/silead.c >>>> +++ b/drivers/input/touchscreen/silead.c >>>> @@ -580,12 +580,26 @@ static const struct acpi_device_id silead_ts_acpi_match[] = { >>>> MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match); >>>> #endif >>>> >>>> +#ifdef CONFIG_OF >>>> +static const struct of_device_id silead_ts_of_match[] = { >>>> + { .compatible = "silead,gsl1680" }, >>>> + { .compatible = "silead,gsl1688" }, >>>> + { .compatible = "silead,gsl3670" }, >>>> + { .compatible = "silead,gsl3675" }, >>>> + { .compatible = "silead,gsl3692" }, >>>> + { .compatible = "silead,mssl1680" }, >>>> + { }, >>>> +}; >>> >>> Please drop the mssl1680 compatible, that id an ACPI ugliness >> >> Ok, I'll drop that compatible if isn't needed for Device Tree. >> >>> which we don't need for devicetree. >>> >> >> I'm not sure I understood your ACPI comment, > > There is no silead chip named mssl1680, the mssl stands > for microsoft silead (or so I believe) and it is used > to identify the gsl1680 in some ACPI tables. > Ah, thanks a lot for the clarification. I'll re-spin the patch removing this entry then and adding your explanation in the commit message. > Regards, > > Hans Best regards,
diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c index 404830a4a366..aae3ba1c3e02 100644 --- a/drivers/input/touchscreen/silead.c +++ b/drivers/input/touchscreen/silead.c @@ -580,12 +580,26 @@ static const struct acpi_device_id silead_ts_acpi_match[] = { MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match); #endif +#ifdef CONFIG_OF +static const struct of_device_id silead_ts_of_match[] = { + { .compatible = "silead,gsl1680" }, + { .compatible = "silead,gsl1688" }, + { .compatible = "silead,gsl3670" }, + { .compatible = "silead,gsl3675" }, + { .compatible = "silead,gsl3692" }, + { .compatible = "silead,mssl1680" }, + { }, +}; +MODULE_DEVICE_TABLE(of, silead_ts_of_match); +#endif + static struct i2c_driver silead_ts_driver = { .probe = silead_ts_probe, .id_table = silead_ts_id, .driver = { .name = SILEAD_TS_NAME, .acpi_match_table = ACPI_PTR(silead_ts_acpi_match), + .of_match_table = of_match_ptr(silead_ts_of_match), .pm = &silead_ts_pm, }, };
The driver doesn't have a struct of_device_id table but supported devices are registered via Device Trees. This is working on the assumption that a I2C device registered via OF will always match a legacy I2C device ID and that the MODALIAS reported will always be of the form i2c:<device>. But this could change in the future so the correct approach is to have an OF device ID table if the devices are registered via OF. Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> --- drivers/input/touchscreen/silead.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)