Re: patch for fwcontrol

Sean Bruno <[email protected]>
Newsgroups gmane.os.freebsd.devel.firewire
Message-ID <[email protected]> 
Dieter wrote:
> In message <[email protected]>, Sean Bruno writes:
>
>   
>> Dieter wrote:
>>     
>>> The patch below:
>>>
>>> 	Fixes several err() vs errx() errors
>>> 	Improves some range checks
>>> 	Improves some messages
>>> 	Supports NetBSD in addition to FreeBSD
>>>   
>>>       
>> Ok, slightly reworked patch.  Seems to work fine for me.  Comments?
>>     
>
> Sean,
>
> 	Sorry for the delay, my normal outgoing email is hosed at
> the moment due to some problems upstream, so I'm trying workarounds
> with varying success.  Incoming email is fine.
>
> 	Your version of the patch looks fine, although leaving out
> the ".0" of the device name will keep it from working on NetBSD.
>
> 	What do you think of something like
>
> #if defined(__FreeBSD__)
> 	snprintf(devbase, sizeof(devbase), "%s%d", device_string, current_board);
> #elif defined(__NetBSD__)
> 	snprintf(devbase, sizeof(devbase), "%s%d.0", device_string, current_board);
> #else
> #warning "You need to add support for your OS"
> #endif
>
>
> This make the code a bit messier, and FreeBSD works fine with the extra ".0"
> which is why I just added the ".0".  I still don't know what the ".0" was/is for.
>
> So I see three options:
>
> 	1) Check your patch in as is.  NetBSDers will have to modify something.
>
> 	2) Add the ".0".  Obviously you don't like this option.
>
> 	3) Put in the #if stuff as above.
>
> I'd prefer #2 or #3, but if you prefer #1 that's ok.
>
> thanks,
> Dieter
>   
Dieter:

It's been a while, but I've finally found some time to revisit this 
code.  I've looked over the firewire driver
in FreeBSD and see no reason to use /dev/fw0.0 as the device as /dev/fw0 
is just an alias for it.

So, with a little more verbose error reporting with regards to 
permissions(non root opens) here is an updated
patch.


-- 
Sean Bruno
MiraLink Corporation
6015 NE 80th Ave, Ste 100
Portland, OR 97218
Phone 503-621-5143
Fax 503-621-5199
MSN: [email protected]
Google:  [email protected]
Yahoo:  [email protected]

_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-firewire
To unsubscribe, send any mail to "[email protected]"
fwcontrol.diff (text/plain, 7.5 KB) 
Index: fwmpegts.c
===================================================================
--- fwmpegts.c	(revision 185940)
+++ fwmpegts.c	(working copy)
@@ -52,9 +52,17 @@
 #include <string.h>
 #include <sysexits.h>
 
+#if defined(__FreeBSD__)
 #include <dev/firewire/firewire.h>
 #include <dev/firewire/iec68113.h>
+#elif defined(__NetBSD__)
+#include <dev/ieee1394/firewire.h>
+#include <dev/ieee1394/iec68113.h>
+#else
+#warning "You need to add support for your OS"
+#endif
 
+
 #include "fwmethods.h"
 
 #define	DEBUG 0
Index: fwcontrol.c
===================================================================
--- fwcontrol.c	(revision 185940)
+++ fwcontrol.c	(working copy)
@@ -32,8 +32,10 @@
  * SUCH DAMAGE.
  */
 
+#if defined(__FreeBSD__)
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
+#endif
 
 #include <sys/param.h>
 #include <sys/malloc.h>
@@ -42,12 +44,23 @@
 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <sys/errno.h>
+#if defined(__FreeBSD__)
 #include <sys/eui64.h>
 #include <dev/firewire/firewire.h>
 #include <dev/firewire/iec13213.h>
 #include <dev/firewire/fwphyreg.h>
 #include <dev/firewire/iec68113.h>
+#elif defined(__NetBSD__)
+#include "eui64.h"
+#include <dev/ieee1394/firewire.h>
+#include <dev/ieee1394/iec13213.h>
+#include <dev/ieee1394/fwphyreg.h>
+#include <dev/ieee1394/iec68113.h>
+#else
+#warning "You need to add support for your OS"
+#endif
 
+
 #include <netinet/in.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -66,7 +79,7 @@
 usage(void)
 {
 	fprintf(stderr,
-		"fwcontrol [-u bus_num] [-prt] [-c node] [-d node] [-o node] [-s node]\n"
+		"%s [-u bus_num] [-prt] [-c node] [-d node] [-o node] [-s node]\n"
 		"\t  [-l file] [-g gap_count] [-f force_root ] [-b pri_req]\n"
 		"\t  [-M mode] [-R filename] [-S filename] [-m EUI64 | hostname]\n"
 		"\t-u: specify bus number\n"
@@ -78,14 +91,15 @@
 		"\t-o: send link-on packet to the node\n"
 		"\t-s: write RESET_START register on the node\n"
 		"\t-l: load and parse hex dump file of configuration ROM\n"
-		"\t-g: broadcast gap_count by phy_config packet\n"
-		"\t-f: broadcast force_root by phy_config packet\n"
+		"\t-g: set gap count\n"
+		"\t-f: force root node\n"
 		"\t-b: set PRIORITY_BUDGET register on all supported nodes\n"
 		"\t-M: specify dv or mpeg\n"
 		"\t-R: Receive DV or MPEG TS stream\n"
 		"\t-S: Send DV stream\n"
-		"\t-m: set fwmem target\n");
-	exit(EX_USAGE);
+		"\t-m: set fwmem target\n"
+		, getprogname() );
+	fprintf(stderr, "\n");
 }
 
 static void
@@ -488,7 +502,7 @@
 	int len=1024, i;
 
 	if ((file = fopen(filename, "r")) == NULL)
-		err(1, "load_crom");
+	  err(1, "load_crom %s", filename);
 	for (i = 0; i < len/(4*8); i ++) {
 		fscanf(file, DUMP_FORMAT,
 			p, p+1, p+2, p+3, p+4, p+5, p+6, p+7);
@@ -691,7 +705,7 @@
 	 */
 	len = read(fd, buf, RECV_NUM_PACKET * RECV_PACKET_SZ);
 	if (len < 0)
-		err(EX_IOERR, "%s: error reading from device\n", __func__);
+		err(EX_IOERR, "%s: error reading from device", __func__);
 	ptr = (u_int32_t *) buf;
 	ciph = (struct ciphdr *)(ptr + 1);
 
@@ -757,10 +771,12 @@
 
 	if (argc < 2) {
 		for (current_board = 0; current_board < MAX_BOARDS; current_board++) {
-			snprintf(devbase, sizeof(devbase), "%s%d", device_string, current_board);
+			snprintf(devbase, sizeof(devbase), "%s%d.0", device_string, current_board);
 			if (open_dev(&fd, devbase) < 0) {
 				if (current_board == 0) {
 					usage();
+		  			err(EX_IOERR, "%s: Error opening firewire controller #%d %s",
+						      __func__, current_board, devbase);
 				}
 				return(EIO);
 			}
@@ -777,7 +793,7 @@
 		case 'b':
 			priority_budget = strtol(optarg, NULL, 0);
 			if (priority_budget < 0 || priority_budget > INT32_MAX)
-				errx(EX_USAGE, "%s: invalid number: %s", __func__, optarg);
+				errx(EX_USAGE, "%s: priority_budget out of range: %s", __func__, optarg);
 			command_set = true;
 			open_needed = true;
 			display_board_only = false;
@@ -787,7 +803,7 @@
 			if (crom_string == NULL)
 				err(EX_SOFTWARE, "%s:crom_string malloc", __func__);
 			if ( (strtol(crom_string, NULL, 0) < 0) || strtol(crom_string, NULL, 0) > MAX_BOARDS)
-				err(EX_USAGE, "%s:Invalid value for node", __func__);
+				errx(EX_USAGE, "%s:Invalid value for node", __func__);
 			strcpy(crom_string, optarg);
 			display_crom = 1;
 			open_needed = true;
@@ -808,7 +824,7 @@
 #define MAX_PHY_CONFIG 0x3f
 			set_root_node = strtol(optarg, NULL, 0);
 			if ( (set_root_node < 0) || (set_root_node > MAX_PHY_CONFIG) )
-				err(EX_USAGE, "%s:set_root_node out of range", __func__);
+				errx(EX_USAGE, "%s:set_root_node out of range", __func__);
 			open_needed = true;
 			command_set = true;
 			display_board_only = false;
@@ -816,7 +832,7 @@
 		case 'g':
 			set_gap_count = strtol(optarg, NULL, 0);
 			if ( (set_gap_count < 0) || (set_gap_count > MAX_PHY_CONFIG) )
-				err(EX_USAGE, "%s:set_gap_count out of range", __func__);
+				errx(EX_USAGE, "%s:set_gap_count out of range", __func__);
 			open_needed = true;
 			command_set = true;
 			display_board_only = false;
@@ -834,12 +850,12 @@
 			display_board_only = false;
 			if (eui64_hostton(optarg, &target) != 0 &&
 			    eui64_aton(optarg, &target) != 0)
-				err(EX_USAGE, "%s: invalid target: %s", __func__, optarg);
+				errx(EX_USAGE, "%s: invalid target: %s", __func__, optarg);
 			break;
 		case 'o':
 			send_link_on = str2node(fd, optarg);
-			if ( (send_link_on < 0) || (send_link_on > INT32_MAX) )
-				err(EX_USAGE, "%s: node out of range: %s\n",__func__, optarg);
+			if ( (send_link_on < 0) || (send_link_on > MAX_PHY_CONFIG) )
+				errx(EX_USAGE, "%s: node out of range: %s\n",__func__, optarg);
 			open_needed = true;
 			command_set = true;
 			display_board_only = false;
@@ -858,8 +874,8 @@
 			break;
 		case 's':
 			send_reset_start  = str2node(fd, optarg);
-			if ( (send_reset_start < 0) || (send_reset_start > INT32_MAX) )
-				err(EX_USAGE, "%s: node out of range: %s\n", __func__, optarg);
+			if ( (send_reset_start < 0) || (send_reset_start > MAX_PHY_CONFIG) )
+				errx(EX_USAGE, "%s: node out of range: %s\n", __func__, optarg);
 			open_needed = true;
 			command_set = true;
 			display_board_only = false;
@@ -924,9 +940,9 @@
 	*
 	*/
 	if(open_needed){
-		snprintf(devbase, sizeof(devbase), "%s%d", device_string, current_board);
+		snprintf(devbase, sizeof(devbase), "%s%d.0", device_string, current_board);
 		if (open_dev(&fd, devbase) < 0) {
-			errx(EX_IOERR, "%s: Error opening board #%d\n", __func__, current_board);
+		  err(EX_IOERR, "%s: Error opening firewire controller #%d %s", __func__, current_board, devbase);
 		}
 	}
 	/*
@@ -996,8 +1012,16 @@
 	if (set_fwmem_target) {
 		eui.hi = ntohl(*(u_int32_t*)&(target.octet[0]));
 		eui.lo = ntohl(*(u_int32_t*)&(target.octet[4]));
+#if defined(__FreeBSD__)
 		sysctl_set_int("hw.firewire.fwmem.eui64_hi", eui.hi);
 		sysctl_set_int("hw.firewire.fwmem.eui64_lo", eui.lo);
+#elif defined(__NetBSD__)
+		sysctl_set_int("hw.fwmem.eui64_hi", eui.hi);
+		sysctl_set_int("hw.fwmem.eui64_lo", eui.lo);
+#else
+#warning "You need to add support for your OS"
+#endif
+
 	}
 
 	/*
@@ -1028,9 +1052,9 @@
 			recvfn = detect_recv_fn(fd, TAG | CHANNEL);
 			close(fd);
 		}
-		snprintf(devbase, sizeof(devbase), "%s%d", device_string, current_board);
+		snprintf(devbase, sizeof(devbase), "%s%d.0", device_string, current_board);
 		if (open_dev(&fd, devbase) < 0)
-			errx(EX_IOERR, "%s: Error opening board #%d in recv_data\n", __func__, current_board);
+		  err(EX_IOERR, "%s: Error opening firewire controller #%d %s in recv_data\n", __func__, current_board, devbase);
 		(*recvfn)(fd, recv_data, TAG | CHANNEL, -1);
 		free(recv_data);
 	}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.