[Intel-gfx] [PATCH 1/3] intel_reg_read: add support for getopt

2012-02-29 Thread Eugeni Dodonov 
This will allow us to pass more options to it in the future.

v2: fix whitespacing issues and improve scary warning text as suggested by
Paul Menzel.

Reviewed-by: Paul Menzel paulepan...@users.sourceforge.net
Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
 tools/intel_reg_read.c |   44 +++-
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/tools/intel_reg_read.c b/tools/intel_reg_read.c
index cad30ff..95d28c5 100644
--- a/tools/intel_reg_read.c
+++ b/tools/intel_reg_read.c
@@ -41,21 +41,45 @@ static void dump_range(uint32_t start, uint32_t end)
   *(volatile uint32_t *)((volatile char*)mmio + i));
 }
 
+static void usage(char *cmdname)
+{
+   printf(Usage: %s [-f | addr]\n, cmdname);
+   printf(\t -f : read back full range of registers.\n);
+   printf(\t  WARNING! This option may result in a machine hang!\n);
+   printf(\t addr : in 0x format\n);
+}
+
 int main(int argc, char** argv)
 {
+   int ret = 0;
uint32_t reg;
+   int ch;
+   char *cmdname = strdup(argv[0]);
+   int full_dump = 0;
+
+   while ((ch = getopt(argc, argv, fh)) != -1) {
+   switch(ch) {
+   case 'f':
+   full_dump = 1;
+   break;
+   case 'h':
+   usage(cmdname);
+   ret = 1;
+   goto out;
+   }
+   }
+   argc -= optind;
+   argv += optind;
 
-   if (argc != 2) {
-   printf(Usage: %s [-f | addr]\n, argv[0]);
-   printf(\t -f : read back full range of registers.\n);
-   printf(\t  WARNING! This could be danger to hang the 
machine!\n);
-   printf(\t addr : in 0x format\n);
-   exit(1);
+   if (argc != 1) {
+   usage(cmdname);
+   ret = 1;
+   goto out;
}
 
intel_register_access_init(intel_get_pci_device(), 0);
 
-   if (!strcmp(argv[1], -f)) {
+   if (full_dump) {
dump_range(0x0, 0x00fff);   /* VGA registers */
dump_range(0x02000, 0x02fff);   /* instruction, memory, 
interrupt control registers */
dump_range(0x03000, 0x031ff);   /* FENCE and PPGTT control 
registers */
@@ -71,12 +95,14 @@ int main(int argc, char** argv)
dump_range(0x7, 0x72fff);   /* display and cursor registers 
*/
dump_range(0x73000, 0x73fff);   /* performance counters */
} else {
-   sscanf(argv[1], 0x%x, reg);
+   sscanf(argv[0], 0x%x, reg);
dump_range(reg, reg + 4);
}
 
intel_register_access_fini();
 
-   return 0;
+out:
+   free(cmdname);
+   return ret;
 }
 
-- 
1.7.9.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/3] intel_reg_read: add support for getopt

2012-02-28 Thread Eugeni Dodonov 
This will allow us to pass more options to it in the future.

Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
---
 tools/intel_reg_read.c |   44 +++-
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/tools/intel_reg_read.c b/tools/intel_reg_read.c
index cad30ff..c3e559b 100644
--- a/tools/intel_reg_read.c
+++ b/tools/intel_reg_read.c
@@ -41,21 +41,45 @@ static void dump_range(uint32_t start, uint32_t end)
   *(volatile uint32_t *)((volatile char*)mmio + i));
 }
 
+static void usage(char *cmdname)
+{
+   printf(Usage: %s [-f | addr]\n, cmdname);
+   printf(\t -f : read back full range of registers.\n);
+   printf(\t  WARNING! This could be danger to hang the machine!\n);
+   printf(\t addr : in 0x format\n);
+}
+
 int main(int argc, char** argv)
 {
+   int ret=0;
uint32_t reg;
+   int ch;
+   char *cmdname = strdup(argv[0]);
+   int full_dump=0;
+
+   while ((ch = getopt(argc, argv, fh)) != -1) {
+   switch(ch) {
+   case 'f':
+   full_dump=1;
+ break;
+   case 'h':
+ usage(cmdname);
+ ret=1;
+ goto out;
+   }
+   }
+   argc -= optind;
+   argv += optind;
 
-   if (argc != 2) {
-   printf(Usage: %s [-f | addr]\n, argv[0]);
-   printf(\t -f : read back full range of registers.\n);
-   printf(\t  WARNING! This could be danger to hang the 
machine!\n);
-   printf(\t addr : in 0x format\n);
-   exit(1);
+   if (argc != 1) {
+   usage(cmdname);
+   ret=1;
+   goto out;
}
 
intel_register_access_init(intel_get_pci_device(), 0);
 
-   if (!strcmp(argv[1], -f)) {
+   if (full_dump) {
dump_range(0x0, 0x00fff);   /* VGA registers */
dump_range(0x02000, 0x02fff);   /* instruction, memory, 
interrupt control registers */
dump_range(0x03000, 0x031ff);   /* FENCE and PPGTT control 
registers */
@@ -71,12 +95,14 @@ int main(int argc, char** argv)
dump_range(0x7, 0x72fff);   /* display and cursor registers 
*/
dump_range(0x73000, 0x73fff);   /* performance counters */
} else {
-   sscanf(argv[1], 0x%x, reg);
+   sscanf(argv[0], 0x%x, reg);
dump_range(reg, reg + 4);
}
 
intel_register_access_fini();
 
-   return 0;
+out:
+   free(cmdname);
+   return ret;
 }
 
-- 
1.7.9.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] intel_reg_read: add support for getopt

2012-02-28 Thread Paul Menzel 
Dear Eugeni,


Am Dienstag, den 28.02.2012, 17:55 -0300 schrieb Eugeni Dodonov:
 This will allow us to pass more options to it in the future.
 
 Signed-off-by: Eugeni Dodonov eugeni.dodo...@intel.com
 ---
  tools/intel_reg_read.c |   44 +++-
  1 file changed, 35 insertions(+), 9 deletions(-)
 
 diff --git a/tools/intel_reg_read.c b/tools/intel_reg_read.c
 index cad30ff..c3e559b 100644
 --- a/tools/intel_reg_read.c
 +++ b/tools/intel_reg_read.c
 @@ -41,21 +41,45 @@ static void dump_range(uint32_t start, uint32_t end)
  *(volatile uint32_t *)((volatile char*)mmio + i));
  }
  
 +static void usage(char *cmdname)
 +{
 + printf(Usage: %s [-f | addr]\n, cmdname);
 + printf(\t -f : read back full range of registers.\n);
 + printf(\t  WARNING! This could be danger to hang the machine!\n);

I know you just copied this sentence, but could a native English speaker
proof read it? It sounds strange to me.

 + printf(\t addr : in 0x format\n);
 +}
 +
  int main(int argc, char** argv)
  {
 + int ret=0;

Coding style seems to differ, does not it? There are some occurrences
where `=` does have spaces around. Care to send a patch cleaning that up
beforehand?

   uint32_t reg;
 + int ch;
 + char *cmdname = strdup(argv[0]);
 + int full_dump=0;
 +
 + while ((ch = getopt(argc, argv, fh)) != -1) {

I did not check further. But I guess no headers need to be included for
`getopt`.

 + switch(ch) {
 + case 'f':
 + full_dump=1;
 +   break;

Indentation seems to differ in the line `full_dump = 1`.

 + case 'h':
 +   usage(cmdname);
 +   ret=1;
 +   goto out;
 + }
 + }
 + argc -= optind;
 + argv += optind;
  
 - if (argc != 2) {
 - printf(Usage: %s [-f | addr]\n, argv[0]);
 - printf(\t -f : read back full range of registers.\n);
 - printf(\t  WARNING! This could be danger to hang the 
 machine!\n);
 - printf(\t addr : in 0x format\n);
 - exit(1);
 + if (argc != 1) {
 + usage(cmdname);
 + ret=1;
 + goto out;
   }
  
   intel_register_access_init(intel_get_pci_device(), 0);
  
 - if (!strcmp(argv[1], -f)) {
 + if (full_dump) {
   dump_range(0x0, 0x00fff);   /* VGA registers */
   dump_range(0x02000, 0x02fff);   /* instruction, memory, 
 interrupt control registers */
   dump_range(0x03000, 0x031ff);   /* FENCE and PPGTT control 
 registers */
 @@ -71,12 +95,14 @@ int main(int argc, char** argv)
   dump_range(0x7, 0x72fff);   /* display and cursor registers 
 */
   dump_range(0x73000, 0x73fff);   /* performance counters */
   } else {
 - sscanf(argv[1], 0x%x, reg);
 + sscanf(argv[0], 0x%x, reg);
   dump_range(reg, reg + 4);
   }
  
   intel_register_access_fini();
  
 - return 0;
 +out:
 + free(cmdname);
 + return ret;
  }

Otherwise you can add

Reviewed-by: Paul Menzel paulepan...@users.sourceforge.net


Thanks,

Paul


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx