2013-07-18 00:21:24

by Benson Leung

[permalink] [raw]
Subject: [PATCH 0/2] Platform: x86: chromeos_laptop - Deferred Probing

The following patch series refactors the dmi check system and
returns -EPROBE_DEFER when an expected i2c adapter is not present
at probe time.

This will allow the touchpad, touchscreen, and light sensors on
Pixel to load even if the i915 DDC and PANEL buses are instantiated
after chromeos_laptop.

[PATCH 1/2] Platform: x86: chromeos_laptop - Restructure device associations
[PATCH 2/2] Platform: x86: chromeos_laptop - Use deferred probing


2013-07-18 00:21:28

by Benson Leung

[permalink] [raw]
Subject: [PATCH 1/2] Platform: x86: chromeos_laptop - Restructure device associations

From: Aaron Durbin <[email protected]>

The previous code had a single DMI matching entry
for each device on a board. Instead provide a single
DMI entry for each board which references a structure
about each board that lists the associated peripherals.
This allows for a lower number of DMI matching sequences
as well making it easier to add new boards.

Signed-off-by: Aaron Durbin <[email protected]>
Signed-off-by: Benson Leung <[email protected]>
---
drivers/platform/x86/chromeos_laptop.c | 197 +++++++++++++++++++++------------
1 file changed, 126 insertions(+), 71 deletions(-)

diff --git a/drivers/platform/x86/chromeos_laptop.c b/drivers/platform/x86/chromeos_laptop.c
index 3e5b4497..086f347 100644
--- a/drivers/platform/x86/chromeos_laptop.c
+++ b/drivers/platform/x86/chromeos_laptop.c
@@ -53,6 +53,17 @@ enum i2c_adapter_type {
I2C_ADAPTER_PANEL,
};

+struct i2c_peripheral {
+ void (*add)(enum i2c_adapter_type type);
+ enum i2c_adapter_type type;
+};
+
+#define MAX_I2C_PERIPHERALS 3
+
+struct chromeos_laptop {
+ struct i2c_peripheral i2c_peripherals[MAX_I2C_PERIPHERALS];
+};
+
static struct i2c_board_info __initdata cyapa_device = {
I2C_BOARD_INFO("cyapa", CYAPA_TP_I2C_ADDR),
.flags = I2C_CLIENT_WAKE,
@@ -233,149 +244,193 @@ static __init struct i2c_client *add_i2c_device(const char *name,
addr_list);
}

-
-static struct i2c_client __init *add_smbus_device(const char *name,
- struct i2c_board_info *info)
-{
- return add_i2c_device(name, I2C_ADAPTER_SMBUS, info);
-}
-
-static int __init setup_cyapa_smbus_tp(const struct dmi_system_id *id)
+static int __init setup_cyapa_tp(enum i2c_adapter_type type)
{
- /* add cyapa touchpad on smbus */
- tp = add_smbus_device("trackpad", &cyapa_device);
+ /* add cyapa touchpad */
+ tp = add_i2c_device("trackpad", type, &cyapa_device);
return 0;
}

-static int __init setup_atmel_224s_tp(const struct dmi_system_id *id)
+static int __init 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 };

- /* add atmel mxt touchpad on VGA DDC GMBus */
- tp = add_probed_i2c_device("trackpad", I2C_ADAPTER_VGADDC,
+ /* add atmel mxt touchpad */
+ tp = add_probed_i2c_device("trackpad", type,
&atmel_224s_tp_device, addr_list);
return 0;
}

-static int __init setup_atmel_1664s_ts(const struct dmi_system_id *id)
+static int __init 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 };

- /* add atmel mxt touch device on PANEL GMBus */
- ts = add_probed_i2c_device("touchscreen", I2C_ADAPTER_PANEL,
+ /* add atmel mxt touch device */
+ ts = add_probed_i2c_device("touchscreen", type,
&atmel_1664s_device, addr_list);
return 0;
}

-
-static int __init setup_isl29018_als(const struct dmi_system_id *id)
+static int __init setup_isl29018_als(enum i2c_adapter_type type)
{
/* add isl29018 light sensor */
- als = add_smbus_device("lightsensor", &isl_als_device);
+ als = add_i2c_device("lightsensor", type, &isl_als_device);
return 0;
}

-static int __init setup_isl29023_als(const struct dmi_system_id *id)
+static int __init setup_tsl2583_als(enum i2c_adapter_type type)
{
- /* add isl29023 light sensor on Panel GMBus */
- als = add_i2c_device("lightsensor", I2C_ADAPTER_PANEL,
- &isl_als_device);
+ /* add tsl2583 light sensor */
+ als = add_i2c_device(NULL, type, &tsl2583_als_device);
return 0;
}

-static int __init setup_tsl2583_als(const struct dmi_system_id *id)
+static int __init setup_tsl2563_als(enum i2c_adapter_type type)
{
- /* add tsl2583 light sensor on smbus */
- als = add_smbus_device(NULL, &tsl2583_als_device);
+ /* add tsl2563 light sensor */
+ als = add_i2c_device(NULL, type, &tsl2563_als_device);
return 0;
}

-static int __init setup_tsl2563_als(const struct dmi_system_id *id)
+static int __init
+chromeos_laptop_add_peripherals(const struct dmi_system_id *id)
{
- /* add tsl2563 light sensor on smbus */
- als = add_smbus_device(NULL, &tsl2563_als_device);
- return 0;
+ int i;
+ struct chromeos_laptop *cros_laptop = (void *)id->driver_data;
+
+ pr_debug("Adding peripherals for %s.\n", id->ident);
+
+ for (i = 0; i < MAX_I2C_PERIPHERALS; i++) {
+ struct i2c_peripheral *i2c_dev;
+
+ i2c_dev = &cros_laptop->i2c_peripherals[i];
+
+ /* No more peripherals. */
+ if (i2c_dev->add == NULL)
+ break;
+
+ /* Add the device. */
+ i2c_dev->add(i2c_dev->type);
+ }
+
+ /* Indicate to dmi_scan that processing is done. */
+ return 1;
}

+static struct chromeos_laptop __initdata samsung_series_5_550 = {
+ .i2c_peripherals = {
+ /* Touchpad. */
+ { .add = setup_cyapa_tp, I2C_ADAPTER_SMBUS },
+ /* Light Sensor. */
+ { .add = setup_isl29018_als, I2C_ADAPTER_SMBUS },
+ },
+};
+
+static struct chromeos_laptop __initdata samsung_series_5 = {
+ .i2c_peripherals = {
+ /* Light Sensor. */
+ { .add = setup_tsl2583_als, I2C_ADAPTER_SMBUS },
+ },
+};
+
+static struct chromeos_laptop __initdata chromebook_pixel = {
+ .i2c_peripherals = {
+ /* Touch Screen. */
+ { .add = setup_atmel_1664s_ts, I2C_ADAPTER_PANEL },
+ /* Touchpad. */
+ { .add = setup_atmel_224s_tp, I2C_ADAPTER_VGADDC },
+ /* Light Sensor. */
+ { .add = setup_isl29018_als, I2C_ADAPTER_PANEL },
+ },
+};
+
+static struct chromeos_laptop __initdata acer_c7_chromebook = {
+ .i2c_peripherals = {
+ /* Touchpad. */
+ { .add = setup_cyapa_tp, I2C_ADAPTER_SMBUS },
+ },
+};
+
+static struct chromeos_laptop __initdata acer_ac700 = {
+ .i2c_peripherals = {
+ /* Light Sensor. */
+ { .add = setup_tsl2563_als, I2C_ADAPTER_SMBUS },
+ },
+};
+
+static struct chromeos_laptop __initdata hp_pavilion_14_chromebook = {
+ .i2c_peripherals = {
+ /* Touchpad. */
+ { .add = setup_cyapa_tp, I2C_ADAPTER_SMBUS },
+ },
+};
+
+static struct chromeos_laptop __initdata cr48 = {
+ .i2c_peripherals = {
+ /* Light Sensor. */
+ { .add = setup_tsl2563_als, I2C_ADAPTER_SMBUS },
+ },
+};
+
+#define _CBDD(board_) \
+ .callback = &chromeos_laptop_add_peripherals, \
+ .driver_data = (void *)&board_
+
static struct dmi_system_id __initdata chromeos_laptop_dmi_table[] = {
{
- .ident = "Samsung Series 5 550 - Touchpad",
+ .ident = "Samsung Series 5 550",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG"),
DMI_MATCH(DMI_PRODUCT_NAME, "Lumpy"),
},
- .callback = setup_cyapa_smbus_tp,
- },
- {
- .ident = "Chromebook Pixel - Touchscreen",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Link"),
- },
- .callback = setup_atmel_1664s_ts,
- },
- {
- .ident = "Chromebook Pixel - Touchpad",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Link"),
- },
- .callback = setup_atmel_224s_tp,
+ _CBDD(samsung_series_5_550),
},
{
- .ident = "Samsung Series 5 550 - Light Sensor",
+ .ident = "Samsung Series 5",
.matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Lumpy"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Alex"),
},
- .callback = setup_isl29018_als,
+ _CBDD(samsung_series_5),
},
{
- .ident = "Chromebook Pixel - Light Sensor",
+ .ident = "Chromebook Pixel",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
DMI_MATCH(DMI_PRODUCT_NAME, "Link"),
},
- .callback = setup_isl29023_als,
+ _CBDD(chromebook_pixel),
},
{
- .ident = "Acer C7 Chromebook - Touchpad",
+ .ident = "Acer C7 Chromebook",
.matches = {
DMI_MATCH(DMI_PRODUCT_NAME, "Parrot"),
},
- .callback = setup_cyapa_smbus_tp,
+ _CBDD(acer_c7_chromebook),
},
{
- .ident = "HP Pavilion 14 Chromebook - Touchpad",
+ .ident = "Acer AC700",
.matches = {
- DMI_MATCH(DMI_PRODUCT_NAME, "Butterfly"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ZGB"),
},
- .callback = setup_cyapa_smbus_tp,
+ _CBDD(acer_ac700),
},
{
- .ident = "Samsung Series 5 - Light Sensor",
+ .ident = "HP Pavilion 14 Chromebook",
.matches = {
- DMI_MATCH(DMI_PRODUCT_NAME, "Alex"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Butterfly"),
},
- .callback = setup_tsl2583_als,
+ _CBDD(hp_pavilion_14_chromebook),
},
{
- .ident = "Cr-48 - Light Sensor",
+ .ident = "Cr-48",
.matches = {
DMI_MATCH(DMI_PRODUCT_NAME, "Mario"),
},
- .callback = setup_tsl2563_als,
- },
- {
- .ident = "Acer AC700 - Light Sensor",
- .matches = {
- DMI_MATCH(DMI_PRODUCT_NAME, "ZGB"),
- },
- .callback = setup_tsl2563_als,
+ _CBDD(cr48),
},
{ }
};
--
1.8.3

2013-07-18 00:21:41

by Benson Leung

[permalink] [raw]
Subject: [PATCH 2/2] Platform: x86: chromeos_laptop - Use deferred probing

Further refactor chromeos_laptop, adding a probe function.
Init will call dmi_check_system, but will only use the match to select
a chromeos_laptop structure of the current board.

Probe will add the devices, and on errors return -EPROBE_DEFER.
If i2c adapters are loaded after chromeos_laptop inits, the deferred
probe will instantiate the peripherals when the bus appears.

Signed-off-by: Benson Leung <[email protected]>
---
drivers/platform/x86/chromeos_laptop.c | 139 +++++++++++++++++++++++----------
1 file changed, 98 insertions(+), 41 deletions(-)

diff --git a/drivers/platform/x86/chromeos_laptop.c b/drivers/platform/x86/chromeos_laptop.c
index 086f347..fc64f65 100644
--- a/drivers/platform/x86/chromeos_laptop.c
+++ b/drivers/platform/x86/chromeos_laptop.c
@@ -27,6 +27,7 @@
#include <linux/input.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/platform_device.h>

#define ATMEL_TP_I2C_ADDR 0x4b
#define ATMEL_TP_I2C_BL_ADDR 0x25
@@ -54,7 +55,7 @@ enum i2c_adapter_type {
};

struct i2c_peripheral {
- void (*add)(enum i2c_adapter_type type);
+ int (*add)(enum i2c_adapter_type type);
enum i2c_adapter_type type;
};

@@ -64,20 +65,22 @@ struct chromeos_laptop {
struct i2c_peripheral i2c_peripherals[MAX_I2C_PERIPHERALS];
};

-static struct i2c_board_info __initdata cyapa_device = {
+static struct chromeos_laptop *cros_laptop;
+
+static struct i2c_board_info cyapa_device = {
I2C_BOARD_INFO("cyapa", CYAPA_TP_I2C_ADDR),
.flags = I2C_CLIENT_WAKE,
};

-static struct i2c_board_info __initdata isl_als_device = {
+static struct i2c_board_info isl_als_device = {
I2C_BOARD_INFO("isl29018", ISL_ALS_I2C_ADDR),
};

-static struct i2c_board_info __initdata tsl2583_als_device = {
+static struct i2c_board_info tsl2583_als_device = {
I2C_BOARD_INFO("tsl2583", TAOS_ALS_I2C_ADDR),
};

-static struct i2c_board_info __initdata tsl2563_als_device = {
+static struct i2c_board_info tsl2563_als_device = {
I2C_BOARD_INFO("tsl2563", TAOS_ALS_I2C_ADDR),
};

@@ -100,7 +103,7 @@ static struct mxt_platform_data atmel_224s_tp_platform_data = {
.config_length = 0,
};

-static struct i2c_board_info __initdata atmel_224s_tp_device = {
+static struct i2c_board_info atmel_224s_tp_device = {
I2C_BOARD_INFO("atmel_mxt_tp", ATMEL_TP_I2C_ADDR),
.platform_data = &atmel_224s_tp_platform_data,
.flags = I2C_CLIENT_WAKE,
@@ -121,13 +124,13 @@ static struct mxt_platform_data atmel_1664s_platform_data = {
.config_length = 0,
};

-static struct i2c_board_info __initdata atmel_1664s_device = {
+static struct i2c_board_info atmel_1664s_device = {
I2C_BOARD_INFO("atmel_mxt_ts", ATMEL_TS_I2C_ADDR),
.platform_data = &atmel_1664s_platform_data,
.flags = I2C_CLIENT_WAKE,
};

-static struct i2c_client __init *__add_probed_i2c_device(
+static struct i2c_client *__add_probed_i2c_device(
const char *name,
int bus,
struct i2c_board_info *info,
@@ -180,7 +183,7 @@ static struct i2c_client __init *__add_probed_i2c_device(
return client;
}

-static int __init __find_i2c_adap(struct device *dev, void *data)
+static int __find_i2c_adap(struct device *dev, void *data)
{
const char *name = data;
static const char *prefix = "i2c-";
@@ -191,7 +194,7 @@ static int __init __find_i2c_adap(struct device *dev, void *data)
return (strncmp(adapter->name, name, strlen(name)) == 0);
}

-static int __init find_i2c_adapter_num(enum i2c_adapter_type type)
+static int find_i2c_adapter_num(enum i2c_adapter_type type)
{
struct device *dev = NULL;
struct i2c_adapter *adapter;
@@ -216,7 +219,7 @@ static int __init find_i2c_adapter_num(enum i2c_adapter_type type)
* Returns NULL if no devices found.
* See Documentation/i2c/instantiating-devices for more information.
*/
-static __init struct i2c_client *add_probed_i2c_device(
+static struct i2c_client *add_probed_i2c_device(
const char *name,
enum i2c_adapter_type type,
struct i2c_board_info *info,
@@ -233,7 +236,7 @@ static __init struct i2c_client *add_probed_i2c_device(
* info->addr.
* Returns NULL if no device found.
*/
-static __init struct i2c_client *add_i2c_device(const char *name,
+static struct i2c_client *add_i2c_device(const char *name,
enum i2c_adapter_type type,
struct i2c_board_info *info)
{
@@ -244,65 +247,87 @@ static __init struct i2c_client *add_i2c_device(const char *name,
addr_list);
}

-static int __init setup_cyapa_tp(enum i2c_adapter_type type)
+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);
- return 0;
+ return (!tp) ? -EAGAIN : 0;
}

-static int __init setup_atmel_224s_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;

/* add atmel mxt touchpad */
tp = add_probed_i2c_device("trackpad", type,
&atmel_224s_tp_device, addr_list);
- return 0;
+ return (!tp) ? -EAGAIN : 0;
}

-static int __init setup_atmel_1664s_ts(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;

/* add atmel mxt touch device */
ts = add_probed_i2c_device("touchscreen", type,
&atmel_1664s_device, addr_list);
- return 0;
+ return (!ts) ? -EAGAIN : 0;
}

-static int __init setup_isl29018_als(enum i2c_adapter_type type)
+static int setup_isl29018_als(enum i2c_adapter_type type)
{
+ if (als)
+ return 0;
+
/* add isl29018 light sensor */
als = add_i2c_device("lightsensor", type, &isl_als_device);
- return 0;
+ return (!als) ? -EAGAIN : 0;
}

-static int __init setup_tsl2583_als(enum i2c_adapter_type type)
+static int setup_tsl2583_als(enum i2c_adapter_type type)
{
+ if (als)
+ return 0;
+
/* add tsl2583 light sensor */
als = add_i2c_device(NULL, type, &tsl2583_als_device);
- return 0;
+ return (!als) ? -EAGAIN : 0;
}

-static int __init setup_tsl2563_als(enum i2c_adapter_type type)
+static int setup_tsl2563_als(enum i2c_adapter_type type)
{
+ if (als)
+ return 0;
+
/* add tsl2563 light sensor */
als = add_i2c_device(NULL, type, &tsl2563_als_device);
- return 0;
+ return (!als) ? -EAGAIN : 0;
}

-static int __init
-chromeos_laptop_add_peripherals(const struct dmi_system_id *id)
+static int __init chromeos_laptop_dmi_matched(const struct dmi_system_id *id)
{
- int i;
- struct chromeos_laptop *cros_laptop = (void *)id->driver_data;
+ cros_laptop = (void *)id->driver_data;
+ pr_debug("DMI Matched %s.\n", id->ident);

- pr_debug("Adding peripherals for %s.\n", id->ident);
+ /* Indicate to dmi_scan that processing is done. */
+ return 1;
+}
+
+static int chromeos_laptop_probe(struct platform_device *pdev)
+{
+ int i;
+ int ret = 0;

for (i = 0; i < MAX_I2C_PERIPHERALS; i++) {
struct i2c_peripheral *i2c_dev;
@@ -313,15 +338,15 @@ chromeos_laptop_add_peripherals(const struct dmi_system_id *id)
if (i2c_dev->add == NULL)
break;

- /* Add the device. */
- i2c_dev->add(i2c_dev->type);
+ /* Add the device. Set -EPROBE_DEFER on any failure */
+ if (i2c_dev->add(i2c_dev->type))
+ ret = -EPROBE_DEFER;
}

- /* Indicate to dmi_scan that processing is done. */
- return 1;
+ return ret;
}

-static struct chromeos_laptop __initdata samsung_series_5_550 = {
+static struct chromeos_laptop samsung_series_5_550 = {
.i2c_peripherals = {
/* Touchpad. */
{ .add = setup_cyapa_tp, I2C_ADAPTER_SMBUS },
@@ -330,14 +355,14 @@ static struct chromeos_laptop __initdata samsung_series_5_550 = {
},
};

-static struct chromeos_laptop __initdata samsung_series_5 = {
+static struct chromeos_laptop samsung_series_5 = {
.i2c_peripherals = {
/* Light Sensor. */
{ .add = setup_tsl2583_als, I2C_ADAPTER_SMBUS },
},
};

-static struct chromeos_laptop __initdata chromebook_pixel = {
+static struct chromeos_laptop chromebook_pixel = {
.i2c_peripherals = {
/* Touch Screen. */
{ .add = setup_atmel_1664s_ts, I2C_ADAPTER_PANEL },
@@ -348,28 +373,28 @@ static struct chromeos_laptop __initdata chromebook_pixel = {
},
};

-static struct chromeos_laptop __initdata acer_c7_chromebook = {
+static struct chromeos_laptop acer_c7_chromebook = {
.i2c_peripherals = {
/* Touchpad. */
{ .add = setup_cyapa_tp, I2C_ADAPTER_SMBUS },
},
};

-static struct chromeos_laptop __initdata acer_ac700 = {
+static struct chromeos_laptop acer_ac700 = {
.i2c_peripherals = {
/* Light Sensor. */
{ .add = setup_tsl2563_als, I2C_ADAPTER_SMBUS },
},
};

-static struct chromeos_laptop __initdata hp_pavilion_14_chromebook = {
+static struct chromeos_laptop hp_pavilion_14_chromebook = {
.i2c_peripherals = {
/* Touchpad. */
{ .add = setup_cyapa_tp, I2C_ADAPTER_SMBUS },
},
};

-static struct chromeos_laptop __initdata cr48 = {
+static struct chromeos_laptop cr48 = {
.i2c_peripherals = {
/* Light Sensor. */
{ .add = setup_tsl2563_als, I2C_ADAPTER_SMBUS },
@@ -377,7 +402,7 @@ static struct chromeos_laptop __initdata cr48 = {
};

#define _CBDD(board_) \
- .callback = &chromeos_laptop_add_peripherals, \
+ .callback = chromeos_laptop_dmi_matched, \
.driver_data = (void *)&board_

static struct dmi_system_id __initdata chromeos_laptop_dmi_table[] = {
@@ -436,13 +461,45 @@ static struct dmi_system_id __initdata chromeos_laptop_dmi_table[] = {
};
MODULE_DEVICE_TABLE(dmi, chromeos_laptop_dmi_table);

+static struct platform_device *cros_platform_device;
+
+static struct platform_driver cros_platform_driver = {
+ .driver = {
+ .name = "chromeos_laptop",
+ .owner = THIS_MODULE,
+ },
+ .probe = chromeos_laptop_probe,
+};
+
static int __init chromeos_laptop_init(void)
{
+ int ret;
if (!dmi_check_system(chromeos_laptop_dmi_table)) {
pr_debug("%s unsupported system.\n", __func__);
return -ENODEV;
}
+
+ ret = platform_driver_register(&cros_platform_driver);
+ if (ret)
+ return ret;
+
+ cros_platform_device = platform_device_alloc("chromeos_laptop", -1);
+ if (!cros_platform_device) {
+ ret = -ENOMEM;
+ goto fail_platform_device1;
+ }
+
+ ret = platform_device_add(cros_platform_device);
+ if (ret)
+ goto fail_platform_device2;
+
return 0;
+
+fail_platform_device2:
+ platform_device_put(cros_platform_device);
+fail_platform_device1:
+ platform_driver_unregister(&cros_platform_driver);
+ return ret;
}

static void __exit chromeos_laptop_exit(void)
--
1.8.3

2013-07-18 16:13:38

by Martin Nordholts

[permalink] [raw]
Subject: Re: [PATCH 0/2] Platform: x86: chromeos_laptop - Deferred Probing

2013/7/18 Benson Leung <[email protected]>
>
> The following patch series refactors the dmi check system and
> returns -EPROBE_DEFER when an expected i2c adapter is not present
> at probe time.
>
> This will allow the touchpad, touchscreen, and light sensors on
> Pixel to load even if the i915 DDC and PANEL buses are instantiated
> after chromeos_laptop.
>
> [PATCH 1/2] Platform: x86: chromeos_laptop - Restructure device associations
> [PATCH 2/2] Platform: x86: chromeos_laptop - Use deferred probing


Hi,

I have worked on the same problem for the last days and just submitted
my solution proposal for review:
http://www.mail-archive.com/[email protected]/msg04440.html

One downside with the solution in this set of patches is that more
lines are added to the driver.
By making use of the i2c_driver.detect() mechanism like in my patch,
we can actually reduce the number of lines in the driver.

Looking forward to your comments on my other solution proposal!

/ Martin

2013-08-05 04:05:53

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH 0/2] Platform: x86: chromeos_laptop - Deferred Probing

Hi Martin,

I commented on your patch, but I want to add a little bit more in response here.

On Thu, Jul 18, 2013 at 9:13 AM, Martin Nordholts <[email protected]> wrote:
> One downside with the solution in this set of patches is that more
> lines are added to the driver.
> By making use of the i2c_driver.detect() mechanism like in my patch,
> we can actually reduce the number of lines in the driver.
>

It looks like the vast majority of the savings in number of lines of
code in your patch is from removing the board specific enumeration of
peripherals.

For example, in my patch, I have a data structure that describes the
chromebook pixel thusly :

static struct chromeos_laptop chromebook_pixel = {
.i2c_peripherals = {
/* Touch Screen. */
{ .add = setup_atmel_1664s_ts, I2C_ADAPTER_PANEL },
/* Touchpad. */
{ .add = setup_atmel_224s_tp, I2C_ADAPTER_VGADDC },
/* Light Sensor. */
{ .add = setup_isl29018_als, I2C_ADAPTER_PANEL },
},
};

And so on for every other board that the driver supports. It
explicitly describes the small set of devices that are known to exist
on a particular system, and describes precisely which bus it lives on.
That way, we can use i2c_new_probed_device.

Your patch removes these, and instead, makes one list of all devices
that the driver supports across all systems that pass the dmi check.
Your driver then uses detect in sort of a shotgun approach for all
supported i2c adapters. The approach may work for Pixel, but as I
mentioned in my other email, it causes failed probes on other systems.

It would be my preference to continue to use i2c_new_probed_device,
and explicitly describe each Chromebook system



--
Benson Leung
Software Engineer, Chrom* OS
[email protected]