linux-kernel - Re: [PATCH 2/2] i2c: piix4: Use usleep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6af53537-f160-8ea5-2491-98f8a7966975@opengear.com>
Date:   Thu, 22 Feb 2018 13:59:49 +1000
From:   Andrew Cooks <andrew.cooks@...ngear.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        Wolfram Sang <wsa@...-dreams.de>
Cc:     Jean Delvare <jdelvare@...e.com>,
        Zoltán Böszörményi 
        <zboszor@...hu>, linux-i2c@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] i2c: piix4: Use usleep_range()


On 31/12/17 02:50, Guenter Roeck wrote:
> The piix4 i2c driver is extremely slow. Replacing msleep()
> with usleep_range() increases its speed substantially.
> 
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
>  drivers/i2c/busses/i2c-piix4.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 78dd5951d6e7..52a8b1c5c110 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -467,13 +467,13 @@ static int piix4_transaction(struct i2c_adapter *piix4_adapter)
>  
>  	/* We will always wait for a fraction of a second! (See PIIX4 docs errata) */
>  	if (srvrworks_csb5_delay) /* Extra delay for SERVERWORKS_CSB5 */
> -		msleep(2);
> +		usleep_range(2000, 2000);
>  	else
> -		msleep(1);
> +		usleep_range(500, 1000);
>  
>  	while ((++timeout < MAX_TIMEOUT) &&
>  	       ((temp = inb_p(SMBHSTSTS)) & 0x01))
> -		msleep(1);
> +		usleep_range(200, 500);
>  
>  	/* If the SMBus is still busy, we give up */
>  	if (timeout == MAX_TIMEOUT) {
> 
Thanks for this patch.

FWIW, this also makes a noticeable difference on AMD Family 16h Model 30h, used in embedded designs and also commonly available as AMD GX-412TC SoC in the PC Engines APU2.

Among the tests I did were reading from the SoC temperature sensor in a loop:
while [ true ] ; do i2cget -y 0 0x4C 0x01 ; done

and scanning for peripherals in a loop:
while [ true ] ; do i2cdetect -y 0 ; done

These tests may be artificial and trivial, but the speedup matters to us because we have more than one bus master and the embedded controller needs to poll multiple sensors.

Tested-by: Andrew Cooks <andrew.cooks@...ngear.com>