The branch main has been updated by phk:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=5ab41ff8e92da548acf06b50384df538737aa8ee

commit 5ab41ff8e92da548acf06b50384df538737aa8ee
Author:     Poul-Henning Kamp <p...@freebsd.org>
AuthorDate: 2021-05-12 21:34:58 +0000
Commit:     Poul-Henning Kamp <p...@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);
 }
_______________________________________________
dev-commits-src-main@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
To unsubscribe, send any mail to "dev-commits-src-main-unsubscr...@freebsd.org"

Reply via email to