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 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
ble...@chromium.org
--
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/
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 ense...@gmail.com 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
ble...@chromium.org
--
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/
Re: [PATCH 0/2] Platform: x86: chromeos_laptop - Deferred Probing
2013/7/18 Benson Leung > > 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/platform-driver-x86@vger.kernel.org/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 -- 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/
Re: [PATCH 0/2] Platform: x86: chromeos_laptop - Deferred Probing
2013/7/18 Benson Leung ble...@chromium.org 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/platform-driver-x86@vger.kernel.org/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 -- 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/
