public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1
@ 2015-04-09 23:57 Dmitry Torokhov
  2015-04-10 17:41 ` Dmitry Torokhov
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2015-04-09 23:57 UTC (permalink / raw)
  To: Olof Johansson; +Cc: Benson Leung, Nick Dyer, linux-kernel

Atmel MXT devices use different i2c addresses, depending on the current
mode of operation (bootloader or application). The new Atmel MXT driver
expects i2c client's address contain the application address of the
chip, and calculates the expected bootloader address form the
application address. Unfortunately chromeos_laptop does probe the
devices and if touchpad (or touchscreen, or both) comes up in bootloader
mode, the i2c device gets instantiated with the bootloader address
instead of application address, which confuses the driver.

Given that hardware on Pixel is set and is not going to change let's not
try to probe devices to see if they are present or not, but rather
instantiate them always at expected addresses.

Since all devices are now probed and/or instantiated at given address,
we no longer need to support probing multiple addresses for the same
device.

Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Changes:

- v2 - addressed Bensons comments - removed Atmel bootloader addresses;

Olof,

We might want to mark this for stable as well as otherwise Pixel is
pretty broken if either touchpad or touchscreen resets to bootloader
mode.

Thanks,
Dmitry

 drivers/platform/chrome/chromeos_laptop.c | 72 ++++++-------------------------
 1 file changed, 13 insertions(+), 59 deletions(-)

diff --git a/drivers/platform/chrome/chromeos_laptop.c b/drivers/platform/chrome/chromeos_laptop.c
index b84fdd6..3dc258d 100644
--- a/drivers/platform/chrome/chromeos_laptop.c
+++ b/drivers/platform/chrome/chromeos_laptop.c
@@ -30,9 +30,7 @@
 #include <linux/platform_device.h>
 
 #define ATMEL_TP_I2C_ADDR	0x4b
-#define ATMEL_TP_I2C_BL_ADDR	0x25
 #define ATMEL_TS_I2C_ADDR	0x4a
-#define ATMEL_TS_I2C_BL_ADDR	0x26
 #define CYAPA_TP_I2C_ADDR	0x67
 #define ISL_ALS_I2C_ADDR	0x44
 #define TAOS_ALS_I2C_ADDR	0x29
@@ -129,12 +127,12 @@ static struct i2c_board_info atmel_1664s_device = {
 	.flags		= I2C_CLIENT_WAKE,
 };
 
-static struct i2c_client *__add_probed_i2c_device(
-		const char *name,
-		int bus,
-		struct i2c_board_info *info,
-		const unsigned short *addrs)
+static struct i2c_client *add_i2c_device(const char *name,
+					 int bus,
+					 struct i2c_board_info *info,
+					 bool probe)
 {
+	const unsigned short addrs[] = { info->addr, I2C_CLIENT_END };
 	const struct dmi_device *dmi_dev;
 	const struct dmi_dev_onboard *dev_data;
 	struct i2c_adapter *adapter;
@@ -170,7 +168,8 @@ static struct i2c_client *__add_probed_i2c_device(
 	}
 
 	/* add the i2c device */
-	client = i2c_new_probed_device(adapter, info, addrs, NULL);
+	client = probe ? i2c_new_probed_device(adapter, info, addrs, NULL) :
+			 i2c_new_device(adapter, info);
 	if (!client)
 		pr_notice("%s failed to register device %d-%02x\n",
 			  __func__, bus, info->addr);
@@ -225,78 +224,33 @@ static int find_i2c_adapter_num(enum i2c_adapter_type type)
 	return adapter->nr;
 }
 
-/*
- * Takes a list of addresses in addrs as such :
- * { addr1, ... , addrn, I2C_CLIENT_END };
- * add_probed_i2c_device will use i2c_new_probed_device
- * and probe for devices at all of the addresses listed.
- * Returns NULL if no devices found.
- * See Documentation/i2c/instantiating-devices for more information.
- */
-static struct i2c_client *add_probed_i2c_device(
-		const char *name,
-		enum i2c_adapter_type type,
-		struct i2c_board_info *info,
-		const unsigned short *addrs)
-{
-	return __add_probed_i2c_device(name,
-				       find_i2c_adapter_num(type),
-				       info,
-				       addrs);
-}
-
-/*
- * Probes for a device at a single address, the one provided by
- * info->addr.
- * Returns NULL if no device found.
- */
-static struct i2c_client *add_i2c_device(const char *name,
-						enum i2c_adapter_type type,
-						struct i2c_board_info *info)
-{
-	const unsigned short addr_list[] = { info->addr, I2C_CLIENT_END };
-
-	return __add_probed_i2c_device(name,
-				       find_i2c_adapter_num(type),
-				       info,
-				       addr_list);
-}
-
 static int setup_cyapa_tp(enum i2c_adapter_type type)
 {
 	if (tp)
 		return 0;
 
 	/* add cyapa touchpad */
-	tp = add_i2c_device("trackpad", type, &cyapa_device);
+	tp = add_i2c_device("trackpad", type, &cyapa_device, true);
 	return (!tp) ? -EAGAIN : 0;
 }
 
 static int setup_atmel_224s_tp(enum i2c_adapter_type type)
 {
-	const unsigned short addr_list[] = { ATMEL_TP_I2C_BL_ADDR,
-					     ATMEL_TP_I2C_ADDR,
-					     I2C_CLIENT_END };
 	if (tp)
 		return 0;
 
 	/* add atmel mxt touchpad */
-	tp = add_probed_i2c_device("trackpad", type,
-				   &atmel_224s_tp_device, addr_list);
+	tp = add_i2c_device("trackpad", type, &atmel_224s_tp_device, false);
 	return (!tp) ? -EAGAIN : 0;
 }
 
 static int setup_atmel_1664s_ts(enum i2c_adapter_type type)
 {
-	const unsigned short addr_list[] = { ATMEL_TS_I2C_BL_ADDR,
-					     ATMEL_TS_I2C_ADDR,
-					     I2C_CLIENT_END };
 	if (ts)
 		return 0;
 
 	/* add atmel mxt touch device */
-	ts = add_probed_i2c_device("touchscreen", type,
-				   &atmel_1664s_device, addr_list);
+	ts = add_i2c_device("touchscreen", type, &atmel_1664s_device, false);
 	return (!ts) ? -EAGAIN : 0;
 }
 
@@ -306,7 +260,7 @@ static int setup_isl29018_als(enum i2c_adapter_type type)
 		return 0;
 
 	/* add isl29018 light sensor */
-	als = add_i2c_device("lightsensor", type, &isl_als_device);
+	als = add_i2c_device("lightsensor", type, &isl_als_device, true);
 	return (!als) ? -EAGAIN : 0;
 }
 
@@ -316,7 +270,7 @@ static int setup_tsl2583_als(enum i2c_adapter_type type)
 		return 0;
 
 	/* add tsl2583 light sensor */
-	als = add_i2c_device(NULL, type, &tsl2583_als_device);
+	als = add_i2c_device(NULL, type, &tsl2583_als_device, true);
 	return (!als) ? -EAGAIN : 0;
 }
 
@@ -326,7 +280,7 @@ static int setup_tsl2563_als(enum i2c_adapter_type type)
 		return 0;
 
 	/* add tsl2563 light sensor */
-	als = add_i2c_device(NULL, type, &tsl2563_als_device);
+	als = add_i2c_device(NULL, type, &tsl2563_als_device, true);
 	return (!als) ? -EAGAIN : 0;
 }
 
-- 
2.2.0.rc0.207.ga3a616c


-- 
Dmitry

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1
  2015-04-09 23:57 [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1 Dmitry Torokhov
@ 2015-04-10 17:41 ` Dmitry Torokhov
  2015-04-14 20:50   ` Dmitry Torokhov
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2015-04-10 17:41 UTC (permalink / raw)
  To: Olof Johansson; +Cc: Benson Leung, Nick Dyer, linux-kernel

On Thu, Apr 09, 2015 at 04:57:59PM -0700, Dmitry Torokhov wrote:
> Atmel MXT devices use different i2c addresses, depending on the current
> mode of operation (bootloader or application). The new Atmel MXT driver
> expects i2c client's address contain the application address of the
> chip, and calculates the expected bootloader address form the
> application address. Unfortunately chromeos_laptop does probe the
> devices and if touchpad (or touchscreen, or both) comes up in bootloader
> mode, the i2c device gets instantiated with the bootloader address
> instead of application address, which confuses the driver.
> 
> Given that hardware on Pixel is set and is not going to change let's not
> try to probe devices to see if they are present or not, but rather
> instantiate them always at expected addresses.
> 
> Since all devices are now probed and/or instantiated at given address,
> we no longer need to support probing multiple addresses for the same

Hmm, that strategy won't work on C720 since there are devices with touchscreen
and without one, so we do want to probe but always instantiate at primary
address. V3 will be upcoming...

> device.
> 
> Reviewed-by: Benson Leung <bleung@chromium.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Changes:
> 
> - v2 - addressed Bensons comments - removed Atmel bootloader addresses;
> 
> Olof,
> 
> We might want to mark this for stable as well as otherwise Pixel is
> pretty broken if either touchpad or touchscreen resets to bootloader
> mode.
> 
> Thanks,
> Dmitry
> 
>  drivers/platform/chrome/chromeos_laptop.c | 72 ++++++-------------------------
>  1 file changed, 13 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/platform/chrome/chromeos_laptop.c b/drivers/platform/chrome/chromeos_laptop.c
> index b84fdd6..3dc258d 100644
> --- a/drivers/platform/chrome/chromeos_laptop.c
> +++ b/drivers/platform/chrome/chromeos_laptop.c
> @@ -30,9 +30,7 @@
>  #include <linux/platform_device.h>
>  
>  #define ATMEL_TP_I2C_ADDR	0x4b
> -#define ATMEL_TP_I2C_BL_ADDR	0x25
>  #define ATMEL_TS_I2C_ADDR	0x4a
> -#define ATMEL_TS_I2C_BL_ADDR	0x26
>  #define CYAPA_TP_I2C_ADDR	0x67
>  #define ISL_ALS_I2C_ADDR	0x44
>  #define TAOS_ALS_I2C_ADDR	0x29
> @@ -129,12 +127,12 @@ static struct i2c_board_info atmel_1664s_device = {
>  	.flags		= I2C_CLIENT_WAKE,
>  };
>  
> -static struct i2c_client *__add_probed_i2c_device(
> -		const char *name,
> -		int bus,
> -		struct i2c_board_info *info,
> -		const unsigned short *addrs)
> +static struct i2c_client *add_i2c_device(const char *name,
> +					 int bus,
> +					 struct i2c_board_info *info,
> +					 bool probe)
>  {
> +	const unsigned short addrs[] = { info->addr, I2C_CLIENT_END };
>  	const struct dmi_device *dmi_dev;
>  	const struct dmi_dev_onboard *dev_data;
>  	struct i2c_adapter *adapter;
> @@ -170,7 +168,8 @@ static struct i2c_client *__add_probed_i2c_device(
>  	}
>  
>  	/* add the i2c device */
> -	client = i2c_new_probed_device(adapter, info, addrs, NULL);
> +	client = probe ? i2c_new_probed_device(adapter, info, addrs, NULL) :
> +			 i2c_new_device(adapter, info);
>  	if (!client)
>  		pr_notice("%s failed to register device %d-%02x\n",
>  			  __func__, bus, info->addr);
> @@ -225,78 +224,33 @@ static int find_i2c_adapter_num(enum i2c_adapter_type type)
>  	return adapter->nr;
>  }
>  
> -/*
> - * Takes a list of addresses in addrs as such :
> - * { addr1, ... , addrn, I2C_CLIENT_END };
> - * add_probed_i2c_device will use i2c_new_probed_device
> - * and probe for devices at all of the addresses listed.
> - * Returns NULL if no devices found.
> - * See Documentation/i2c/instantiating-devices for more information.
> - */
> -static struct i2c_client *add_probed_i2c_device(
> -		const char *name,
> -		enum i2c_adapter_type type,
> -		struct i2c_board_info *info,
> -		const unsigned short *addrs)
> -{
> -	return __add_probed_i2c_device(name,
> -				       find_i2c_adapter_num(type),
> -				       info,
> -				       addrs);
> -}
> -
> -/*
> - * Probes for a device at a single address, the one provided by
> - * info->addr.
> - * Returns NULL if no device found.
> - */
> -static struct i2c_client *add_i2c_device(const char *name,
> -						enum i2c_adapter_type type,
> -						struct i2c_board_info *info)
> -{
> -	const unsigned short addr_list[] = { info->addr, I2C_CLIENT_END };
> -
> -	return __add_probed_i2c_device(name,
> -				       find_i2c_adapter_num(type),
> -				       info,
> -				       addr_list);
> -}
> -
>  static int setup_cyapa_tp(enum i2c_adapter_type type)
>  {
>  	if (tp)
>  		return 0;
>  
>  	/* add cyapa touchpad */
> -	tp = add_i2c_device("trackpad", type, &cyapa_device);
> +	tp = add_i2c_device("trackpad", type, &cyapa_device, true);
>  	return (!tp) ? -EAGAIN : 0;
>  }
>  
>  static int setup_atmel_224s_tp(enum i2c_adapter_type type)
>  {
> -	const unsigned short addr_list[] = { ATMEL_TP_I2C_BL_ADDR,
> -					     ATMEL_TP_I2C_ADDR,
> -					     I2C_CLIENT_END };
>  	if (tp)
>  		return 0;
>  
>  	/* add atmel mxt touchpad */
> -	tp = add_probed_i2c_device("trackpad", type,
> -				   &atmel_224s_tp_device, addr_list);
> +	tp = add_i2c_device("trackpad", type, &atmel_224s_tp_device, false);
>  	return (!tp) ? -EAGAIN : 0;
>  }
>  
>  static int setup_atmel_1664s_ts(enum i2c_adapter_type type)
>  {
> -	const unsigned short addr_list[] = { ATMEL_TS_I2C_BL_ADDR,
> -					     ATMEL_TS_I2C_ADDR,
> -					     I2C_CLIENT_END };
>  	if (ts)
>  		return 0;
>  
>  	/* add atmel mxt touch device */
> -	ts = add_probed_i2c_device("touchscreen", type,
> -				   &atmel_1664s_device, addr_list);
> +	ts = add_i2c_device("touchscreen", type, &atmel_1664s_device, false);
>  	return (!ts) ? -EAGAIN : 0;
>  }
>  
> @@ -306,7 +260,7 @@ static int setup_isl29018_als(enum i2c_adapter_type type)
>  		return 0;
>  
>  	/* add isl29018 light sensor */
> -	als = add_i2c_device("lightsensor", type, &isl_als_device);
> +	als = add_i2c_device("lightsensor", type, &isl_als_device, true);
>  	return (!als) ? -EAGAIN : 0;
>  }
>  
> @@ -316,7 +270,7 @@ static int setup_tsl2583_als(enum i2c_adapter_type type)
>  		return 0;
>  
>  	/* add tsl2583 light sensor */
> -	als = add_i2c_device(NULL, type, &tsl2583_als_device);
> +	als = add_i2c_device(NULL, type, &tsl2583_als_device, true);
>  	return (!als) ? -EAGAIN : 0;
>  }
>  
> @@ -326,7 +280,7 @@ static int setup_tsl2563_als(enum i2c_adapter_type type)
>  		return 0;
>  
>  	/* add tsl2563 light sensor */
> -	als = add_i2c_device(NULL, type, &tsl2563_als_device);
> +	als = add_i2c_device(NULL, type, &tsl2563_als_device, true);
>  	return (!als) ? -EAGAIN : 0;
>  }
>  
> -- 
> 2.2.0.rc0.207.ga3a616c
> 
> 
> -- 
> Dmitry

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1
  2015-04-10 17:41 ` Dmitry Torokhov
@ 2015-04-14 20:50   ` Dmitry Torokhov
  2015-04-14 20:50     ` Dmitry Torokhov
  2015-04-25  5:09     ` Olof Johansson
  0 siblings, 2 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2015-04-14 20:50 UTC (permalink / raw)
  To: Olof Johansson; +Cc: Benson Leung, Nick Dyer, linux-kernel

On Fri, Apr 10, 2015 at 10:41:54AM -0700, Dmitry Torokhov wrote:
> On Thu, Apr 09, 2015 at 04:57:59PM -0700, Dmitry Torokhov wrote:
> > Atmel MXT devices use different i2c addresses, depending on the current
> > mode of operation (bootloader or application). The new Atmel MXT driver
> > expects i2c client's address contain the application address of the
> > chip, and calculates the expected bootloader address form the
> > application address. Unfortunately chromeos_laptop does probe the
> > devices and if touchpad (or touchscreen, or both) comes up in bootloader
> > mode, the i2c device gets instantiated with the bootloader address
> > instead of application address, which confuses the driver.
> > 
> > Given that hardware on Pixel is set and is not going to change let's not
> > try to probe devices to see if they are present or not, but rather
> > instantiate them always at expected addresses.
> > 
> > Since all devices are now probed and/or instantiated at given address,
> > we no longer need to support probing multiple addresses for the same
> 
> Hmm, that strategy won't work on C720 since there are devices with touchscreen
> and without one, so we do want to probe but always instantiate at primary
> address. V3 will be upcoming...

OK, new version. Not sending to the wide world for now in case we decide
it is too ugly...


>From 480ea02024a0b9a2ad2f91e2e0ca02f34577972c Mon Sep 17 00:00:00 2001
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Mon, 12 Jan 2015 13:59:32 -0800
Subject: [PATCH] platform/chrome: chromeos_laptop - instantiate Atmel at
 primary address

The new Atmel MXT driver expects i2c client's address contain the
primary (main address) of the chip, and calculates the expected
bootloader address form the primary address. Unfortunately chrome_laptop
does probe the devices and if touchpad (or touchscreen, or both) comes
up in bootloader mode the i2c device gets instantiated with the
bootloader address which confuses the driver.

To work around this issue let's probe the primary address first. If the
device is not detected at the primary address we'll probe alternative
addresses as "dummy" devices. If any of them are found, destroy the
dummy client and instantiate client with proper name at primary address
still.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/platform/chrome/chromeos_laptop.c | 35 +++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/chrome/chromeos_laptop.c b/drivers/platform/chrome/chromeos_laptop.c
index b84fdd6..a04019a 100644
--- a/drivers/platform/chrome/chromeos_laptop.c
+++ b/drivers/platform/chrome/chromeos_laptop.c
@@ -133,12 +133,13 @@ static struct i2c_client *__add_probed_i2c_device(
 		const char *name,
 		int bus,
 		struct i2c_board_info *info,
-		const unsigned short *addrs)
+		const unsigned short *alt_addr_list)
 {
 	const struct dmi_device *dmi_dev;
 	const struct dmi_dev_onboard *dev_data;
 	struct i2c_adapter *adapter;
-	struct i2c_client *client;
+	struct i2c_client *client = NULL;
+	const unsigned short addr_list[] = { info->addr, I2C_CLIENT_END };
 
 	if (bus < 0)
 		return NULL;
@@ -169,8 +170,28 @@ static struct i2c_client *__add_probed_i2c_device(
 		return NULL;
 	}
 
-	/* add the i2c device */
-	client = i2c_new_probed_device(adapter, info, addrs, NULL);
+	/*
+	 * Add the i2c device. If we can't detect it at the primary
+	 * address we scan secondary addresses. In any case the client
+	 * structure gets assigned primary address.
+	 */
+	client = i2c_new_probed_device(adapter, info, addr_list, NULL);
+	if (!client && alt_addr_list) {
+		struct i2c_board_info dummy_info = {
+			I2C_BOARD_INFO("dummy", info->addr),
+		};
+		struct i2c_client *dummy;
+
+		dummy = i2c_new_probed_device(adapter, &dummy_info,
+					      alt_addr_list, NULL);
+		if (dummy) {
+			pr_debug("%s %d-%02x is probed at %02x\n",
+				  __func__, bus, info->addr, dummy->addr);
+			i2c_unregister_device(dummy);
+			client = i2c_new_device(adapter, info);
+		}
+	}
+
 	if (!client)
 		pr_notice("%s failed to register device %d-%02x\n",
 			  __func__, bus, info->addr);
@@ -254,12 +275,10 @@ static struct i2c_client *add_i2c_device(const char *name,
 						enum i2c_adapter_type type,
 						struct i2c_board_info *info)
 {
-	const unsigned short addr_list[] = { info->addr, I2C_CLIENT_END };
-
 	return __add_probed_i2c_device(name,
 				       find_i2c_adapter_num(type),
 				       info,
-				       addr_list);
+				       NULL);
 }
 
 static int setup_cyapa_tp(enum i2c_adapter_type type)
@@ -275,7 +294,6 @@ static int setup_cyapa_tp(enum i2c_adapter_type type)
 static int setup_atmel_224s_tp(enum i2c_adapter_type type)
 {
 	const unsigned short addr_list[] = { ATMEL_TP_I2C_BL_ADDR,
-					     ATMEL_TP_I2C_ADDR,
 					     I2C_CLIENT_END };
 	if (tp)
 		return 0;
@@ -289,7 +307,6 @@ static int setup_atmel_224s_tp(enum i2c_adapter_type type)
 static int setup_atmel_1664s_ts(enum i2c_adapter_type type)
 {
 	const unsigned short addr_list[] = { ATMEL_TS_I2C_BL_ADDR,
-					     ATMEL_TS_I2C_ADDR,
 					     I2C_CLIENT_END };
 	if (ts)
 		return 0;
-- 
2.2.0.rc0.207.ga3a616c


-- 
Dmitry

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1
  2015-04-14 20:50   ` Dmitry Torokhov
@ 2015-04-14 20:50     ` Dmitry Torokhov
  2015-04-25  5:09     ` Olof Johansson
  1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2015-04-14 20:50 UTC (permalink / raw)
  To: Olof Johansson; +Cc: Benson Leung, Nick Dyer, linux-kernel

On Tue, Apr 14, 2015 at 01:50:09PM -0700, Dmitry Torokhov wrote:
> On Fri, Apr 10, 2015 at 10:41:54AM -0700, Dmitry Torokhov wrote:
> > On Thu, Apr 09, 2015 at 04:57:59PM -0700, Dmitry Torokhov wrote:
> > > Atmel MXT devices use different i2c addresses, depending on the current
> > > mode of operation (bootloader or application). The new Atmel MXT driver
> > > expects i2c client's address contain the application address of the
> > > chip, and calculates the expected bootloader address form the
> > > application address. Unfortunately chromeos_laptop does probe the
> > > devices and if touchpad (or touchscreen, or both) comes up in bootloader
> > > mode, the i2c device gets instantiated with the bootloader address
> > > instead of application address, which confuses the driver.
> > > 
> > > Given that hardware on Pixel is set and is not going to change let's not
> > > try to probe devices to see if they are present or not, but rather
> > > instantiate them always at expected addresses.
> > > 
> > > Since all devices are now probed and/or instantiated at given address,
> > > we no longer need to support probing multiple addresses for the same
> > 
> > Hmm, that strategy won't work on C720 since there are devices with touchscreen
> > and without one, so we do want to probe but always instantiate at primary
> > address. V3 will be upcoming...
> 
> OK, new version. Not sending to the wide world for now in case we decide

... and I failed...

> it is too ugly...
> 
> 
> From 480ea02024a0b9a2ad2f91e2e0ca02f34577972c Mon Sep 17 00:00:00 2001
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Mon, 12 Jan 2015 13:59:32 -0800
> Subject: [PATCH] platform/chrome: chromeos_laptop - instantiate Atmel at
>  primary address
> 
> The new Atmel MXT driver expects i2c client's address contain the
> primary (main address) of the chip, and calculates the expected
> bootloader address form the primary address. Unfortunately chrome_laptop
> does probe the devices and if touchpad (or touchscreen, or both) comes
> up in bootloader mode the i2c device gets instantiated with the
> bootloader address which confuses the driver.
> 
> To work around this issue let's probe the primary address first. If the
> device is not detected at the primary address we'll probe alternative
> addresses as "dummy" devices. If any of them are found, destroy the
> dummy client and instantiate client with proper name at primary address
> still.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/platform/chrome/chromeos_laptop.c | 35 +++++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/chrome/chromeos_laptop.c b/drivers/platform/chrome/chromeos_laptop.c
> index b84fdd6..a04019a 100644
> --- a/drivers/platform/chrome/chromeos_laptop.c
> +++ b/drivers/platform/chrome/chromeos_laptop.c
> @@ -133,12 +133,13 @@ static struct i2c_client *__add_probed_i2c_device(
>  		const char *name,
>  		int bus,
>  		struct i2c_board_info *info,
> -		const unsigned short *addrs)
> +		const unsigned short *alt_addr_list)
>  {
>  	const struct dmi_device *dmi_dev;
>  	const struct dmi_dev_onboard *dev_data;
>  	struct i2c_adapter *adapter;
> -	struct i2c_client *client;
> +	struct i2c_client *client = NULL;
> +	const unsigned short addr_list[] = { info->addr, I2C_CLIENT_END };
>  
>  	if (bus < 0)
>  		return NULL;
> @@ -169,8 +170,28 @@ static struct i2c_client *__add_probed_i2c_device(
>  		return NULL;
>  	}
>  
> -	/* add the i2c device */
> -	client = i2c_new_probed_device(adapter, info, addrs, NULL);
> +	/*
> +	 * Add the i2c device. If we can't detect it at the primary
> +	 * address we scan secondary addresses. In any case the client
> +	 * structure gets assigned primary address.
> +	 */
> +	client = i2c_new_probed_device(adapter, info, addr_list, NULL);
> +	if (!client && alt_addr_list) {
> +		struct i2c_board_info dummy_info = {
> +			I2C_BOARD_INFO("dummy", info->addr),
> +		};
> +		struct i2c_client *dummy;
> +
> +		dummy = i2c_new_probed_device(adapter, &dummy_info,
> +					      alt_addr_list, NULL);
> +		if (dummy) {
> +			pr_debug("%s %d-%02x is probed at %02x\n",
> +				  __func__, bus, info->addr, dummy->addr);
> +			i2c_unregister_device(dummy);
> +			client = i2c_new_device(adapter, info);
> +		}
> +	}
> +
>  	if (!client)
>  		pr_notice("%s failed to register device %d-%02x\n",
>  			  __func__, bus, info->addr);
> @@ -254,12 +275,10 @@ static struct i2c_client *add_i2c_device(const char *name,
>  						enum i2c_adapter_type type,
>  						struct i2c_board_info *info)
>  {
> -	const unsigned short addr_list[] = { info->addr, I2C_CLIENT_END };
> -
>  	return __add_probed_i2c_device(name,
>  				       find_i2c_adapter_num(type),
>  				       info,
> -				       addr_list);
> +				       NULL);
>  }
>  
>  static int setup_cyapa_tp(enum i2c_adapter_type type)
> @@ -275,7 +294,6 @@ static int setup_cyapa_tp(enum i2c_adapter_type type)
>  static int setup_atmel_224s_tp(enum i2c_adapter_type type)
>  {
>  	const unsigned short addr_list[] = { ATMEL_TP_I2C_BL_ADDR,
> -					     ATMEL_TP_I2C_ADDR,
>  					     I2C_CLIENT_END };
>  	if (tp)
>  		return 0;
> @@ -289,7 +307,6 @@ static int setup_atmel_224s_tp(enum i2c_adapter_type type)
>  static int setup_atmel_1664s_ts(enum i2c_adapter_type type)
>  {
>  	const unsigned short addr_list[] = { ATMEL_TS_I2C_BL_ADDR,
> -					     ATMEL_TS_I2C_ADDR,
>  					     I2C_CLIENT_END };
>  	if (ts)
>  		return 0;
> -- 
> 2.2.0.rc0.207.ga3a616c
> 
> 
> -- 
> Dmitry

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1
  2015-04-14 20:50   ` Dmitry Torokhov
  2015-04-14 20:50     ` Dmitry Torokhov
@ 2015-04-25  5:09     ` Olof Johansson
  1 sibling, 0 replies; 5+ messages in thread
From: Olof Johansson @ 2015-04-25  5:09 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Benson Leung, Nick Dyer, linux-kernel

On Tue, Apr 14, 2015 at 01:50:09PM -0700, Dmitry Torokhov wrote:
> On Fri, Apr 10, 2015 at 10:41:54AM -0700, Dmitry Torokhov wrote:
> > On Thu, Apr 09, 2015 at 04:57:59PM -0700, Dmitry Torokhov wrote:
> > > Atmel MXT devices use different i2c addresses, depending on the current
> > > mode of operation (bootloader or application). The new Atmel MXT driver
> > > expects i2c client's address contain the application address of the
> > > chip, and calculates the expected bootloader address form the
> > > application address. Unfortunately chromeos_laptop does probe the
> > > devices and if touchpad (or touchscreen, or both) comes up in bootloader
> > > mode, the i2c device gets instantiated with the bootloader address
> > > instead of application address, which confuses the driver.
> > > 
> > > Given that hardware on Pixel is set and is not going to change let's not
> > > try to probe devices to see if they are present or not, but rather
> > > instantiate them always at expected addresses.
> > > 
> > > Since all devices are now probed and/or instantiated at given address,
> > > we no longer need to support probing multiple addresses for the same
> > 
> > Hmm, that strategy won't work on C720 since there are devices with touchscreen
> > and without one, so we do want to probe but always instantiate at primary
> > address. V3 will be upcoming...
> 
> OK, new version. Not sending to the wide world for now in case we decide
> it is too ugly...

I'm not a huge fan of this, but as mentioned in person, the whole situation is
somewhat awkward and we don't really have any better ideas.

I've applied this now, and will include it in the 4.1 branch since it's needed
for stable operation on pixel 1 going forward.


-Olof

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-25  5:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-09 23:57 [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1 Dmitry Torokhov
2015-04-10 17:41 ` Dmitry Torokhov
2015-04-14 20:50   ` Dmitry Torokhov
2015-04-14 20:50     ` Dmitry Torokhov
2015-04-25  5:09     ` Olof Johansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox