linux-kernel - Re: [PATCH 5/8 v2] i2c:i2c-bfin-twi: TWI fails to restart next transfer in high system load.

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] [day] [month] [year] [list]

Date:	Fri, 13 Jul 2012 08:38:38 +0200
From:	Wolfram Sang <w.sang@...gutronix.de>
To:	Sonic Zhang <sonic.adi@...il.com>
Cc:	Ben Dooks <ben-linux@...ff.org>, linux-i2c@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	uclinux-dist-devel@...ckfin.uclinux.org,
	Sonic Zhang <sonic.zhang@...log.com>
Subject: Re: [PATCH 5/8 v2] i2c:i2c-bfin-twi: TWI fails to restart next
 transfer in high system load.

On Wed, Jun 13, 2012 at 04:22:44PM +0800, Sonic Zhang wrote:
> From: Sonic Zhang <sonic.zhang@...log.com>
> 
> Current driver was developed based on BF537 0.2 HRM. In high system load, BUFRDERR error
> interrupt may be raised if XMTSERV interrupt of last TX byte is not served in time
> (set RSTART bit), which breaks restart tranfer as expected.
> 
> "Buffer Read Error (BUFRDERR)" description in Blackfin HRM only applys to BF537
> rev. < 0.3. In later rev. and later announced Blackfin chips, such as BF527 and
> BF548, a new TWI master feature "Clock Stretching" is added into the TWI controller,
> BUFRDERR interrupt is not triggered after TX FIFO is empty.
> 
> This patch sets RSTART bit at the beginning of the first transfer. The SCL and SDA
> is hold till XMTSERV interrupt of last TX byte is served. Restart transfer is not broken
> in high system load.
> 
> Signed-off-by: Sonic Zhang <sonic.zhang@...log.com>
> ---
>  drivers/i2c/busses/i2c-bfin-twi.c |   23 +++++++++++++----------
>  1 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
> index 2e59bbd..e75ee91 100644
> --- a/drivers/i2c/busses/i2c-bfin-twi.c
> +++ b/drivers/i2c/busses/i2c-bfin-twi.c
> @@ -99,7 +99,7 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
>  		 */
>  		else if (iface->cur_mode == TWI_I2C_MODE_COMBINED)
>  			write_MASTER_CTL(iface,
> -				read_MASTER_CTL(iface) | MDIR | RSTART);
> +				read_MASTER_CTL(iface) | MDIR);
>  		else if (iface->manual_stop)
>  			write_MASTER_CTL(iface,
>  				read_MASTER_CTL(iface) | STOP);
> @@ -107,10 +107,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
>  		         iface->cur_msg + 1 < iface->msg_num) {
>  			if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
>  				write_MASTER_CTL(iface,
> -					read_MASTER_CTL(iface) | RSTART | MDIR);
> +					read_MASTER_CTL(iface) | MDIR);
>  			else
>  				write_MASTER_CTL(iface,
> -					(read_MASTER_CTL(iface) | RSTART) & ~MDIR);
> +					read_MASTER_CTL(iface) & ~MDIR);
>  		}
>  	}
>  	if (twi_int_status & RCVSERV) {
> @@ -144,10 +144,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
>  					iface->cur_msg + 1 < iface->msg_num) {
>  				if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
>  					write_MASTER_CTL(iface,
> -						read_MASTER_CTL(iface) | RSTART | MDIR);
> +						read_MASTER_CTL(iface) | MDIR);
>  				else
>  					write_MASTER_CTL(iface,
> -						(read_MASTER_CTL(iface) | RSTART) & ~MDIR);
> +						read_MASTER_CTL(iface) & ~MDIR);
>  			}
>  		}
>  	}
> @@ -262,9 +262,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
>  					(0xff << 6)));
>  				iface->manual_stop = 1;
>  			}
> -			/* remove restart bit and enable master receive */
> -			write_MASTER_CTL(iface,
> -				read_MASTER_CTL(iface) & ~RSTART);
> +			/* remove restart bit before last message */
> +			if (iface->cur_msg+1 == iface->msg_num)

Spaces around operators. I fixed it for you.

> +				write_MASTER_CTL(iface,
> +					read_MASTER_CTL(iface) & ~RSTART);
>  		} else {
>  			iface->result = 1;
>  			write_INT_MASK(iface, 0);
> @@ -321,7 +322,8 @@ static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,
>  		return -EINVAL;
>  	}
>  
> -	iface->cur_mode = TWI_I2C_MODE_REPEAT;
> +	if (iface->msg_num > 1)
> +		iface->cur_mode = TWI_I2C_MODE_REPEAT;
>  	iface->manual_stop = 0;
>  	iface->transPtr = pmsg->buf;
>  	iface->writeNum = iface->readNum = pmsg->len;
> @@ -366,6 +368,7 @@ static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,
>  
>  	/* Master enable */
>  	write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN |
> +		(iface->msg_num > 1 ? RSTART : 0) |
>  		((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
>  		((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 100) ? FAST : 0));
>  	SSYNC();
> @@ -530,7 +533,7 @@ int bfin_twi_do_smbus_xfer(struct i2c_adapter *adap, u16 addr,
>  		else
>  			write_MASTER_CTL(iface, 0x1 << 6);
>  		/* Master enable */
> -		write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN |
> +		write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN | RSTART |
>  			((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
>  		break;
>  	default:
> -- 
> 1.7.0.4
> 
> 

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)