[PATCH] i2c: i801: Prefer async probe

Mani Milani posted 1 patch 3 years, 7 months ago
drivers/i2c/busses/i2c-i801.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] i2c: i801: Prefer async probe
Posted by Mani Milani 3 years, 7 months ago
This i801 driver probe can take more than ~190ms in some devices, since
the "i2c_register_spd()" call was added inside
"i801_probe_optional_slaves()".

Prefer async probe so that other drivers can be probed and boot can
continue in parallel while this driver loads, to reduce boot time. There is
no reason to block other drivers from probing while this driver is
loading.

Signed-off-by: Mani Milani <mani@chromium.org>
---

 drivers/i2c/busses/i2c-i801.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index a176296f4fff..e06509edc5f3 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1838,6 +1838,7 @@ static struct pci_driver i801_driver = {
 	.shutdown	= i801_shutdown,
 	.driver		= {
 		.pm	= &i801_pm_ops,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 };
 
-- 
2.37.2.672.g94769d06f0-goog
Re: [PATCH] i2c: i801: Prefer async probe
Posted by Wolfram Sang 3 years, 6 months ago
On Fri, Aug 26, 2022 at 05:44:30PM +1000, Mani Milani wrote:
> This i801 driver probe can take more than ~190ms in some devices, since
> the "i2c_register_spd()" call was added inside
> "i801_probe_optional_slaves()".
> 
> Prefer async probe so that other drivers can be probed and boot can
> continue in parallel while this driver loads, to reduce boot time. There is
> no reason to block other drivers from probing while this driver is
> loading.
> 
> Signed-off-by: Mani Milani <mani@chromium.org>

Applied to for-next, thanks!

Re: [PATCH] i2c: i801: Prefer async probe
Posted by Jean Delvare 3 years, 6 months ago
On Fri, 26 Aug 2022 17:44:30 +1000, Mani Milani wrote:
> This i801 driver probe can take more than ~190ms in some devices, since
> the "i2c_register_spd()" call was added inside
> "i801_probe_optional_slaves()".
> 
> Prefer async probe so that other drivers can be probed and boot can
> continue in parallel while this driver loads, to reduce boot time. There is
> no reason to block other drivers from probing while this driver is
> loading.
> 
> Signed-off-by: Mani Milani <mani@chromium.org>
> ---
> 
>  drivers/i2c/busses/i2c-i801.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index a176296f4fff..e06509edc5f3 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1838,6 +1838,7 @@ static struct pci_driver i801_driver = {
>  	.shutdown	= i801_shutdown,
>  	.driver		= {
>  		.pm	= &i801_pm_ops,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
>  };
>  

Reviewed-by: Jean Delvare <jdelvare@suse.de>

-- 
Jean Delvare
SUSE L3 Support
Re: [PATCH] i2c: i801: Prefer async probe
Posted by Jean Delvare 3 years, 6 months ago
Hi Mani,

On Fri, 26 Aug 2022 17:44:30 +1000, Mani Milani wrote:
> This i801 driver probe can take more than ~190ms in some devices, since
> the "i2c_register_spd()" call was added inside
> "i801_probe_optional_slaves()".

Is there anything special about this 190 ms value?

> Prefer async probe so that other drivers can be probed and boot can
> continue in parallel while this driver loads, to reduce boot time. There is
> no reason to block other drivers from probing while this driver is
> loading.
> 
> Signed-off-by: Mani Milani <mani@chromium.org>
> ---
> 
>  drivers/i2c/busses/i2c-i801.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index a176296f4fff..e06509edc5f3 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1838,6 +1838,7 @@ static struct pci_driver i801_driver = {
>  	.shutdown	= i801_shutdown,
>  	.driver		= {
>  		.pm	= &i801_pm_ops,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
>  };
>  

Seems reasonable. I can't foresee any problem that would occur from
this change, and preliminary testing on my own workstation is OK.

Jarkko, Heiner, Wolfram, can you think of any reason why we should NOT
apply this change?

Thanks,
-- 
Jean Delvare
SUSE L3 Support
Re: [PATCH] i2c: i801: Prefer async probe
Posted by Jarkko Nikula 3 years, 6 months ago
On 9/29/22 18:43, Jean Delvare wrote:
>> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
>> index a176296f4fff..e06509edc5f3 100644
>> --- a/drivers/i2c/busses/i2c-i801.c
>> +++ b/drivers/i2c/busses/i2c-i801.c
>> @@ -1838,6 +1838,7 @@ static struct pci_driver i801_driver = {
>>   	.shutdown	= i801_shutdown,
>>   	.driver		= {
>>   		.pm	= &i801_pm_ops,
>> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>   	},
>>   };
>>   
> 
> Seems reasonable. I can't foresee any problem that would occur from
> this change, and preliminary testing on my own workstation is OK.
> 
> Jarkko, Heiner, Wolfram, can you think of any reason why we should NOT
> apply this change?
> 
I tested this on two systems. One with EE1004-compliant SPD EEPROM and 
another with touchpad connected to RMI4 SMB bus.

Average boot time improved ~20 ms over 5 boots on the first system and 
did not see any issue with the touchpad on second.

Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Re: [PATCH] i2c: i801: Prefer async probe
Posted by Wolfram Sang 3 years, 6 months ago
> Jarkko, Heiner, Wolfram, can you think of any reason why we should NOT
> apply this change?

Nope. Even if we overlooked something, we can still revert and figure
out what went wrong.

Re: [PATCH] i2c: i801: Prefer async probe
Posted by Mani Milani 3 years, 6 months ago
Hi Jean,

No, there is nothing special about the 190ms. It is just what I
measured on my device under test. I decided to include it in the
commit message to give a rough idea on how much delay this driver
probe can be adding to boot time.

Thank you Jean and Wolfram for reviewing.

P.S:   Apologies for sending this twice! My previous message was
rejected by the mailing list due to containing HTML subparts. (fingers
crossed this time)