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

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 <redacted>
---

 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

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 <redacted>
---

 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

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 <redacted>
---

 drivers/input/mouse/synaptics_i2c.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/input/mouse/synaptics_i2c.c b/drivers/input/mouse/synaptics_i2c.c
index cb2bf203f4ca..8538318d332c 100644
--- a/drivers/input/mouse/synaptics_i2c.c
+++ b/drivers/input/mouse/synaptics_i2c.c

@@ -652,9 +652,18 @@ static const struct i2c_device_id synaptics_i2c_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, synaptics_i2c_id_table);
 
+#ifdef CONFIG_OF
+static const struct of_device_id synaptics_i2c_of_match[] = {
+	{ .compatible = "synaptics,synaptics_i2c", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, synaptics_i2c_of_match);
+#endif
+
 static struct i2c_driver synaptics_i2c_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,
+		.of_match_table = of_match_ptr(synaptics_i2c_of_match),
 		.pm	= &synaptics_i2c_pm,
 	},
 

-- 
2.9.3

Hi,

On 21-02-17 19:12, Javier Martinez Canillas wrote:

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:

quoted

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 <redacted>
---

 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

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:

quoted

Hi,

On 21-02-17 19:12, Javier Martinez Canillas wrote:

quoted

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 <redacted>
---

 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.

quoted

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:

quoted

Hello Hans,

Thanks for your feedback.

On 02/22/2017 05:29 AM, Hans de Goede wrote:

quoted

Hi,

On 21-02-17 19:12, Javier Martinez Canillas wrote:

quoted

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 <redacted>
---

 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.

quoted

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

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.

-- 
Dmitry

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:

quoted

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.

quoted

Signed-off-by: Javier Martinez Canillas <redacted>
---

 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

On Tue, Feb 21, 2017 at 03:12:53PM -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.

This driver is for touchpad controller on a specific device, and as far
as I understand, for a specific firmware. I am not sure if it will ever
be reused.

Igor, maybe we need to drop it altogether?

-- 
Dmitry

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:

quoted

On Tue, Feb 21, 2017 at 03:12:54PM -0300, Javier Martinez Canillas wrote:

quoted

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.

quoted

quoted

Signed-off-by: Javier Martinez Canillas <redacted>
---

 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Hello Dmitry,

On 02/23/2017 05:31 AM, Dmitry Torokhov wrote:

On Tue, Feb 21, 2017 at 03:12:53PM -0300, Javier Martinez Canillas wrote:

quoted

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.

This driver is for touchpad controller on a specific device, and as far
as I understand, for a specific firmware. I am not sure if it will ever
be reused.

Igor, maybe we need to drop it altogether?

It's used in a mainline DTS (arch/arm/boot/dts/imx23-sansa.dts), that's why
I included in the set.

quoted

Signed-off-by: Javier Martinez Canillas <redacted>
---

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America