2013-04-18 21:55:52

by Olof Johansson

[permalink] [raw]
Subject: [PATCH] Platform: x86: chromeos_laptop: defer probing if no i2c busses found

If chromeos_laptop is loaded before the DRM driver, the i2c busses will
not yet be available. Defer probe for that case and try again later.

Trickling the error up to the module init function is unfortuantely a
little awkward, since the i2c bus lookup and device registration happens
as a dmi callback.

Reported-by: Dirk Hohndel <[email protected]>
Signed-off-by: Olof Johansson <[email protected]>
---
drivers/platform/x86/chromeos_laptop.c | 33 ++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/chromeos_laptop.c b/drivers/platform/x86/chromeos_laptop.c
index 3e5b4497..7ff12fa 100644
--- a/drivers/platform/x86/chromeos_laptop.c
+++ b/drivers/platform/x86/chromeos_laptop.c
@@ -39,6 +39,7 @@
static struct i2c_client *als;
static struct i2c_client *tp;
static struct i2c_client *ts;
+static int setup_error;

const char *i2c_adapter_names[] = {
"SMBus I801 adapter",
@@ -191,7 +192,8 @@ static int __init find_i2c_adapter_num(enum i2c_adapter_type type)
if (!dev) {
pr_err("%s: i2c adapter %s not found on system.\n", __func__,
name);
+ setup_error = -EPROBE_DEFER;
return -ENODEV;
}
adapter = to_i2c_adapter(dev);
return adapter->nr;
@@ -381,23 +383,38 @@ static struct dmi_system_id __initdata chromeos_laptop_dmi_table[] = {
};
MODULE_DEVICE_TABLE(dmi, chromeos_laptop_dmi_table);

+static void chromeos_laptop_unregister(void)
+{
+ if (als)
+ i2c_unregister_device(als);
+ if (tp)
+ i2c_unregister_device(tp);
+ if (ts)
+ i2c_unregister_device(ts);
+
+ als = NULL;
+ tp = NULL;
+ ts = NULL;
+}
+
static int __init chromeos_laptop_init(void)
{
+ setup_error = 0;
+
if (!dmi_check_system(chromeos_laptop_dmi_table)) {
pr_debug("%s unsupported system.\n", __func__);
return -ENODEV;
}
- return 0;
+
+ if (setup_error)
+ chromeos_laptop_unregister();
+
+ return setup_error;
}

static void __exit chromeos_laptop_exit(void)
{
- if (als)
- i2c_unregister_device(als);
- if (tp)
- i2c_unregister_device(tp);
- if (ts)
- i2c_unregister_device(ts);
+ chromeos_laptop_unregister();
}

module_init(chromeos_laptop_init);
--
1.7.10.4


2013-04-18 22:00:04

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH] Platform: x86: chromeos_laptop: defer probing if no i2c busses found

Acked-by: Benson Leung <[email protected]>

On Thu, Apr 18, 2013 at 2:55 PM, Olof Johansson <[email protected]> wrote:
> If chromeos_laptop is loaded before the DRM driver, the i2c busses will
> not yet be available. Defer probe for that case and try again later.
>
> Trickling the error up to the module init function is unfortuantely a
> little awkward, since the i2c bus lookup and device registration happens
> as a dmi callback.
>
> Reported-by: Dirk Hohndel <[email protected]>
> Signed-off-by: Olof Johansson <[email protected]>
> ---
> drivers/platform/x86/chromeos_laptop.c | 33 ++++++++++++++++++++++++--------
> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/chromeos_laptop.c b/drivers/platform/x86/chromeos_laptop.c
> index 3e5b4497..7ff12fa 100644
> --- a/drivers/platform/x86/chromeos_laptop.c
> +++ b/drivers/platform/x86/chromeos_laptop.c
> @@ -39,6 +39,7 @@
> static struct i2c_client *als;
> static struct i2c_client *tp;
> static struct i2c_client *ts;
> +static int setup_error;
>
> const char *i2c_adapter_names[] = {
> "SMBus I801 adapter",
> @@ -191,7 +192,8 @@ static int __init find_i2c_adapter_num(enum i2c_adapter_type type)
> if (!dev) {
> pr_err("%s: i2c adapter %s not found on system.\n", __func__,
> name);
> + setup_error = -EPROBE_DEFER;
> return -ENODEV;
> }
> adapter = to_i2c_adapter(dev);
> return adapter->nr;
> @@ -381,23 +383,38 @@ static struct dmi_system_id __initdata chromeos_laptop_dmi_table[] = {
> };
> MODULE_DEVICE_TABLE(dmi, chromeos_laptop_dmi_table);
>
> +static void chromeos_laptop_unregister(void)
> +{
> + if (als)
> + i2c_unregister_device(als);
> + if (tp)
> + i2c_unregister_device(tp);
> + if (ts)
> + i2c_unregister_device(ts);
> +
> + als = NULL;
> + tp = NULL;
> + ts = NULL;
> +}
> +
> static int __init chromeos_laptop_init(void)
> {
> + setup_error = 0;
> +
> if (!dmi_check_system(chromeos_laptop_dmi_table)) {
> pr_debug("%s unsupported system.\n", __func__);
> return -ENODEV;
> }
> - return 0;
> +
> + if (setup_error)
> + chromeos_laptop_unregister();
> +
> + return setup_error;
> }
>
> static void __exit chromeos_laptop_exit(void)
> {
> - if (als)
> - i2c_unregister_device(als);
> - if (tp)
> - i2c_unregister_device(tp);
> - if (ts)
> - i2c_unregister_device(ts);
> + chromeos_laptop_unregister();
> }
>
> module_init(chromeos_laptop_init);
> --
> 1.7.10.4
>



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

2013-04-18 22:05:50

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH] Platform: x86: chromeos_laptop: defer probing if no i2c busses found

On Thu, Apr 18, 2013 at 2:55 PM, Olof Johansson <[email protected]> wrote:
> If chromeos_laptop is loaded before the DRM driver, the i2c busses will
> not yet be available. Defer probe for that case and try again later.
>
> Trickling the error up to the module init function is unfortuantely a
> little awkward, since the i2c bus lookup and device registration happens
> as a dmi callback.

Uh, wait, hold off on this. I'm returning -EPROBE_DEFER from the
module init function. It's not valid outside of the driver model.


-Olof

2013-04-18 22:44:23

by Olof Johansson

[permalink] [raw]
Subject: Re: [PATCH] Platform: x86: chromeos_laptop: defer probing if no i2c busses found

On Thu, Apr 18, 2013 at 3:05 PM, Olof Johansson <[email protected]> wrote:
> On Thu, Apr 18, 2013 at 2:55 PM, Olof Johansson <[email protected]> wrote:
>> If chromeos_laptop is loaded before the DRM driver, the i2c busses will
>> not yet be available. Defer probe for that case and try again later.
>>
>> Trickling the error up to the module init function is unfortuantely a
>> little awkward, since the i2c bus lookup and device registration happens
>> as a dmi callback.
>
> Uh, wait, hold off on this. I'm returning -EPROBE_DEFER from the
> module init function. It's not valid outside of the driver model.

Ugh. Ok, so that worked just because any error seem to re-trigger an
attempt to load the module later on my system (three times, it seems),
and most of the reproductions I had happened to work out on the second
load.

Looks like keeping the module loaded and adding a bus notifier to wait
for the right busses to show up is the only way to do this in a
foolproof way. Revised patch tonight or tomorrow.


-Olof