diff mbox

[v7,1/3] Input: max11801_ts: Add missing of_match_table

Message ID 1490180808-2856-1-git-send-email-jagan@openedev.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jagan Teki March 22, 2017, 11:06 a.m. UTC
From: Jagan Teki <jagan@amarulasolutions.com>

Added missing of_match_table for max11801_ts driver with
compatible as "maxim,max11801_ts"

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Matteo Lisi <matteo.lisi@engicam.com>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v7:
- newly added patch

 drivers/input/touchscreen/max11801_ts.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Dmitry Torokhov March 22, 2017, 6:30 p.m. UTC | #1
On Wed, Mar 22, 2017 at 04:36:46PM +0530, Jagan Teki wrote:
> From: Jagan Teki <jagan@amarulasolutions.com>
> 
> Added missing of_match_table for max11801_ts driver with
> compatible as "maxim,max11801_ts"
> 

Why not "maxim,max11801"? Also, I think we'd need a binding document.

> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Matteo Lisi <matteo.lisi@engicam.com>
> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v7:
> - newly added patch
> 
>  drivers/input/touchscreen/max11801_ts.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/max11801_ts.c b/drivers/input/touchscreen/max11801_ts.c
> index a595ae5..5085d4f 100644
> --- a/drivers/input/touchscreen/max11801_ts.c
> +++ b/drivers/input/touchscreen/max11801_ts.c
> @@ -224,9 +224,16 @@ static int max11801_ts_probe(struct i2c_client *client,
>  };
>  MODULE_DEVICE_TABLE(i2c, max11801_ts_id);
>  
> +static const struct of_device_id max11801_ts_dt_ids[] = {
> +	{ .compatible = "maxim,max11801_ts" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max11801_ts_dt_ids);
> +
>  static struct i2c_driver max11801_ts_driver = {
>  	.driver = {
>  		.name	= "max11801_ts",
> +		.of_match_table	= max11801_ts_dt_ids,
>  	},
>  	.id_table	= max11801_ts_id,
>  	.probe		= max11801_ts_probe,
> -- 
> 1.9.1
>
Jagan Teki March 22, 2017, 6:35 p.m. UTC | #2
On Thu, Mar 23, 2017 at 12:00 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Mar 22, 2017 at 04:36:46PM +0530, Jagan Teki wrote:
>> From: Jagan Teki <jagan@amarulasolutions.com>
>>
>> Added missing of_match_table for max11801_ts driver with
>> compatible as "maxim,max11801_ts"
>>
>
> Why not "maxim,max11801"? Also, I think we'd need a binding document.

Compatibility purpose to make this is for touchscreen and few of other
*_ts.c files follow the same, and added binding documentation in 2/3
patch.

thanks!
Dmitry Torokhov March 22, 2017, 6:44 p.m. UTC | #3
On Thu, Mar 23, 2017 at 12:05:17AM +0530, Jagan Teki wrote:
> On Thu, Mar 23, 2017 at 12:00 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Mar 22, 2017 at 04:36:46PM +0530, Jagan Teki wrote:
> >> From: Jagan Teki <jagan@amarulasolutions.com>
> >>
> >> Added missing of_match_table for max11801_ts driver with
> >> compatible as "maxim,max11801_ts"
> >>
> >
> > Why not "maxim,max11801"? Also, I think we'd need a binding document.
> 
> Compatibility purpose to make this is for touchscreen and few of other

Compatibility with what? The i2c id is "max11801" (without the "-ts").

> *_ts.c files follow the same, and added binding documentation in 2/3
> patch.

Sorry, it seems I am only copied on 1/3 patch.

Thanks.
Dmitry Torokhov March 22, 2017, 6:47 p.m. UTC | #4
On Wed, Mar 22, 2017 at 11:44:21AM -0700, Dmitry Torokhov wrote:
> On Thu, Mar 23, 2017 at 12:05:17AM +0530, Jagan Teki wrote:
> > On Thu, Mar 23, 2017 at 12:00 AM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > > On Wed, Mar 22, 2017 at 04:36:46PM +0530, Jagan Teki wrote:
> > >> From: Jagan Teki <jagan@amarulasolutions.com>
> > >>
> > >> Added missing of_match_table for max11801_ts driver with
> > >> compatible as "maxim,max11801_ts"
> > >>
> > >
> > > Why not "maxim,max11801"? Also, I think we'd need a binding document.
> > 
> > Compatibility purpose to make this is for touchscreen and few of other
> 
> Compatibility with what? The i2c id is "max11801" (without the "-ts").

Now that I looked at your 2/3 and 3/3 patches you use:

+&i2c1 {
+       max11801: touchscreen@48 {
+               compatible = "maxim,max11801";
+               reg = <0x48>;
+               interrupt-parent = <&gpio3>;
+               interrupts = <31 2>;
+       };
+};

so the compatible you are adding to the driver is definitely wrong.

By the way, it would be nice if you used symbolic constants to express
interrupt trigger type.

Thanks.
Jagan Teki March 22, 2017, 6:49 p.m. UTC | #5
On Thu, Mar 23, 2017 at 12:14 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Mar 23, 2017 at 12:05:17AM +0530, Jagan Teki wrote:
>> On Thu, Mar 23, 2017 at 12:00 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Wed, Mar 22, 2017 at 04:36:46PM +0530, Jagan Teki wrote:
>> >> From: Jagan Teki <jagan@amarulasolutions.com>
>> >>
>> >> Added missing of_match_table for max11801_ts driver with
>> >> compatible as "maxim,max11801_ts"
>> >>
>> >
>> > Why not "maxim,max11801"? Also, I think we'd need a binding document.
>>
>> Compatibility purpose to make this is for touchscreen and few of other
>
> Compatibility with what? The i2c id is "max11801" (without the "-ts").

Do we need to sync with i2c id as well? egalax_ts.c follow "_ts"
of-course the i2c there as follow the same.

>
>> *_ts.c files follow the same, and added binding documentation in 2/3
>> patch.
>
> Sorry, it seems I am only copied on 1/3 patch.

Sorry, will add in next version patches.

thanks!
diff mbox

Patch

diff --git a/drivers/input/touchscreen/max11801_ts.c b/drivers/input/touchscreen/max11801_ts.c
index a595ae5..5085d4f 100644
--- a/drivers/input/touchscreen/max11801_ts.c
+++ b/drivers/input/touchscreen/max11801_ts.c
@@ -224,9 +224,16 @@  static int max11801_ts_probe(struct i2c_client *client,
 };
 MODULE_DEVICE_TABLE(i2c, max11801_ts_id);
 
+static const struct of_device_id max11801_ts_dt_ids[] = {
+	{ .compatible = "maxim,max11801_ts" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max11801_ts_dt_ids);
+
 static struct i2c_driver max11801_ts_driver = {
 	.driver = {
 		.name	= "max11801_ts",
+		.of_match_table	= max11801_ts_dt_ids,
 	},
 	.id_table	= max11801_ts_id,
 	.probe		= max11801_ts_probe,