Re: [PATCH v2] platform/chrome: chromeos_laptop - do not probe devices on Pixel 1

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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