[PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[Stephan Gerhold]

MMS345L is another first generation touch screen from Melfas,
which uses mostly the same registers as MMS152.

However, there is some garbage printed during initialization.
Apparently MMS345L does not have the MMS152_COMPAT_GROUP register
that is read+printed during initialization.

  TSP FW Rev: bootloader 0x6 / core 0x26 / config 0x26, Compat group: \x06

On earlier kernel versions the compat group was actually printed as
an ASCII control character, seems like it gets escaped now.

But we probably shouldn't print something from a random register.

Add a separate "melfas,mms345l" compatible that avoids reading
from the MMS152_COMPAT_GROUP register. This might also help in case
there is some other device-specific quirk in the future.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Adding an extra compatible just for a log message that looks weird
might be a bit exaggerated, I'm not sure. But so far I got the impression
that it's better to add separate compatibles if devices behave slightly
different (in case more differences are discovered later).

Alternatively we could also:

  - Remove the "Compat group:" from the log message
    (I'm not sure how useful it is since it's just printed but nothing
     is done with it...)

  - Just accept that we might be reading from some invalid register
    on MMS345L (It doesn't seem to affect functionality at the moment...)

I just thought we should discuss this before I upstream the patch
that adds the touchscreen to the device tree of my board.
---
 drivers/input/touchscreen/mms114.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 2ef1adaed9af..5bdf4ac1a303 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -54,6 +54,7 @@
 enum mms_type {
 	TYPE_MMS114	= 114,
 	TYPE_MMS152	= 152,
+	TYPE_MMS345L	= 345,
 };
 
 struct mms114_data {
@@ -250,6 +251,15 @@ static int mms114_get_version(struct mms114_data *data)
 	int error;
 
 	switch (data->type) {
+	case TYPE_MMS345L:
+		error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
+		if (error)
+			return error;
+
+		dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x\n",
+			 buf[0], buf[1], buf[2]);
+		break;
+
 	case TYPE_MMS152:
 		error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
 		if (error)
@@ -287,8 +297,8 @@ static int mms114_setup_regs(struct mms114_data *data)
 	if (error < 0)
 		return error;
 
-	/* MMS152 has no configuration or power on registers */
-	if (data->type == TYPE_MMS152)
+	/* Only MMS114 has configuration and power on registers */
+	if (data->type != TYPE_MMS114)
 		return 0;
 
 	error = mms114_set_active(data, true);
@@ -597,6 +607,9 @@ static const struct of_device_id mms114_dt_match[] = {
 	}, {
 		.compatible = "melfas,mms152",
 		.data = (void *)TYPE_MMS152,
+	}, {
+		.compatible = "melfas,mms345l",
+		.data = (void *)TYPE_MMS345L,
 	},
 	{ }
 };
-- 
2.26.2

[PATCH 2/2] dt-bindings: mms114: document melfas,mms345l binding

[Stephan Gerhold]

The mms114 driver now supports MMS345L; document the
melfas,mms345l binding that is used for it.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Reviewed-by: Andi Shyti <andi@etezian.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/input/touchscreen/mms114.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
index 2cd954051d29..707234cfd7e6 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
@@ -1,9 +1,10 @@
-* MELFAS MMS114/MMS152 touchscreen controller
+* MELFAS MMS114/MMS152/MMS345L touchscreen controller
 
 Required properties:
 - compatible: should be one of:
 	- "melfas,mms114"
 	- "melfas,mms152"
+	- "melfas,mms345l"
 - reg: I2C address of the chip
 - interrupts: interrupt to which the chip is connected
 - touchscreen-size-x: See [1]
-- 
2.26.2

Re: [PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[Andi Shyti]

Hi guys,

>  	}, {
>  		.compatible = "melfas,mms152",
>  		.data = (void *)TYPE_MMS152,
> +	}, {
> +		.compatible = "melfas,mms345l",
> +		.data = (void *)TYPE_MMS345L,
>  	},

it's been some times I haven't been doing this, but is the order
of the patches correct? shouldn't the binding be updated first?

Andi

Re: [PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[Stephan Gerhold]

On Fri, Apr 24, 2020 at 12:29:37PM +0300, Andi Shyti wrote:
> Hi guys,
> 
> >  	}, {
> >  		.compatible = "melfas,mms152",
> >  		.data = (void *)TYPE_MMS152,
> > +	}, {
> > +		.compatible = "melfas,mms345l",
> > +		.data = (void *)TYPE_MMS345L,
> >  	},
> 
> it's been some times I haven't been doing this, but is the order
> of the patches correct? shouldn't the binding be updated first?
> 

Yes. I had it correct in my original patch, but apparently swapped the
order accidentally for this one. I will do it correct again next time :)

Thanks,
Stephan

Re: [PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[Andi Shyti]

Hi Stephan,

On Fri, Apr 24, 2020 at 01:34:46PM +0200, Stephan Gerhold wrote:
> On Fri, Apr 24, 2020 at 12:29:37PM +0300, Andi Shyti wrote:
> > Hi guys,
> > 
> > >  	}, {
> > >  		.compatible = "melfas,mms152",
> > >  		.data = (void *)TYPE_MMS152,
> > > +	}, {
> > > +		.compatible = "melfas,mms345l",
> > > +		.data = (void *)TYPE_MMS345L,
> > >  	},
> > 
> > it's been some times I haven't been doing this, but is the order
> > of the patches correct? shouldn't the binding be updated first?
> > 
> 
> Yes. I had it correct in my original patch, but apparently swapped the
> order accidentally for this one. I will do it correct again next time :)

then with that change:

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

Re: [PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[Stephan Gerhold]

On Fri, Apr 24, 2020 at 04:22:43PM +0300, Andi Shyti wrote:
> Hi Stephan,
> 
> On Fri, Apr 24, 2020 at 01:34:46PM +0200, Stephan Gerhold wrote:
> > On Fri, Apr 24, 2020 at 12:29:37PM +0300, Andi Shyti wrote:
> > > Hi guys,
> > > 
> > > >  	}, {
> > > >  		.compatible = "melfas,mms152",
> > > >  		.data = (void *)TYPE_MMS152,
> > > > +	}, {
> > > > +		.compatible = "melfas,mms345l",
> > > > +		.data = (void *)TYPE_MMS345L,
> > > >  	},
> > > 
> > > it's been some times I haven't been doing this, but is the order
> > > of the patches correct? shouldn't the binding be updated first?
> > > 
> > 
> > Yes. I had it correct in my original patch, but apparently swapped the
> > order accidentally for this one. I will do it correct again next time :)
> 
> then with that change:
> 
> Reviewed-by: Andi Shyti <andi@etezian.org>
> 

Hi Dmitry,

I assume there is little reason to resend just to swap the order.
(You could just apply them in reverse order since they do not depend
 on each other...)

But if there is something else I should change just let me know.

Thanks,
Stephan

Re: [PATCH 2/2] dt-bindings: mms114: document melfas,mms345l binding

[Dmitry Torokhov]

On Thu, Apr 23, 2020 at 12:24:31PM +0200, Stephan Gerhold wrote:
> The mms114 driver now supports MMS345L; document the
> melfas,mms345l binding that is used for it.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> Reviewed-by: Andi Shyti <andi@etezian.org>
> Acked-by: Rob Herring <robh@kernel.org>

Applied, thank you.

> ---
>  Documentation/devicetree/bindings/input/touchscreen/mms114.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> index 2cd954051d29..707234cfd7e6 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> @@ -1,9 +1,10 @@
> -* MELFAS MMS114/MMS152 touchscreen controller
> +* MELFAS MMS114/MMS152/MMS345L touchscreen controller
>  
>  Required properties:
>  - compatible: should be one of:
>  	- "melfas,mms114"
>  	- "melfas,mms152"
> +	- "melfas,mms345l"
>  - reg: I2C address of the chip
>  - interrupts: interrupt to which the chip is connected
>  - touchscreen-size-x: See [1]
> -- 
> 2.26.2
> 

-- 
Dmitry

Re: [PATCH 1/2] Input: mms114 - add extra compatible for mms345l

[Dmitry Torokhov]

On Sat, Apr 25, 2020 at 08:24:17PM +0200, Stephan Gerhold wrote:
> On Fri, Apr 24, 2020 at 04:22:43PM +0300, Andi Shyti wrote:
> > Hi Stephan,
> > 
> > On Fri, Apr 24, 2020 at 01:34:46PM +0200, Stephan Gerhold wrote:
> > > On Fri, Apr 24, 2020 at 12:29:37PM +0300, Andi Shyti wrote:
> > > > Hi guys,
> > > > 
> > > > >  	}, {
> > > > >  		.compatible = "melfas,mms152",
> > > > >  		.data = (void *)TYPE_MMS152,
> > > > > +	}, {
> > > > > +		.compatible = "melfas,mms345l",
> > > > > +		.data = (void *)TYPE_MMS345L,
> > > > >  	},
> > > > 
> > > > it's been some times I haven't been doing this, but is the order
> > > > of the patches correct? shouldn't the binding be updated first?
> > > > 
> > > 
> > > Yes. I had it correct in my original patch, but apparently swapped the
> > > order accidentally for this one. I will do it correct again next time :)
> > 
> > then with that change:
> > 
> > Reviewed-by: Andi Shyti <andi@etezian.org>
> > 
> 
> Hi Dmitry,
> 
> I assume there is little reason to resend just to swap the order.
> (You could just apply them in reverse order since they do not depend
>  on each other...)
> 
> But if there is something else I should change just let me know.

No, that is it, I just applied the both.

Thanks.

-- 
Dmitry