Date: Thu, 28 Aug 2008 16:49:05 -0700 From: Sean Bruno <sbruno@miralink.com> To: Dieter <freebsd@sopwith.solgatos.com> Cc: freebsd-firewire@freebsd.org Subject: Re: This is where I'm going with fwcontrol Message-ID: <48B73971.90706@miralink.com> In-Reply-To: <200808282313.XAA17597@sopwith.solgatos.com> References: <200808282313.XAA17597@sopwith.solgatos.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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)
<snippet from document>
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.
<end snippet>
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?48B73971.90706>