linux-kernel - Re: [PATCH v1 08/12] i2c: isch: Use read_poll

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: <pwvhzkxeniutopyxczvimkau3elchfy5x32cimlqwjnmqjzv42@zpojd2lxs3o4>
Date: Thu, 12 Sep 2024 09:29:38 +0200
From: Andi Shyti <andi.shyti@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Jean Delvare <jdelvare@...e.com>
Subject: Re: [PATCH v1 08/12] i2c: isch: Use read_poll_timeout()

Hi Andy,

...

> @@ -83,7 +80,6 @@ static int sch_transaction(struct i2c_adapter *adap)
>  	struct sch_i2c *priv = container_of(adap, struct sch_i2c, adapter);
>  	int temp;
>  	int result = 0;
> -	int retries = 0;
>  
>  	dev_dbg(&adap->dev,
>  		"Transaction (pre): CNT=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
> @@ -112,15 +108,11 @@ static int sch_transaction(struct i2c_adapter *adap)
>  	temp |= 0x10;
>  	sch_io_wr8(priv, SMBHSTCNT, temp);
>  
> -	do {
> -		usleep_range(100, 200);
> -		temp = sch_io_rd8(priv, SMBHSTSTS) & 0x0f;
> -	} while ((temp & 0x08) && (retries++ < MAX_RETRIES));
> -
> +	result = read_poll_timeout(sch_io_rd8, temp, !(temp & 0x08), 200, 500000, true,
> +				   priv, SMBHSTSTS);
>  	/* If the SMBus is still busy, we give up */
> -	if (retries > MAX_RETRIES) {
> +	if (result) {
>  		dev_err(&adap->dev, "SMBus Timeout!\n");
> -		result = -ETIMEDOUT;
>  	} else if (temp & 0x04) {
>  		result = -EIO;
>  		dev_dbg(&adap->dev, "Bus collision! SMBus may be locked until next hard reset. (sorry!)\n");
> @@ -130,7 +122,7 @@ static int sch_transaction(struct i2c_adapter *adap)
>  		dev_err(&adap->dev, "Error: no response!\n");
>  	} else if (temp & 0x01) {
>  		dev_dbg(&adap->dev, "Post complete!\n");
> -		sch_io_wr8(priv, SMBHSTSTS, temp);
> +		sch_io_wr8(priv, SMBHSTSTS, temp & 0x0f);

there is still a dev_dbg() using temp. To be on the safe side, do
we want to do a "temp &= 0x0f" after the read_poll_timeout?

Andi

>  		temp = sch_io_rd8(priv, SMBHSTSTS) & 0x07;
>  		if (temp & 0x06) {
>  			/* Completion clear failed */
> -- 
> 2.43.0.rc1.1336.g36b5255a03ac
>