diff mbox

Input: tsc200x - Report proper input_dev name

Message ID 1468986592-4973-1-git-send-email-mwelling@ieee.org (mailing list archive)
State Superseded
Headers show

Commit Message

Michael Welling July 20, 2016, 3:49 a.m. UTC
Passes input_id struct to the the common probe function for the tsc200x drivers
instead of just the bustype.

This allows for the use of the product variable to set the input_dev->name
variable according to the type of touchscreen used.

Signed-off-by: Michael Welling <mwelling@ieee.org>
---
 drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
 drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
 drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
 drivers/input/touchscreen/tsc200x-core.h | 2 +-
 4 files changed, 17 insertions(+), 6 deletions(-)

Comments

Pavel Machek July 20, 2016, 6:31 a.m. UTC | #1
Hi!

> Passes input_id struct to the the common probe function for the tsc200x drivers
> instead of just the bustype.
> 
> This allows for the use of the product variable to set the input_dev->name
> variable according to the type of touchscreen used.
> 
> Signed-off-by: Michael Welling <mwelling@ieee.org>
> ---
>  drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
>  drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
>  drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
>  drivers/input/touchscreen/tsc200x-core.h | 2 +-
>  4 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
> index 7295c19..6fe55d5 100644
> --- a/drivers/input/touchscreen/tsc2004.c
> +++ b/drivers/input/touchscreen/tsc2004.c
> @@ -22,6 +22,11 @@
>  #include <linux/regmap.h>
>  #include "tsc200x-core.h"
>  
> +static const struct input_id tsc2004_input_id = {
> +	.bustype = BUS_I2C,
> +	.product = 2004,
> +};
> +
>  static int tsc2004_cmd(struct device *dev, u8 cmd)
>  {
>  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c,
>  			 const struct i2c_device_id *id)
>  
>  {
> -	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> +	return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id,
>  			     devm_regmap_init_i2c(i2c, &tsc200x_regmap_config),
>  			     tsc2004_cmd);
>  }
> diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
> index b9f593d..f2c5f0e 100644
> --- a/drivers/input/touchscreen/tsc2005.c
> +++ b/drivers/input/touchscreen/tsc2005.c
> @@ -24,6 +24,11 @@
>  #include <linux/regmap.h>
>  #include "tsc200x-core.h"
>  
> +static const struct input_id tsc2005_input_id = {
> +	.bustype = BUS_SPI,
> +	.product = 2005,
> +};
> +
>  static int tsc2005_cmd(struct device *dev, u8 cmd)
>  {
>  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi)
>  	if (error)
>  		return error;
>  
> -	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
> +	return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id,
>  			     devm_regmap_init_spi(spi, &tsc200x_regmap_config),
>  			     tsc2005_cmd);
>  }
> diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
> index 26e81d1b..5e625c4 100644
> --- a/drivers/input/touchscreen/tsc200x-core.c
> +++ b/drivers/input/touchscreen/tsc200x-core.c
> @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input)
>  	mutex_unlock(&ts->mutex);
>  }
>  
> -int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
>  		  struct regmap *regmap,
>  		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
>  {
> @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
>  	snprintf(ts->phys, sizeof(ts->phys),
>  		 "%s/input-ts", dev_name(dev));
>  
> -	input_dev->name = "TSC200X touchscreen";
> +	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen",
> +					tsc_id->product);

What about:

     if (tsc_id->product == 2005)
          input_dev->name = "TSC2005 touchscreen";
     else
          input_dev->name = "TSC200X touchscreen";

We do want to use 'TSC2005' name for TSC2005, because compatibility,
but you should keep TSC200X.... because compatibility. You don't want
to break people's setups by going from TSC200X to TSC2004.

Best regards,
									Pavel
Michael Welling July 20, 2016, 6:50 a.m. UTC | #2
On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote:
> Hi!
> 
> > Passes input_id struct to the the common probe function for the tsc200x drivers
> > instead of just the bustype.
> > 
> > This allows for the use of the product variable to set the input_dev->name
> > variable according to the type of touchscreen used.
> > 
> > Signed-off-by: Michael Welling <mwelling@ieee.org>
> > ---
> >  drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
> >  drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
> >  drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
> >  drivers/input/touchscreen/tsc200x-core.h | 2 +-
> >  4 files changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
> > index 7295c19..6fe55d5 100644
> > --- a/drivers/input/touchscreen/tsc2004.c
> > +++ b/drivers/input/touchscreen/tsc2004.c
> > @@ -22,6 +22,11 @@
> >  #include <linux/regmap.h>
> >  #include "tsc200x-core.h"
> >  
> > +static const struct input_id tsc2004_input_id = {
> > +	.bustype = BUS_I2C,
> > +	.product = 2004,
> > +};
> > +
> >  static int tsc2004_cmd(struct device *dev, u8 cmd)
> >  {
> >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c,
> >  			 const struct i2c_device_id *id)
> >  
> >  {
> > -	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > +	return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id,
> >  			     devm_regmap_init_i2c(i2c, &tsc200x_regmap_config),
> >  			     tsc2004_cmd);
> >  }
> > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
> > index b9f593d..f2c5f0e 100644
> > --- a/drivers/input/touchscreen/tsc2005.c
> > +++ b/drivers/input/touchscreen/tsc2005.c
> > @@ -24,6 +24,11 @@
> >  #include <linux/regmap.h>
> >  #include "tsc200x-core.h"
> >  
> > +static const struct input_id tsc2005_input_id = {
> > +	.bustype = BUS_SPI,
> > +	.product = 2005,
> > +};
> > +
> >  static int tsc2005_cmd(struct device *dev, u8 cmd)
> >  {
> >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi)
> >  	if (error)
> >  		return error;
> >  
> > -	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
> > +	return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id,
> >  			     devm_regmap_init_spi(spi, &tsc200x_regmap_config),
> >  			     tsc2005_cmd);
> >  }
> > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
> > index 26e81d1b..5e625c4 100644
> > --- a/drivers/input/touchscreen/tsc200x-core.c
> > +++ b/drivers/input/touchscreen/tsc200x-core.c
> > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input)
> >  	mutex_unlock(&ts->mutex);
> >  }
> >  
> > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
> >  		  struct regmap *regmap,
> >  		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
> >  {
> > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> >  	snprintf(ts->phys, sizeof(ts->phys),
> >  		 "%s/input-ts", dev_name(dev));
> >  
> > -	input_dev->name = "TSC200X touchscreen";
> > +	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen",
> > +					tsc_id->product);
> 
> What about:
> 
>      if (tsc_id->product == 2005)
>           input_dev->name = "TSC2005 touchscreen";
>      else
>           input_dev->name = "TSC200X touchscreen";
> 
> We do want to use 'TSC2005' name for TSC2005, because compatibility,
> but you should keep TSC200X.... because compatibility. You don't want
> to break people's setups by going from TSC200X to TSC2004.

By same logic we shouldn't change from TSC200X back to TSC2005 because of
people's possibly new setup in the last 9 months.

> 
> Best regards,
> 									Pavel
> 
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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
Pavel Machek July 20, 2016, 6:54 a.m. UTC | #3
On Wed 2016-07-20 01:50:24, Michael Welling wrote:
> On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > Passes input_id struct to the the common probe function for the tsc200x drivers
> > > instead of just the bustype.
> > > 
> > > This allows for the use of the product variable to set the input_dev->name
> > > variable according to the type of touchscreen used.
> > > 
> > > Signed-off-by: Michael Welling <mwelling@ieee.org>
> > > ---
> > >  drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
> > >  drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
> > >  drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
> > >  drivers/input/touchscreen/tsc200x-core.h | 2 +-
> > >  4 files changed, 17 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
> > > index 7295c19..6fe55d5 100644
> > > --- a/drivers/input/touchscreen/tsc2004.c
> > > +++ b/drivers/input/touchscreen/tsc2004.c
> > > @@ -22,6 +22,11 @@
> > >  #include <linux/regmap.h>
> > >  #include "tsc200x-core.h"
> > >  
> > > +static const struct input_id tsc2004_input_id = {
> > > +	.bustype = BUS_I2C,
> > > +	.product = 2004,
> > > +};
> > > +
> > >  static int tsc2004_cmd(struct device *dev, u8 cmd)
> > >  {
> > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c,
> > >  			 const struct i2c_device_id *id)
> > >  
> > >  {
> > > -	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > > +	return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id,
> > >  			     devm_regmap_init_i2c(i2c, &tsc200x_regmap_config),
> > >  			     tsc2004_cmd);
> > >  }
> > > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
> > > index b9f593d..f2c5f0e 100644
> > > --- a/drivers/input/touchscreen/tsc2005.c
> > > +++ b/drivers/input/touchscreen/tsc2005.c
> > > @@ -24,6 +24,11 @@
> > >  #include <linux/regmap.h>
> > >  #include "tsc200x-core.h"
> > >  
> > > +static const struct input_id tsc2005_input_id = {
> > > +	.bustype = BUS_SPI,
> > > +	.product = 2005,
> > > +};
> > > +
> > >  static int tsc2005_cmd(struct device *dev, u8 cmd)
> > >  {
> > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi)
> > >  	if (error)
> > >  		return error;
> > >  
> > > -	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
> > > +	return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id,
> > >  			     devm_regmap_init_spi(spi, &tsc200x_regmap_config),
> > >  			     tsc2005_cmd);
> > >  }
> > > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
> > > index 26e81d1b..5e625c4 100644
> > > --- a/drivers/input/touchscreen/tsc200x-core.c
> > > +++ b/drivers/input/touchscreen/tsc200x-core.c
> > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input)
> > >  	mutex_unlock(&ts->mutex);
> > >  }
> > >  
> > > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> > > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
> > >  		  struct regmap *regmap,
> > >  		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
> > >  {
> > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> > >  	snprintf(ts->phys, sizeof(ts->phys),
> > >  		 "%s/input-ts", dev_name(dev));
> > >  
> > > -	input_dev->name = "TSC200X touchscreen";
> > > +	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen",
> > > +					tsc_id->product);
> > 
> > What about:
> > 
> >      if (tsc_id->product == 2005)
> >           input_dev->name = "TSC2005 touchscreen";
> >      else
> >           input_dev->name = "TSC200X touchscreen";
> > 
> > We do want to use 'TSC2005' name for TSC2005, because compatibility,
> > but you should keep TSC200X.... because compatibility. You don't want
> > to break people's setups by going from TSC200X to TSC2004.
> 
> By same logic we shouldn't change from TSC200X back to TSC2005 because of
> people's possibly new setup in the last 9 months.

That's your, broken logic.

TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005
for past few years.

TSC200X->TSC2004 is just introducing regression, because noone ever
seen TSC2004, AFAICT.

									Pavel
Michael Welling July 20, 2016, 7:06 a.m. UTC | #4
On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote:
> > By same logic we shouldn't change from TSC200X back to TSC2005 because of
> > people's possibly new setup in the last 9 months.
> 
> That's your, broken logic.
> 
> TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005
> for past few years.
> 
> TSC200X->TSC2004 is just introducing regression, because noone ever
> seen TSC2004, AFAICT.

I am pretty sure no one cares. Escpecially me.

> 
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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
Pavel Machek July 20, 2016, 7:48 a.m. UTC | #5
On Wed 2016-07-20 02:06:17, Michael Welling wrote:
> On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote:
> > > By same logic we shouldn't change from TSC200X back to TSC2005 because of
> > > people's possibly new setup in the last 9 months.
> > 
> > That's your, broken logic.
> > 
> > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005
> > for past few years.
> > 
> > TSC200X->TSC2004 is just introducing regression, because noone ever
> > seen TSC2004, AFAICT.
> 
> I am pretty sure no one cares. Escpecially me.

Dunno. Lets do the right thing: TSC2005 for 2005, TSC200X for everyone
else. Interface issues are tricky to handle once created, so its best
to be careful.

									Pavel
Dmitry Torokhov July 20, 2016, 4:45 p.m. UTC | #6
On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote:
> On Wed 2016-07-20 01:50:24, Michael Welling wrote:
> > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > Passes input_id struct to the the common probe function for the tsc200x drivers
> > > > instead of just the bustype.
> > > > 
> > > > This allows for the use of the product variable to set the input_dev->name
> > > > variable according to the type of touchscreen used.
> > > > 
> > > > Signed-off-by: Michael Welling <mwelling@ieee.org>
> > > > ---
> > > >  drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
> > > >  drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
> > > >  drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
> > > >  drivers/input/touchscreen/tsc200x-core.h | 2 +-
> > > >  4 files changed, 17 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
> > > > index 7295c19..6fe55d5 100644
> > > > --- a/drivers/input/touchscreen/tsc2004.c
> > > > +++ b/drivers/input/touchscreen/tsc2004.c
> > > > @@ -22,6 +22,11 @@
> > > >  #include <linux/regmap.h>
> > > >  #include "tsc200x-core.h"
> > > >  
> > > > +static const struct input_id tsc2004_input_id = {
> > > > +	.bustype = BUS_I2C,
> > > > +	.product = 2004,
> > > > +};
> > > > +
> > > >  static int tsc2004_cmd(struct device *dev, u8 cmd)
> > > >  {
> > > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c,
> > > >  			 const struct i2c_device_id *id)
> > > >  
> > > >  {
> > > > -	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > > > +	return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id,
> > > >  			     devm_regmap_init_i2c(i2c, &tsc200x_regmap_config),
> > > >  			     tsc2004_cmd);
> > > >  }
> > > > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
> > > > index b9f593d..f2c5f0e 100644
> > > > --- a/drivers/input/touchscreen/tsc2005.c
> > > > +++ b/drivers/input/touchscreen/tsc2005.c
> > > > @@ -24,6 +24,11 @@
> > > >  #include <linux/regmap.h>
> > > >  #include "tsc200x-core.h"
> > > >  
> > > > +static const struct input_id tsc2005_input_id = {
> > > > +	.bustype = BUS_SPI,
> > > > +	.product = 2005,
> > > > +};
> > > > +
> > > >  static int tsc2005_cmd(struct device *dev, u8 cmd)
> > > >  {
> > > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi)
> > > >  	if (error)
> > > >  		return error;
> > > >  
> > > > -	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
> > > > +	return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id,
> > > >  			     devm_regmap_init_spi(spi, &tsc200x_regmap_config),
> > > >  			     tsc2005_cmd);
> > > >  }
> > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
> > > > index 26e81d1b..5e625c4 100644
> > > > --- a/drivers/input/touchscreen/tsc200x-core.c
> > > > +++ b/drivers/input/touchscreen/tsc200x-core.c
> > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input)
> > > >  	mutex_unlock(&ts->mutex);
> > > >  }
> > > >  
> > > > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> > > > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
> > > >  		  struct regmap *regmap,
> > > >  		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
> > > >  {
> > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
> > > >  	snprintf(ts->phys, sizeof(ts->phys),
> > > >  		 "%s/input-ts", dev_name(dev));
> > > >  
> > > > -	input_dev->name = "TSC200X touchscreen";
> > > > +	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen",
> > > > +					tsc_id->product);
> > > 
> > > What about:
> > > 
> > >      if (tsc_id->product == 2005)
> > >           input_dev->name = "TSC2005 touchscreen";
> > >      else
> > >           input_dev->name = "TSC200X touchscreen";
> > > 
> > > We do want to use 'TSC2005' name for TSC2005, because compatibility,
> > > but you should keep TSC200X.... because compatibility. You don't want
> > > to break people's setups by going from TSC200X to TSC2004.
> > 
> > By same logic we shouldn't change from TSC200X back to TSC2005 because of
> > people's possibly new setup in the last 9 months.
> 
> That's your, broken logic.
> 
> TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005
> for past few years.

People might have also seen  TSC2005->TSC200X for the last few months.
They could be dependent on this now and start complaining once we
revert.

I am sorely tempted to leave the name as is. Just adjust your script to
check the name. Something like

TSC_MODEL=`sed -ne 's/^N: Name="TSC\([0-9]\+\) .*/\1/p' /proc/bus/input/devices`
xinput ... "TSC${TSC_MODEL} Touchscreen"

Or fetch the current name from sysfs.

Thanks.
Pali Rohár July 20, 2016, 4:56 p.m. UTC | #7
On Wednesday 20 July 2016 18:45:03 Dmitry Torokhov wrote:
> On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote:
> > On Wed 2016-07-20 01:50:24, Michael Welling wrote:
> > > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > > Passes input_id struct to the the common probe function for
> > > > > the tsc200x drivers instead of just the bustype.
> > > > > 
> > > > > This allows for the use of the product variable to set the
> > > > > input_dev->name variable according to the type of
> > > > > touchscreen used.
> > > > > 
> > > > > Signed-off-by: Michael Welling <mwelling@ieee.org>
> > > > > ---
> > > > > 
> > > > >  drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
> > > > >  drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
> > > > >  drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
> > > > >  drivers/input/touchscreen/tsc200x-core.h | 2 +-
> > > > >  4 files changed, 17 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/input/touchscreen/tsc2004.c
> > > > > b/drivers/input/touchscreen/tsc2004.c index 7295c19..6fe55d5
> > > > > 100644
> > > > > --- a/drivers/input/touchscreen/tsc2004.c
> > > > > +++ b/drivers/input/touchscreen/tsc2004.c
> > > > > @@ -22,6 +22,11 @@
> > > > > 
> > > > >  #include <linux/regmap.h>
> > > > >  #include "tsc200x-core.h"
> > > > > 
> > > > > +static const struct input_id tsc2004_input_id = {
> > > > > +	.bustype = BUS_I2C,
> > > > > +	.product = 2004,
> > > > > +};
> > > > > +
> > > > > 
> > > > >  static int tsc2004_cmd(struct device *dev, u8 cmd)
> > > > >  {
> > > > >  
> > > > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > > > 
> > > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client
> > > > > *i2c,
> > > > > 
> > > > >  			 const struct i2c_device_id *id)
> > > > >  
> > > > >  {
> > > > > 
> > > > > -	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > > > > +	return tsc200x_probe(&i2c->dev, i2c->irq,
> > > > > &tsc2004_input_id,
> > > > > 
> > > > >  			     devm_regmap_init_i2c(i2c,
> > > > >  			     &tsc200x_regmap_config),
> > > > >  			     tsc2004_cmd);
> > > > >  
> > > > >  }
> > > > > 
> > > > > diff --git a/drivers/input/touchscreen/tsc2005.c
> > > > > b/drivers/input/touchscreen/tsc2005.c index b9f593d..f2c5f0e
> > > > > 100644
> > > > > --- a/drivers/input/touchscreen/tsc2005.c
> > > > > +++ b/drivers/input/touchscreen/tsc2005.c
> > > > > @@ -24,6 +24,11 @@
> > > > > 
> > > > >  #include <linux/regmap.h>
> > > > >  #include "tsc200x-core.h"
> > > > > 
> > > > > +static const struct input_id tsc2005_input_id = {
> > > > > +	.bustype = BUS_SPI,
> > > > > +	.product = 2005,
> > > > > +};
> > > > > +
> > > > > 
> > > > >  static int tsc2005_cmd(struct device *dev, u8 cmd)
> > > > >  {
> > > > >  
> > > > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > > > 
> > > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device
> > > > > *spi)
> > > > > 
> > > > >  	if (error)
> > > > >  	
> > > > >  		return error;
> > > > > 
> > > > > -	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
> > > > > +	return tsc200x_probe(&spi->dev, spi->irq,
> > > > > &tsc2005_input_id,
> > > > > 
> > > > >  			     devm_regmap_init_spi(spi,
> > > > >  			     &tsc200x_regmap_config),
> > > > >  			     tsc2005_cmd);
> > > > >  
> > > > >  }
> > > > > 
> > > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c
> > > > > b/drivers/input/touchscreen/tsc200x-core.c index
> > > > > 26e81d1b..5e625c4 100644
> > > > > --- a/drivers/input/touchscreen/tsc200x-core.c
> > > > > +++ b/drivers/input/touchscreen/tsc200x-core.c
> > > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct
> > > > > input_dev *input)
> > > > > 
> > > > >  	mutex_unlock(&ts->mutex);
> > > > >  
> > > > >  }
> > > > > 
> > > > > -int tsc200x_probe(struct device *dev, int irq, __u16
> > > > > bustype, +int tsc200x_probe(struct device *dev, int irq,
> > > > > const struct input_id *tsc_id,
> > > > > 
> > > > >  		  struct regmap *regmap,
> > > > >  		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
> > > > >  
> > > > >  {
> > > > > 
> > > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev,
> > > > > int irq, __u16 bustype,
> > > > > 
> > > > >  	snprintf(ts->phys, sizeof(ts->phys),
> > > > >  	
> > > > >  		 "%s/input-ts", dev_name(dev));
> > > > > 
> > > > > -	input_dev->name = "TSC200X touchscreen";
> > > > > +	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d
> > > > > touchscreen", +					tsc_id->product);
> > > > 
> > > > What about:
> > > >      if (tsc_id->product == 2005)
> > > >      
> > > >           input_dev->name = "TSC2005 touchscreen";
> > > >      
> > > >      else
> > > >      
> > > >           input_dev->name = "TSC200X touchscreen";
> > > > 
> > > > We do want to use 'TSC2005' name for TSC2005, because
> > > > compatibility, but you should keep TSC200X.... because
> > > > compatibility. You don't want to break people's setups by
> > > > going from TSC200X to TSC2004.
> > > 
> > > By same logic we shouldn't change from TSC200X back to TSC2005
> > > because of people's possibly new setup in the last 9 months.
> > 
> > That's your, broken logic.
> > 
> > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005
> > for past few years.
> 
> People might have also seen  TSC2005->TSC200X for the last few
> months. They could be dependent on this now and start complaining
> once we revert.

They could. But did it? Is there real breakage of real existing 
applications which are in use? I doubt. But there is for opposite 
scenario.

Older applications (like Maemo mce) worked with older kernel version, 
but not with new due to this change TSC2005->TSC200X.

So I still think this is a regression, which should be fixed...
Dmitry Torokhov July 20, 2016, 5:04 p.m. UTC | #8
On Wed, Jul 20, 2016 at 06:56:51PM +0200, Pali Rohár wrote:
> On Wednesday 20 July 2016 18:45:03 Dmitry Torokhov wrote:
> > On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote:
> > > On Wed 2016-07-20 01:50:24, Michael Welling wrote:
> > > > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote:
> > > > > Hi!
> > > > > 
> > > > > > Passes input_id struct to the the common probe function for
> > > > > > the tsc200x drivers instead of just the bustype.
> > > > > > 
> > > > > > This allows for the use of the product variable to set the
> > > > > > input_dev->name variable according to the type of
> > > > > > touchscreen used.
> > > > > > 
> > > > > > Signed-off-by: Michael Welling <mwelling@ieee.org>
> > > > > > ---
> > > > > > 
> > > > > >  drivers/input/touchscreen/tsc2004.c      | 7 ++++++-
> > > > > >  drivers/input/touchscreen/tsc2005.c      | 7 ++++++-
> > > > > >  drivers/input/touchscreen/tsc200x-core.c | 7 ++++---
> > > > > >  drivers/input/touchscreen/tsc200x-core.h | 2 +-
> > > > > >  4 files changed, 17 insertions(+), 6 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/input/touchscreen/tsc2004.c
> > > > > > b/drivers/input/touchscreen/tsc2004.c index 7295c19..6fe55d5
> > > > > > 100644
> > > > > > --- a/drivers/input/touchscreen/tsc2004.c
> > > > > > +++ b/drivers/input/touchscreen/tsc2004.c
> > > > > > @@ -22,6 +22,11 @@
> > > > > > 
> > > > > >  #include <linux/regmap.h>
> > > > > >  #include "tsc200x-core.h"
> > > > > > 
> > > > > > +static const struct input_id tsc2004_input_id = {
> > > > > > +	.bustype = BUS_I2C,
> > > > > > +	.product = 2004,
> > > > > > +};
> > > > > > +
> > > > > > 
> > > > > >  static int tsc2004_cmd(struct device *dev, u8 cmd)
> > > > > >  {
> > > > > >  
> > > > > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > > > > 
> > > > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client
> > > > > > *i2c,
> > > > > > 
> > > > > >  			 const struct i2c_device_id *id)
> > > > > >  
> > > > > >  {
> > > > > > 
> > > > > > -	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > > > > > +	return tsc200x_probe(&i2c->dev, i2c->irq,
> > > > > > &tsc2004_input_id,
> > > > > > 
> > > > > >  			     devm_regmap_init_i2c(i2c,
> > > > > >  			     &tsc200x_regmap_config),
> > > > > >  			     tsc2004_cmd);
> > > > > >  
> > > > > >  }
> > > > > > 
> > > > > > diff --git a/drivers/input/touchscreen/tsc2005.c
> > > > > > b/drivers/input/touchscreen/tsc2005.c index b9f593d..f2c5f0e
> > > > > > 100644
> > > > > > --- a/drivers/input/touchscreen/tsc2005.c
> > > > > > +++ b/drivers/input/touchscreen/tsc2005.c
> > > > > > @@ -24,6 +24,11 @@
> > > > > > 
> > > > > >  #include <linux/regmap.h>
> > > > > >  #include "tsc200x-core.h"
> > > > > > 
> > > > > > +static const struct input_id tsc2005_input_id = {
> > > > > > +	.bustype = BUS_SPI,
> > > > > > +	.product = 2005,
> > > > > > +};
> > > > > > +
> > > > > > 
> > > > > >  static int tsc2005_cmd(struct device *dev, u8 cmd)
> > > > > >  {
> > > > > >  
> > > > > >  	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
> > > > > > 
> > > > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device
> > > > > > *spi)
> > > > > > 
> > > > > >  	if (error)
> > > > > >  	
> > > > > >  		return error;
> > > > > > 
> > > > > > -	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
> > > > > > +	return tsc200x_probe(&spi->dev, spi->irq,
> > > > > > &tsc2005_input_id,
> > > > > > 
> > > > > >  			     devm_regmap_init_spi(spi,
> > > > > >  			     &tsc200x_regmap_config),
> > > > > >  			     tsc2005_cmd);
> > > > > >  
> > > > > >  }
> > > > > > 
> > > > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c
> > > > > > b/drivers/input/touchscreen/tsc200x-core.c index
> > > > > > 26e81d1b..5e625c4 100644
> > > > > > --- a/drivers/input/touchscreen/tsc200x-core.c
> > > > > > +++ b/drivers/input/touchscreen/tsc200x-core.c
> > > > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct
> > > > > > input_dev *input)
> > > > > > 
> > > > > >  	mutex_unlock(&ts->mutex);
> > > > > >  
> > > > > >  }
> > > > > > 
> > > > > > -int tsc200x_probe(struct device *dev, int irq, __u16
> > > > > > bustype, +int tsc200x_probe(struct device *dev, int irq,
> > > > > > const struct input_id *tsc_id,
> > > > > > 
> > > > > >  		  struct regmap *regmap,
> > > > > >  		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
> > > > > >  
> > > > > >  {
> > > > > > 
> > > > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev,
> > > > > > int irq, __u16 bustype,
> > > > > > 
> > > > > >  	snprintf(ts->phys, sizeof(ts->phys),
> > > > > >  	
> > > > > >  		 "%s/input-ts", dev_name(dev));
> > > > > > 
> > > > > > -	input_dev->name = "TSC200X touchscreen";
> > > > > > +	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d
> > > > > > touchscreen", +					tsc_id->product);
> > > > > 
> > > > > What about:
> > > > >      if (tsc_id->product == 2005)
> > > > >      
> > > > >           input_dev->name = "TSC2005 touchscreen";
> > > > >      
> > > > >      else
> > > > >      
> > > > >           input_dev->name = "TSC200X touchscreen";
> > > > > 
> > > > > We do want to use 'TSC2005' name for TSC2005, because
> > > > > compatibility, but you should keep TSC200X.... because
> > > > > compatibility. You don't want to break people's setups by
> > > > > going from TSC200X to TSC2004.
> > > > 
> > > > By same logic we shouldn't change from TSC200X back to TSC2005
> > > > because of people's possibly new setup in the last 9 months.
> > > 
> > > That's your, broken logic.
> > > 
> > > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005
> > > for past few years.
> > 
> > People might have also seen  TSC2005->TSC200X for the last few
> > months. They could be dependent on this now and start complaining
> > once we revert.
> 
> They could. But did it? Is there real breakage of real existing 
> applications which are in use? I doubt. But there is for opposite 
> scenario.
> 
> Older applications (like Maemo mce) worked with older kernel version, 
> but not with new due to this change TSC2005->TSC200X.
> 
> So I still think this is a regression, which should be fixed...

OK, fine, let's keep 200X for 2004, and real number for everyone else.

Thanks.
diff mbox

Patch

diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
index 7295c19..6fe55d5 100644
--- a/drivers/input/touchscreen/tsc2004.c
+++ b/drivers/input/touchscreen/tsc2004.c
@@ -22,6 +22,11 @@ 
 #include <linux/regmap.h>
 #include "tsc200x-core.h"
 
+static const struct input_id tsc2004_input_id = {
+	.bustype = BUS_I2C,
+	.product = 2004,
+};
+
 static int tsc2004_cmd(struct device *dev, u8 cmd)
 {
 	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
@@ -42,7 +47,7 @@  static int tsc2004_probe(struct i2c_client *i2c,
 			 const struct i2c_device_id *id)
 
 {
-	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
+	return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id,
 			     devm_regmap_init_i2c(i2c, &tsc200x_regmap_config),
 			     tsc2004_cmd);
 }
diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index b9f593d..f2c5f0e 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -24,6 +24,11 @@ 
 #include <linux/regmap.h>
 #include "tsc200x-core.h"
 
+static const struct input_id tsc2005_input_id = {
+	.bustype = BUS_SPI,
+	.product = 2005,
+};
+
 static int tsc2005_cmd(struct device *dev, u8 cmd)
 {
 	u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd;
@@ -62,7 +67,7 @@  static int tsc2005_probe(struct spi_device *spi)
 	if (error)
 		return error;
 
-	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
+	return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id,
 			     devm_regmap_init_spi(spi, &tsc200x_regmap_config),
 			     tsc2005_cmd);
 }
diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index 26e81d1b..5e625c4 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -450,7 +450,7 @@  static void tsc200x_close(struct input_dev *input)
 	mutex_unlock(&ts->mutex);
 }
 
-int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
+int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		  struct regmap *regmap,
 		  int (*tsc200x_cmd)(struct device *dev, u8 cmd))
 {
@@ -547,9 +547,10 @@  int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input-ts", dev_name(dev));
 
-	input_dev->name = "TSC200X touchscreen";
+	input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen",
+					tsc_id->product);
 	input_dev->phys = ts->phys;
-	input_dev->id.bustype = bustype;
+	input_dev->id = *tsc_id;
 	input_dev->dev.parent = dev;
 	input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
diff --git a/drivers/input/touchscreen/tsc200x-core.h b/drivers/input/touchscreen/tsc200x-core.h
index 7a482d1..49a63a3 100644
--- a/drivers/input/touchscreen/tsc200x-core.h
+++ b/drivers/input/touchscreen/tsc200x-core.h
@@ -70,7 +70,7 @@ 
 extern const struct regmap_config tsc200x_regmap_config;
 extern const struct dev_pm_ops tsc200x_pm_ops;
 
-int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
+int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		  struct regmap *regmap,
 		  int (*tsc200x_cmd)(struct device *dev, u8 cmd));
 int tsc200x_remove(struct device *dev);