Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Oct 2017 17:45:31 +0300
From:      Andriy Gapon <avg@FreeBSD.org>
To:        Ian Lepore <ian@FreeBSD.org>, src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org
Subject:   Re: svn commit: r323465 - head/usr.sbin/i2c
Message-ID:  <4c4a916f-9960-6d7f-3389-37b998ba980b@FreeBSD.org>
In-Reply-To: <201709112149.v8BLncAs049328@repo.freebsd.org>
References:  <201709112149.v8BLncAs049328@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 12/09/2017 00:49, Ian Lepore wrote:
> Author: ian
> Date: Mon Sep 11 21:49:38 2017
> New Revision: 323465
> URL: https://svnweb.freebsd.org/changeset/base/323465
> 
> Log:
>   Make i2c -s (device scan) work on hardware that supports only full xfers.
>   
>   The existing scan code is based on sending an i2c START condition and if
>   there is no error it assumes there is a device at that i2c address.  Some
>   i2c controllers don't support sending individual start/stop signals on the
>   bus, they can only perform complete data transfers with start/stop handled
>   in the silicon.
>   
>   This adds a fallback mechanism that attempts to read a single byte from each
>   i2c address.  It's less reliable than looking for an an ACK repsonse to a
>   start, because some devices will NAK an attempt to read that isn't preceeded
>   by a write of a register address.  Writing to devices to probe them is too
>   dangerous to even consider.  The user is told that a less-reliable scan is
>   being done, so even if the read-scan comes up empty too, it's still a vast
>   improvement over the old situation where it would just claim there were no
>   devices on the bus even though the devices were there and working fine.
>   
>   If the i2c controller responds with a proper ENODEV (device doesn't support
>   operation) or an almost-proper EOPNOTSUPP, the START/STOP scan is switched
>   to a read-scan right away.  Most controllers respond with ENXIO or EIO if
>   they don't support START/STOP, so no quick-out is available.  For those,
>   if a scan of all 127 addresses and come up empty, the scan is re-done using
>   the read method.


Perhaps the new scan method should have been added as a separate option that has
to be explicitly activated...  My concern is that there are some extremely
simple I2C devices out there that do no sanity checking and may get confused

>   Reported by:	Maxim Filimonov <che@bein.link>
> 
> Modified:
>   head/usr.sbin/i2c/i2c.c
> 
> Modified: head/usr.sbin/i2c/i2c.c
> ==============================================================================
> --- head/usr.sbin/i2c/i2c.c	Mon Sep 11 21:32:35 2017	(r323464)
> +++ head/usr.sbin/i2c/i2c.c	Mon Sep 11 21:49:38 2017	(r323465)
> @@ -121,9 +121,12 @@ skip_get_tokens(char *skip_addr, int *sk_addr, int max
>  static int
>  scan_bus(struct iiccmd cmd, char *dev, int skip, char *skip_addr)
>  {
> +	struct iic_msg rdmsg;
> +	struct iic_rdwr_data rdwrdata;
>  	struct skip_range addr_range = { 0, 0 };
>  	int *tokens, fd, error, i, index, j;
> -	int len = 0, do_skip = 0, no_range = 1;
> +	int len = 0, do_skip = 0, no_range = 1, num_found = 0, use_read_xfer = 0;
> +	uint8_t rdbyte;
>  
>  	fd = open(dev, O_RDWR);
>  	if (fd == -1) {
> @@ -157,6 +160,14 @@ scan_bus(struct iiccmd cmd, char *dev, int skip, char 
>  	}
>  
>  	printf("Scanning I2C devices on %s: ", dev);
> +
> +start_over:
> +	if (use_read_xfer) {
> +		fprintf(stderr, 
> +		    "Hardware may not support START/STOP scanning; "
> +		    "trying less-reliable read method.\n");
> +	}
> +
>  	for (i = 1; i < 127; i++) {
>  
>  		if (skip && ( addr_range.start < addr_range.end)) {
> @@ -180,17 +191,46 @@ scan_bus(struct iiccmd cmd, char *dev, int skip, char 
>  		cmd.last = 1;
>  		cmd.count = 0;
>  		error = ioctl(fd, I2CRSTCARD, &cmd);
> -		if (error)
> +		if (error) {
> +			fprintf(stderr, "Controller reset failed\n");
>  			goto out;
> -
> -		cmd.slave = i << 1;
> -		cmd.last = 1;
> -		error = ioctl(fd, I2CSTART, &cmd);
> -		if (!error)
> -			printf("%x ", i);
> -		cmd.slave = i << 1;
> -		cmd.last = 1;
> -		error = ioctl(fd, I2CSTOP, &cmd);
> +		}
> +		if (use_read_xfer) {
> +			rdmsg.buf = &rdbyte;
> +			rdmsg.len = 1;
> +			rdmsg.flags = IIC_M_RD;
> +			rdmsg.slave = i << 1;
> +			rdwrdata.msgs = &rdmsg;
> +			rdwrdata.nmsgs = 1;
> +			error = ioctl(fd, I2CRDWR, &rdwrdata);
> +		} else {
> +			cmd.slave = i << 1;
> +			cmd.last = 1;
> +			error = ioctl(fd, I2CSTART, &cmd);
> +			if (errno == ENODEV || errno == EOPNOTSUPP) {
> +				/* If START not supported try reading. */
> +				use_read_xfer = 1;
> +				goto start_over;
> +			}
> +			cmd.slave = i << 1;
> +			cmd.last = 1;
> +			ioctl(fd, I2CSTOP, &cmd);
> +		}
> +		if (error == 0) {
> +			++num_found;
> +			printf("%02x ", i);
> +		}
> +	}
> +	/*
> +	 * If we found nothing, maybe START is not supported and returns a
> +	 * generic error code such as EIO or ENXIO, so try again using reads.
> +	 */
> +	if (num_found == 0) {
> +		if (!use_read_xfer) {
> +			use_read_xfer = 1;
> +			goto start_over;
> +		}
> +		printf("<none found>");
>  	}
>  	printf("\n");
>  
> 


-- 
Andriy Gapon



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4c4a916f-9960-6d7f-3389-37b998ba980b>