From owner-freebsd-firewire@FreeBSD.ORG Thu Aug 28 23:49:06 2008 Return-Path: Delivered-To: freebsd-firewire@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7F41E1065673 for ; Thu, 28 Aug 2008 23:49:06 +0000 (UTC) (envelope-from sbruno@miralink.com) Received: from plato.miralink.com (mail.miralink.com [70.103.185.20]) by mx1.freebsd.org (Postfix) with ESMTP id 4A2678FC13 for ; Thu, 28 Aug 2008 23:49:05 +0000 (UTC) (envelope-from sbruno@miralink.com) Received: from localhost (localhost.localdomain [127.0.0.1]) by plato.miralink.com (Postfix) with ESMTP id D45E71A90E0; Thu, 28 Aug 2008 16:39:30 -0700 (PDT) X-Virus-Scanned: amavisd-new at X-Spam-Flag: NO X-Spam-Score: -4.322 X-Spam-Level: X-Spam-Status: No, score=-4.322 tagged_above=-10 required=6.6 tests=[ALL_TRUSTED=-1.8, AWL=0.077, BAYES_00=-2.599] Received: from plato.miralink.com ([127.0.0.1]) by localhost (plato.miralink.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HNOXqA06v2jh; Thu, 28 Aug 2008 16:39:30 -0700 (PDT) Received: from [10.0.0.40] (iago.office.miralink.com [10.0.0.40]) by plato.miralink.com (Postfix) with ESMTP id 597BC1A90C3; Thu, 28 Aug 2008 16:39:30 -0700 (PDT) Message-ID: <48B73971.90706@miralink.com> Date: Thu, 28 Aug 2008 16:49:05 -0700 From: Sean Bruno User-Agent: Thunderbird 2.0.0.16 (X11/20080723) MIME-Version: 1.0 To: Dieter References: <200808282313.XAA17597@sopwith.solgatos.com> In-Reply-To: <200808282313.XAA17597@sopwith.solgatos.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-firewire@freebsd.org Subject: Re: This is where I'm going with fwcontrol X-BeenThere: freebsd-firewire@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Firewire support in FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Aug 2008 23:49:06 -0000 Dieter wrote: > While looking at the code and wondering what "adjust_gap_count" > had to do with cyclemaster mode, I noticed that -f was setting > the 2nd argument to > send_phy_config(int fd, int root_node, int gap_count) > which is root_node, not gap_count. > > I also noticed that send_phy_config() does (root_node & 0x3f), > so having the range check be 0x3f rather than INT32_MAX > seems to make sense. > > Agreed, the root_node should be checked against 0x3f(or a define that is assigned the value of 0x3f ... whatever that really is). > I haven't yet figured out exactly which 7 bits this stuff is > actually setting. > > send_phy_config() is causing the /dev/fwX device to generate a PHY configuration Packet(IEEE 1394-1995 4.3.4.3) It is possible to optimize Serial Bus performance for particular configurations in the following ways: a) Setting the gap_count used by all nodes to a smaller value (appropriate to the actual worst-case number of hops between any two nodes) b) Forcing a particular node to be the root after the next bus initialization (for example, in isochronous systems, the root shall be cycle-master capable) Both of these actions shall be done for remote nodes using the PHY configuration packet shown in figure 4.20. (For the local node, the PH_CONT.request service is used) The procedures for using this PHY packet are described in clause 7.3.5.2.1. So, I think you are correct adjust_gap_count should become set_root_node as that is what it does. A PHY Config packet should look like this: Message ID Root ID R T Gap Count 00(2 bits) (5 bits) 1 1 (6 bits) So the Root ID(-f in fwcontrol) can't be set to a value greater than 0x3f(63) and the Gap Count can't be greater than 0x7f(127). An "R" value of 1 means that this is a message to set the root ID. An "T" value of 1 means that this is a message to set the gap count for all nodes. If "R" and "T" are both 0, then the message is ignored. > So does the following look like an improvement (however minor) > to anyone else, or just me? :-) > > > @@ -730,7 +730,7 @@ > bool dump_phy_reg = false; > > int32_t priority_budget = -1; > - int32_t adjust_gap_count = -1; > + int32_t set_root_node = -1; > int32_t reset_gap_count = -1; > int32_t send_link_on = -1; > int32_t send_reset_start = -1; > @@ -790,9 +790,9 @@ > display_board_only = false; > break; > case 'f': > - adjust_gap_count = strtol(optarg, NULL, 0); > - if ( (adjust_gap_count < 0) || (adjust_gap_count > INT32_MAX) ) > - err(EX_USAGE, "%s:adjust_gap_count out of range", __func__); > + set_root_node = strtol(optarg, NULL, 0); > + if ( (set_root_node < 0) || (set_root_node > 0x3f) ) > + err(EX_USAGE, "%s:set_root_node out of range", __func__); > open_needed = true; > command_set = true; > display_board_only = false; > @@ -961,8 +961,8 @@ > /* > * Adjust the gap count for this card/bus to value "-f" > */ > - if (adjust_gap_count >= 0) > - send_phy_config(fd, adjust_gap_count, -1); > + if (set_root_node >= 0) > + send_phy_config(fd, set_root_node, -1); > > /* > * Reset the gap count for this card/bus "-g" > I will chew on and implement this patch tomorrow I think. It makes sense to me, it's just the code in send_phy_config() that is distracting. I hope this clarifies what this crap means! :) -- Sean Bruno MiraLink Corporation 6015 NE 80th Ave, Ste 100 Portland, OR 97218 Phone 503-621-5143 Fax 503-621-5199 MSN: sbruno@miralink.com Google: seanwbruno@gmail.com Yahoo: sean_bruno@yahoo.com