[igt-dev] [PATCH i-g-t 3/6] tools/dpcd_reg: Unify file handling.
Rodrigo Vivi
rodrigo.vivi at intel.com
Wed Sep 5 18:17:59 UTC 2018
On Tue, Sep 04, 2018 at 05:56:01PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-08-31 at 21:18 -0700, Rodrigo Vivi wrote:
> > Besides removing few code duplication this will
> > allow us to read multiple regs without having
> > to keep closing and opening the file for every
> > read.
> >
> > Cc: Tarun Vyas <tarun.vyas at intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> > tools/dpcd_reg.c | 98 +++++++++++++++++++++++++++-------------------
> > --
> > 1 file changed, 55 insertions(+), 43 deletions(-)
> >
> > diff --git a/tools/dpcd_reg.c b/tools/dpcd_reg.c
> > index d139007c..09da4109 100644
> > --- a/tools/dpcd_reg.c
> > +++ b/tools/dpcd_reg.c
> > @@ -53,61 +53,56 @@ static void print_usage(char *tool, int help)
> > exit((help == 1)? EXIT_SUCCESS : EXIT_FAILURE);
> > }
> >
> > -static int dpcd_read(char *device, const uint32_t offset, size_t
> > count)
> > +static int dpcd_read(int fd, const uint32_t offset, size_t count)
> > {
> > - int fd, ret, i;
> > + int ret, i;
> > void *buf = malloc(sizeof(uint8_t) * count);
> >
> > + ret = lseek(fd, offset, SEEK_SET);
> Does this offer any benefit over pread()/pwrite()?
I'm honestly not sure. Can we trust pread/pwrite that offsets are always
relative to the begin of the file or relative to where the pointer
was left after last read/write operation but file not closed?
looking to the docs lseek seemed more crispy on the definition
of its navigation and since I cannot guarantee order on dpcd_list
I preferred lseek.
but if you are sure pread/pwrite is reliable and not relative position
and you have strong preference on that I can change back.
Thanks,
Rodrigo.
>
> > + if (ret < 0) {
> > + igt_warn("lseek to %04x failed!\n", offset);
> > + return ret;
> > + }
> > +
> > if (NULL != buf)
> > memset(buf, 0, sizeof(uint8_t) * count);
> > else {
> > igt_warn("Can't allocate read buffer\n");
> > - return EXIT_FAILURE;
> > + return -1;
> > }
> >
> > - fd = open(device, O_RDONLY);
> > - if (fd > 0) {
> > - ret = pread(fd, buf, count, offset);
> > - close(fd);
> > - if (ret != count) {
> > - igt_warn("Failed to read from %s aux device
> > - error %s\n", device, strerror(errno));
> > - ret = EXIT_FAILURE;
> > - goto out;
> > - }
> > -
> > - igt_info("%04x:", offset);
> > - for (i = 0; i < count; i++)
> > - igt_info(" %02x", *(((uint8_t *)(buf)) +
> > i));
> > - igt_info("\n");
> > - }
> > - else {
> > - igt_warn("Failed to open %s aux device - error:
> > %s\n", device, strerror(errno));
> > - ret = EXIT_FAILURE;
> > + ret = read(fd, buf, count);
> > + if (ret != count) {
> > + igt_warn("Failed to read - error %s\n",
> > strerror(errno));
> > + goto out;
> > }
> > +
> > + igt_info("%04x:", offset);
> > + for (i = 0; i < count; i++)
> > + igt_info(" %02x", *(((uint8_t *)(buf)) + i));
> > + igt_info("\n");
> > +
> > out:
> > free(buf);
> > return ret;
> > }
> >
> > -static int dpcd_write(char *device, const uint32_t offset, const
> > uint8_t *val)
> > +static int dpcd_write(int fd, const uint32_t offset, const uint8_t
> > *val)
> > {
> > - int fd, ret;
> > -
> > - fd = open(device, O_RDWR);
> > - if (fd > 0) {
> > - ret = pwrite(fd, (const void *)val, sizeof(uint8_t),
> > offset);
> > - close(fd);
> > - if (ret < 0) {
> > - igt_warn("Failed to write to %s aux device -
> > error %s\n", device, strerror(errno));
> > - ret = EXIT_FAILURE;
> > - goto out;
> > - }
> > - ret = dpcd_read(device, offset, sizeof(uint8_t));
> > + int ret;
> > +
> > + ret = lseek(fd, offset, SEEK_SET);
> > + if (ret < 0) {
> > + igt_warn("lseek to %04x failed!\n", offset);
> > + return ret;
> > }
> > - else {
> > - igt_warn("Failed to open %s aux device - error:
> > %s\n", device, strerror(errno));
> > - ret = EXIT_FAILURE;
> > +
> > + ret = write(fd, (const void *)val, sizeof(uint8_t));
> > + if (ret < 0) {
> > + igt_warn("Failed to write - error %s\n",
> > strerror(errno));
> > + goto out;
> > }
> > + ret = dpcd_read(fd, offset, sizeof(uint8_t));
> > out:
> > return ret;
> > }
> > @@ -115,10 +110,11 @@ out:
> > int main(int argc, char **argv)
> > {
> > char dev_name[20];
> > - int ret, devid, help_flg;
> > + int fd, ret, devid, help_flg;
> > uint32_t offset;
> > uint8_t val;
> > size_t count;
> > + int file_op = O_RDONLY;
> >
> > enum command {
> > INV = -1,
> > @@ -157,11 +153,12 @@ int main(int argc, char **argv)
> > break;
> > /* Command parsing */
> > case 1:
> > - if (strcmp(optarg, "read") == 0)
> > + if (strcmp(optarg, "read") == 0) {
> > cmd = READ;
> > - else if (strcmp(optarg, "write") == 0)
> > + } else if (strcmp(optarg, "write") == 0) {
> > cmd = WRITE;
> > - break;
> > + file_op = O_RDWR;
> > + } break;
> > case ':':
> > igt_warn("The -%c option of %s requires an
> > argument\n",
> > optopt, argv[0]);
> > @@ -185,20 +182,33 @@ int main(int argc, char **argv)
> > memset(dev_name, '\0', 20);
> > snprintf(dev_name, sizeof(aux_dev) + 2, "%s%d", aux_dev,
> > devid);
> >
> > + fd = open(dev_name, file_op);
> > + if (fd < 0) {
> > + igt_warn("Failed to open %s aux device - error:
> > %s\n", dev_name,
> > + strerror(errno));
> > + return EXIT_FAILURE;
> > + }
> > +
> > switch (cmd) {
> > case READ:
> > if (count == INVALID) {
> > igt_warn("Please specify the count in
> > bytes\n");
> > print_usage(argv[0], 0);
> > }
> > - ret = dpcd_read(dev_name, offset, count);
> > + ret = dpcd_read(fd, offset, count);
> > + if (ret != count)
> > + igt_warn("Failed to read from %s aux
> > device\n",
> > + dev_name);
> > break;
> > case WRITE:
> > if (val == INVALID) {
> > igt_warn("Write value is missing\n");
> > print_usage(argv[0], 0);
> > }
> > - ret = dpcd_write(dev_name, offset, &val);
> > + ret = dpcd_write(fd, offset, &val);
> > + if (ret < 0)
> > + igt_warn("Failed to write to %s aux
> > device\n",
> > + dev_name);
> > break;
> > case INV:
> > default:
> > @@ -206,5 +216,7 @@ int main(int argc, char **argv)
> > print_usage(argv[0], 0);
> > }
> >
> > + close(fd);
> > +
> > return ret;
> > }
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list