diff mbox

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

Message ID 20170221181254.14748-1-javier@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Feb. 21, 2017, 6:12 p.m. UTC
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(+)

Comments

Hans de Goede Feb. 22, 2017, 8:29 a.m. UTC | #1
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,
>  	},
>  };
>
--
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
Javier Martinez Canillas Feb. 22, 2017, 12:45 p.m. UTC | #2
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,
Hans de Goede Feb. 22, 2017, 2:23 p.m. UTC | #3
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
--
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
Javier Martinez Canillas Feb. 22, 2017, 2:25 p.m. UTC | #4
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 mbox

Patch

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,
 	},
 };