From owner-svn-src-head@freebsd.org Tue Oct 10 14:46:31 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 65020E33E08; Tue, 10 Oct 2017 14:46:31 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citapm.icyb.net.ua (citapm.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 280F284E84; Tue, 10 Oct 2017 14:46:29 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citapm.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id RAA02552; Tue, 10 Oct 2017 17:46:27 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1e1vnf-0001ey-1y; Tue, 10 Oct 2017 17:46:27 +0300 From: Andriy Gapon Subject: Re: svn commit: r323465 - head/usr.sbin/i2c To: Ian Lepore , src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org References: <201709112149.v8BLncAs049328@repo.freebsd.org> Message-ID: <4c4a916f-9960-6d7f-3389-37b998ba980b@FreeBSD.org> Date: Tue, 10 Oct 2017 17:45:31 +0300 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <201709112149.v8BLncAs049328@repo.freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Oct 2017 14:46:31 -0000 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 > > 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(""); > } > printf("\n"); > > -- Andriy Gapon