From owner-svn-src-all@freebsd.org Tue Oct 10 16:23:41 2017 Return-Path: Delivered-To: svn-src-all@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 9A264E368DF; Tue, 10 Oct 2017 16:23:41 +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 378AC3BE7; Tue, 10 Oct 2017 16:23:38 +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 TAA02734; Tue, 10 Oct 2017 19:23:37 +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 1e1xJh-0001im-JJ; Tue, 10 Oct 2017 19:23:37 +0300 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> <4c4a916f-9960-6d7f-3389-37b998ba980b@FreeBSD.org> <1507651963.84167.37.camel@freebsd.org> From: Andriy Gapon Message-ID: <230cbbbf-9d98-2249-e0db-488b4a3abfb3@FreeBSD.org> Date: Tue, 10 Oct 2017 19:23:02 +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: <1507651963.84167.37.camel@freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Oct 2017 16:23:41 -0000 On 10/10/2017 19:12, Ian Lepore wrote: > On Tue, 2017-10-10 at 17:45 +0300, Andriy Gapon wrote: >> 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 >> > > i2c -s is not a thing that's done routinely in a production system or > normal system operations... it's something a person does manually when > trying to configure or debug a system. Right. And another point, it's not extremely hard to type another option on the command line. I haven't suggested that the functionality should not have been added at all, I have just suggested adding a new option to unlock it. > In that situation, there is > more harm in being told there are no working devices on the bus when in > fact everything is fine, than there is some some hypothetical device > doing some hypothetical "bad thing" in response to a read command.  In > all my years of working with i2c stuff I've never seen a device doing > anything more harmful than hanging the bus, requiring a reset (and even > causing that requires worse behavior than an unexpected read).  On the > other hand, I've seen a lot of people frustrated that i2c -s on freebsd > says there are no devices, while the equivelent command on linux shows > that everything is fine. -- Andriy Gapon