Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1585590 > unrolled thread

[PATCH 1/3] Input: silead - Add OF device ID table

Started byJavier Martinez Canillas <javier@osg.samsung.com>
First post2017-02-21 19:20 +0100
Last post2017-02-22 15:30 +0100
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] Input: silead - Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-02-21 19:20 +0100
    [PATCH 3/3] Input: qt1070 - Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-02-21 19:20 +0100
      Re: [PATCH 3/3] Input: qt1070 - Add OF device ID table Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-23 09:30 +0100
        Re: [PATCH 3/3] Input: qt1070 - Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-02-23 13:40 +0100
      Re: [PATCH 3/3] Input: qt1070 - Add OF device ID table Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-23 09:30 +0100
    Re: [PATCH 1/3] Input: silead - Add OF device ID table Hans de Goede <hdegoede@redhat.com> - 2017-02-22 09:30 +0100
      Re: [PATCH 1/3] Input: silead - Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-02-22 13:50 +0100
        Re: [PATCH 1/3] Input: silead - Add OF device ID table Hans de Goede <hdegoede@redhat.com> - 2017-02-22 15:30 +0100
          Re: [PATCH 1/3] Input: silead - Add OF device ID table Javier Martinez Canillas <javier@osg.samsung.com> - 2017-02-22 15:30 +0100

#1585590 — [PATCH 1/3] Input: silead - Add OF device ID table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2017-02-21 19:20 +0100
Subject[PATCH 1/3] Input: silead - Add OF device ID table
Message-ID<tdncC-6eZ-15@gated-at.bofh.it>
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" },
+	{ },
+};
+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,
 	},
 };
-- 
2.9.3

[toc] | [next] | [standalone]

#1585592 — [PATCH 3/3] Input: qt1070 - Add OF device ID table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2017-02-21 19:20 +0100
Subject[PATCH 3/3] Input: qt1070 - Add OF device ID table
Message-ID<tdncC-6eZ-27@gated-at.bofh.it>
In reply to#1585590
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.

The compatible strings don't have a vendor prefix because that's how it's
used currently, and changing this will be a Device Tree ABI break.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/input/keyboard/qt1070.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c
index 5a5778729e37..76bb51309a78 100644
--- a/drivers/input/keyboard/qt1070.c
+++ b/drivers/input/keyboard/qt1070.c
@@ -274,9 +274,18 @@ static const struct i2c_device_id qt1070_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, qt1070_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id qt1070_of_match[] = {
+	{ .compatible = "qt1070", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, qt1070_of_match);
+#endif
+
 static struct i2c_driver qt1070_driver = {
 	.driver	= {
 		.name	= "qt1070",
+		.of_match_table = of_match_ptr(qt1070_of_match),
 		.pm	= &qt1070_pm_ops,
 	},
 	.id_table	= qt1070_id,
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1586729 — Re: [PATCH 3/3] Input: qt1070 - Add OF device ID table

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2017-02-23 09:30 +0100
SubjectRe: [PATCH 3/3] Input: qt1070 - Add OF device ID table
Message-ID<tdWWK-76D-5@gated-at.bofh.it>
In reply to#1585592
On Thu, Feb 23, 2017 at 12:25:24AM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 21, 2017 at 03:12:54PM -0300, 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.
> > 
> > The compatible strings don't have a vendor prefix because that's how it's
> > used currently, and changing this will be a Device Tree ABI break.
> 
> Are you saying that all legacy I2C names now form DT ABI? Even for
> drivers that do not have of_match_table or OF MODULE_DEVICE_TABLE not
> binding documentation?
> 
> I think this is a bit too much.

Ah, I see that it is actually used in various DTSes.

OK, I think we still need the proper compatible ("atmel,qt1070") along
with the legacy compatible string.

> 
> > 
> > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> > ---
> > 
> >  drivers/input/keyboard/qt1070.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c
> > index 5a5778729e37..76bb51309a78 100644
> > --- a/drivers/input/keyboard/qt1070.c
> > +++ b/drivers/input/keyboard/qt1070.c
> > @@ -274,9 +274,18 @@ static const struct i2c_device_id qt1070_id[] = {
> >  };
> >  MODULE_DEVICE_TABLE(i2c, qt1070_id);
> >  
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id qt1070_of_match[] = {
> > +	{ .compatible = "qt1070", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, qt1070_of_match);
> > +#endif
> > +
> >  static struct i2c_driver qt1070_driver = {
> >  	.driver	= {
> >  		.name	= "qt1070",
> > +		.of_match_table = of_match_ptr(qt1070_of_match),
> >  		.pm	= &qt1070_pm_ops,
> >  	},
> >  	.id_table	= qt1070_id,
> > -- 
> > 2.9.3
> > 
> 
> -- 
> Dmitry

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1586856 — Re: [PATCH 3/3] Input: qt1070 - Add OF device ID table

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2017-02-23 13:40 +0100
SubjectRe: [PATCH 3/3] Input: qt1070 - Add OF device ID table
Message-ID<te0QG-1gv-15@gated-at.bofh.it>
In reply to#1586729
Hello Dmitry,

On 02/23/2017 05:27 AM, Dmitry Torokhov wrote:
> On Thu, Feb 23, 2017 at 12:25:24AM -0800, Dmitry Torokhov wrote:
>> On Tue, Feb 21, 2017 at 03:12:54PM -0300, 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.
>>>
>>> The compatible strings don't have a vendor prefix because that's how it's
>>> used currently, and changing this will be a Device Tree ABI break.
>>
>> Are you saying that all legacy I2C names now form DT ABI? Even for
>> drivers that do not have of_match_table or OF MODULE_DEVICE_TABLE not
>> binding documentation?
>>
>> I think this is a bit too much.
> 
> Ah, I see that it is actually used in various DTSes.
>

Yes, I'm only posting patches for drivers whose I2C device ID .name are
either used by a DTS or its .name mentioned in a binding as a compatible.

The idea is to eventually fix the I2C core to report a proper of MODALIAS
so people won't have to add a duplicated I2C device ID table only for it.
 
> OK, I think we still need the proper compatible ("atmel,qt1070") along
> with the legacy compatible string.
>

I didn't add it because no DTS or DT binding doc mentions the complete
compatible string, so I would had to guess the vendor prefix. Yes, it's
quite likely "atmel", but how can I tell if that's really the case?

IOW, I just want to make sure that no driver module auto-loading will
regress once the I2C core starts reporting MODALIAS=of:N*T*Cqt1070
instead MODALIAS=i2c:qt1070. So I would prefer if someone who cares
about this driver can propose a patch on top to add the compatible
with a vendor prefix.

>>
>>>
>>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>> ---
>>>
 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1586730 — Re: [PATCH 3/3] Input: qt1070 - Add OF device ID table

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2017-02-23 09:30 +0100
SubjectRe: [PATCH 3/3] Input: qt1070 - Add OF device ID table
Message-ID<tdWWK-76D-7@gated-at.bofh.it>
In reply to#1585592
On Tue, Feb 21, 2017 at 03:12:54PM -0300, 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.
> 
> The compatible strings don't have a vendor prefix because that's how it's
> used currently, and changing this will be a Device Tree ABI break.

Are you saying that all legacy I2C names now form DT ABI? Even for
drivers that do not have of_match_table or OF MODULE_DEVICE_TABLE not
binding documentation?

I think this is a bit too much.

> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/input/keyboard/qt1070.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c
> index 5a5778729e37..76bb51309a78 100644
> --- a/drivers/input/keyboard/qt1070.c
> +++ b/drivers/input/keyboard/qt1070.c
> @@ -274,9 +274,18 @@ static const struct i2c_device_id qt1070_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, qt1070_id);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id qt1070_of_match[] = {
> +	{ .compatible = "qt1070", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, qt1070_of_match);
> +#endif
> +
>  static struct i2c_driver qt1070_driver = {
>  	.driver	= {
>  		.name	= "qt1070",
> +		.of_match_table = of_match_ptr(qt1070_of_match),
>  		.pm	= &qt1070_pm_ops,
>  	},
>  	.id_table	= qt1070_id,
> -- 
> 2.9.3
> 

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1585984

FromHans de Goede <hdegoede@redhat.com>
Date2017-02-22 09:30 +0100
Message-ID<tdAtc-7g0-1@gated-at.bofh.it>
In reply to#1585590
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,
>  	},
>  };
>

[toc] | [prev] | [next] | [standalone]

#1586121

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2017-02-22 13:50 +0100
Message-ID<tdEwO-1Jr-11@gated-at.bofh.it>
In reply to#1585984
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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1586196

FromHans de Goede <hdegoede@redhat.com>
Date2017-02-22 15:30 +0100
Message-ID<tdG5z-2Z4-17@gated-at.bofh.it>
In reply to#1586121
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

[toc] | [prev] | [next] | [standalone]

#1586197

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2017-02-22 15:30 +0100
Message-ID<tdG5A-2Z4-27@gated-at.bofh.it>
In reply to#1586196
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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web