Date: Wed, 12 May 2021 21:39:30 GMT From: Poul-Henning Kamp <phk@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 5ab41ff8e92d - main - More refactoring: Message-ID: <202105122139.14CLdU54093839@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by phk: URL: https://cgit.FreeBSD.org/src/commit/?id=5ab41ff8e92da548acf06b50384df538737aa8ee commit 5ab41ff8e92da548acf06b50384df538737aa8ee Author: Poul-Henning Kamp <phk@FreeBSD.org> AuthorDate: 2021-05-12 21:34:58 +0000 Commit: Poul-Henning Kamp <phk@FreeBSD.org> CommitDate: 2021-05-12 21:39:19 +0000 More refactoring: Extract the '-a' mode into a separate function, and simplify the hexdumping logic. Dont call usage() without telling why. --- usr.sbin/i2c/i2c.8 | 36 +++-------- usr.sbin/i2c/i2c.c | 181 +++++++++++++++++++++++++---------------------------- 2 files changed, 97 insertions(+), 120 deletions(-) diff --git a/usr.sbin/i2c/i2c.8 b/usr.sbin/i2c/i2c.8 index 1c5174e63f32..352b13157968 100644 --- a/usr.sbin/i2c/i2c.8 +++ b/usr.sbin/i2c/i2c.8 @@ -64,12 +64,12 @@ The options are as follows: 7-bit address on the I2C device to operate on (hex). .It Fl b binary mode - when performing a read operation, the data read from the device -is output in binary format on stdout; when doing a write, the binary data to -be written to the device is read from stdin. +is output in binary format on stdout. .It Fl c Ar count -number of bytes to transfer (dec). +number of bytes to transfer (decimal). .It Fl d Ar r|w transfer direction: r - read, w - write. +Data to be written is read from stdin as binary bytes. .It Fl f Ar device I2C bus to use (default is /dev/iic0). .It Fl m Ar tr|ss|rs|no @@ -113,7 +113,7 @@ scan the bus for devices. .It Fl v be verbose. .It Fl w Ar 0|8|16|16LE|16BE -device addressing width (in bits). +device offset width (in bits). This is used to determine how to pass .Ar offset specified with @@ -122,28 +122,6 @@ to the slave. Zero means that the offset is ignored and not passed to the slave at all. The endianess defaults to little-endian. .El -.Sh WARNINGS -Great care must be taken when manipulating slave I2C devices with the -.Nm -utility. -Often times important configuration data for the system is kept in non-volatile -but write enabled memories located on the I2C bus, for example Ethernet hardware -addresses, RAM module parameters (SPD), processor reset configuration word etc. -.Pp -It is very easy to render the whole system unusable when such configuration -data is deleted or altered, so use the -.Dq -d w -(write) command only if you know exactly what you are doing. -.Pp -Also avoid ungraceful interrupting of an ongoing transaction on the I2C bus, -as it can lead to potentially dangerous effects. -Consider the following scenario: when the host CPU is reset (for whatever reason) -in the middle of a started I2C transaction, the I2C slave device could be left -in write mode waiting for data or offset to arrive. -When the CPU reinitializes itself and talks to this I2C slave device again, -the commands and other control info it sends are treated by the slave device -as data or offset it was waiting for, and there's great potential for -corruption if such a write is performed. .Sh EXAMPLES .Bl -bullet .It @@ -177,9 +155,15 @@ Reset the controller: .Pp i2c -f /dev/iic1 -r .El +.Sh WARNING +Many systems store critical low-level information in I2C memories, and +may contain other I2C devices, such as temperature or voltage sensors. +Reading these can disturb the firmware's operation and writing to them +can "brick" the hardware. .Sh SEE ALSO .Xr iic 4 , .Xr iicbus 4 +.Xr smbus 4 .Sh HISTORY The .Nm diff --git a/usr.sbin/i2c/i2c.c b/usr.sbin/i2c/i2c.c index 64233366a0a8..c862666fc9a7 100644 --- a/usr.sbin/i2c/i2c.c +++ b/usr.sbin/i2c/i2c.c @@ -73,9 +73,11 @@ struct skip_range { }; __dead2 static void -usage(void) +usage(const char *msg) { + if (msg != NULL) + fprintf(stderr, "%s\n", msg); fprintf(stderr, "usage: %s -a addr [-f device] [-d [r|w]] [-o offset] " "[-w [0|8|16]] [-c count] [-m [tr|ss|rs|no]] [-b] [-v]\n", getprogname()); @@ -561,19 +563,71 @@ i2c_rdwr_transfer(int fd, struct options i2c_opt, char *i2c_buf) return (0); } +static int +access_bus(int fd, struct options i2c_opt) +{ + char *i2c_buf; + int error, chunk_size = 16, i, ch; + + i2c_buf = malloc(i2c_opt.count); + if (i2c_buf == NULL) + err(1, "data malloc"); + + /* + * For a write, read the data to be written to the chip from stdin. + */ + if (i2c_opt.dir == 'w') { + if (i2c_opt.verbose && !i2c_opt.binary) + fprintf(stderr, "Enter %d bytes of data: ", + i2c_opt.count); + for (i = 0; i < i2c_opt.count; i++) { + ch = getchar(); + if (ch == EOF) { + free(i2c_buf); + err(1, "not enough data, exiting\n"); + } + i2c_buf[i] = ch; + } + } + + if (i2c_opt.mode == I2C_MODE_TRANSFER) + error = i2c_rdwr_transfer(fd, i2c_opt, i2c_buf); + else if (i2c_opt.dir == 'w') + error = i2c_write(fd, i2c_opt, i2c_buf); + else + error = i2c_read(fd, i2c_opt, i2c_buf); + + if (error == 0) { + if (i2c_opt.dir == 'r' && i2c_opt.binary) { + (void)fwrite(i2c_buf, 1, i2c_opt.count, stdout); + } else if (i2c_opt.verbose || i2c_opt.dir == 'r') { + if (i2c_opt.verbose) + fprintf(stderr, "\nData %s (hex):\n", + i2c_opt.dir == 'r' ? "read" : "written"); + + for (i = 0; i < i2c_opt.count; i++) { + fprintf (stderr, "%02hhx ", i2c_buf[i]); + if ((i % chunk_size) == chunk_size - 1) + fprintf(stderr, "\n"); + } + if ((i % chunk_size) != 0) + fprintf(stderr, "\n"); + } + } + + free(i2c_buf); + return (error); +} + int main(int argc, char** argv) { struct options i2c_opt; - char *skip_addr = NULL, *i2c_buf; + char *skip_addr = NULL; const char *dev, *err_msg; - int fd, error, chunk_size, i, j, ch; + int fd, error, ch; errno = 0; - error = 0; - - /* Line-break the output every chunk_size bytes */ - chunk_size = 16; dev = I2C_DEV; @@ -595,9 +649,8 @@ main(int argc, char** argv) case 'a': i2c_opt.addr = (strtoul(optarg, 0, 16) << 1); if (i2c_opt.addr == 0 && errno == EINVAL) - i2c_opt.addr_set = 0; - else - i2c_opt.addr_set = 1; + usage("Bad -a argument (hex)"); + i2c_opt.addr_set = 1; break; case 'f': dev = optarg; @@ -608,13 +661,15 @@ main(int argc, char** argv) case 'o': i2c_opt.off = strtoul(optarg, 0, 16); if (i2c_opt.off == 0 && errno == EINVAL) - error = 1; + usage("Bad -o argument (hex)"); break; case 'w': i2c_opt.width = optarg; break; case 'c': - i2c_opt.count = atoi(optarg); + i2c_opt.count = (strtoul(optarg, 0, 10)); + if (i2c_opt.count == 0 && errno == EINVAL) + usage("Bad -c argument (decimal)"); break; case 'm': if (!strcmp(optarg, "no")) @@ -626,7 +681,7 @@ main(int argc, char** argv) else if (!strcmp(optarg, "tr")) i2c_opt.mode = I2C_MODE_TRANSFER; else - usage(); + usage("Bad -m argument ([no|ss|rs|tr])"); break; case 'n': i2c_opt.skip = 1; @@ -645,16 +700,16 @@ main(int argc, char** argv) i2c_opt.reset = 1; break; case 'h': + usage("Help:"); + break; default: - usage(); + usage("Bad argument"); } } argc -= optind; argv += optind; - if (argc > 0) { - fprintf(stderr, "Too many arguments\n"); - usage(); - } + if (argc > 0) + usage("Too many arguments"); /* Set default mode if option -m is not specified */ if (i2c_opt.mode == I2C_MODE_NOTSET) { @@ -664,24 +719,20 @@ main(int argc, char** argv) i2c_opt.mode = I2C_MODE_NONE; } - if (i2c_opt.addr_set) { - err_msg = encode_offset(i2c_opt.width, i2c_opt.off, - i2c_opt.off_buf, &i2c_opt.off_len); - if (err_msg != NULL) { - fprintf(stderr, "%s", err_msg); - exit(EX_USAGE); - } + err_msg = encode_offset(i2c_opt.width, i2c_opt.off, + i2c_opt.off_buf, &i2c_opt.off_len); + if (err_msg != NULL) { + fprintf(stderr, "%s", err_msg); + exit(EX_USAGE); } /* Basic sanity check of command line arguments */ if (i2c_opt.scan) { if (i2c_opt.addr_set) - usage(); + usage("-s and -a are incompatible"); } else if (i2c_opt.reset) { if (i2c_opt.addr_set) - usage(); - } else if (error) { - usage(); + usage("-r and -a are incompatible"); } if (i2c_opt.verbose) @@ -698,71 +749,13 @@ main(int argc, char** argv) } if (i2c_opt.scan) - exit(scan_bus(dev, fd, i2c_opt.skip, skip_addr)); - - if (i2c_opt.reset) - exit(reset_bus(dev, fd)); - - i2c_buf = malloc(i2c_opt.count); - if (i2c_buf == NULL) - err(1, "data malloc"); - - /* - * For a write, read the data to be written to the chip from stdin. - */ - if (i2c_opt.dir == 'w') { - if (i2c_opt.verbose && !i2c_opt.binary) - fprintf(stderr, "Enter %d bytes of data: ", - i2c_opt.count); - for (i = 0; i < i2c_opt.count; i++) { - ch = getchar(); - if (ch == EOF) { - free(i2c_buf); - err(1, "not enough data, exiting\n"); - } - i2c_buf[i] = ch; - } - } - - if (i2c_opt.mode == I2C_MODE_TRANSFER) - error = i2c_rdwr_transfer(fd, i2c_opt, i2c_buf); - else if (i2c_opt.dir == 'w') - error = i2c_write(fd, i2c_opt, i2c_buf); + error = scan_bus(dev, fd, i2c_opt.skip, skip_addr); + else if (i2c_opt.reset) + error = reset_bus(dev, fd); else - error = i2c_read(fd, i2c_opt, i2c_buf); - - if (error != 0) { - free(i2c_buf); - return (1); - } - - error = close(fd); - assert(error == 0); + error = access_bus(fd, i2c_opt); - if (i2c_opt.verbose) - fprintf(stderr, "\nData %s (hex):\n", i2c_opt.dir == 'r' ? - "read" : "written"); - - i = 0; - j = 0; - while (i < i2c_opt.count) { - if (i2c_opt.verbose || (i2c_opt.dir == 'r' && - !i2c_opt.binary)) - fprintf (stderr, "%02hhx ", i2c_buf[i++]); - - if (i2c_opt.dir == 'r' && i2c_opt.binary) { - fprintf(stdout, "%c", i2c_buf[j++]); - if(!i2c_opt.verbose) - i++; - } - if (!i2c_opt.verbose && (i2c_opt.dir == 'w')) - break; - if ((i % chunk_size) == 0) - fprintf(stderr, "\n"); - } - if ((i % chunk_size) != 0) - fprintf(stderr, "\n"); - - free(i2c_buf); - return (0); + ch = close(fd); + assert(ch == 0); + return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105122139.14CLdU54093839>