On Tue, Jun 10, 2014 at 06:17:35AM +0100, Scot Doyle wrote:
> The chromeos_laptop module probes the first i2c adapter with a specific name
> for expected hardware. However, the Acer C720 Chromebook has two i2c
> adapters with the same name. This patch probes each i2c adapter with a
> specific name in turn, until locating the expected hardware.
> 
> Thanks to Mika Westerberg for identifying the need for unique bus addresses
> within each set of identically-named adapters.
> 
> Signed-off-by: Scot Doyle <lkm...@scotdoyle.com>
> CC: Benson Leung <ble...@chromium.org>
> CC: Mika Westerberg <mika.westerb...@linux.intel.com>

Few comments about style:

> ---
> diff --git a/drivers/platform/chrome/chromeos_laptop.c
> b/drivers/platform/chrome/chromeos_laptop.c
> index 7f3aad0..8382315 100644
> --- a/drivers/platform/chrome/chromeos_laptop.c
> +++ b/drivers/platform/chrome/chromeos_laptop.c
> @@ -29,6 +29,8 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> 
> +/* Note: Bus addresses must be unique within each set of identically-named
> +      adapters on a board, otherwise device probing may fail. */

Check Documentation/CodingStyle about format of block comments.

>  #define ATMEL_TP_I2C_ADDR    0x4b
>  #define ATMEL_TP_I2C_BL_ADDR 0x25
>  #define ATMEL_TS_I2C_ADDR    0x4a
> @@ -173,7 +175,7 @@ static struct i2c_client *__add_probed_i2c_device(
>       /* add the i2c device */
>       client = i2c_new_probed_device(adapter, info, addrs, NULL);
>       if (!client)
> -             pr_err("%s failed to register device %d-%02x\n",
> +             pr_debug("%s did not add i2c device %d-%02x\n",
>                      __func__, bus, info->addr);
>       else
>               pr_debug("%s added i2c device %d-%02x\n",
> @@ -194,13 +196,14 @@ static int __find_i2c_adap(struct device *dev, void 
> *data)
>       return (strncmp(adapter->name, name, strlen(name)) == 0);
>  }
> 
> -static int find_i2c_adapter_num(enum i2c_adapter_type type)
> +static int find_i2c_next_adapter_num(enum i2c_adapter_type type,
> +                                  struct device *start)
>  {
>       struct device *dev = NULL;
>       struct i2c_adapter *adapter;
>       const char *name = i2c_adapter_names[type];
> -     /* find the adapter by name */
> -     dev = bus_find_device(&i2c_bus_type, NULL, (void *)name,
> +     /* find the next adapter by name */
> +     dev = bus_find_device(&i2c_bus_type, start, (void *)name,
>                             __find_i2c_adap);
>       if (!dev) {
>               /* Adapters may appear later. Deferred probing will retry */
> @@ -226,10 +229,17 @@ static struct i2c_client *add_probed_i2c_device(
>               struct i2c_board_info *info,
>               const unsigned short *addrs)
>  {
> -     return __add_probed_i2c_device(name,
> -                                    find_i2c_adapter_num(type),
> -                                    info,
> -                                    addrs);
> +     struct i2c_client *client = NULL;
> +     struct device *start = NULL;
> +     int adapter_num = 0;

Please add blank line here.

> +     while (!client && adapter_num > -1) {
> +             adapter_num = find_i2c_next_adapter_num(type, start);
> +             client = __add_probed_i2c_device(name,
> +                                              adapter_num,
> +                                              info,
> +                                              addrs);

Don't the above fit into one line, specifically if you call adapter_num
just num or something like that.

> +     }
> +     return client;
>  }
> 
>  /*
> @@ -242,10 +252,10 @@ static struct i2c_client *add_i2c_device(const char 
> *name,
>                                               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);
> +     return add_probed_i2c_device(name,
> +                                  type,
> +                                  info,
> +                                  addr_list);

Same here.

>  }
> 
>  static int setup_cyapa_tp(enum i2c_adapter_type type)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to