Dear all,
I finally found time to peek into the gpio cable again.
After this there are two patches for it.
First one adds a missing fseek; without it only one TDO value is read.
The second patch moves from FILE pointers to file descriptors. Gain by
this is substantial. (about 35% in my test). See patch file for
details
There are also some smaller optimisation opportunities, but I guess
they won't bring very much.
One of the things I did was inlining gpio_set_value
#define gpio_set_value(fd,value) { char gpio_value = (value) + '0';
write(fd, &gpio_value, 1); }
This gives an additional gain of about 1.5 %
(i can provide a patch if needed)
(btw: note that the return value of gpio_set_value is never used)
Some other remarks on the code:
static int gpio_get_value (int *fd, unsigned int gpio)
{
ssize_t ret;
char value;
lseek(fd, 0, 0);
ret = read(fd, &value, 1);
if (ret != 1)
{
urj_warning (_("Error getting value of gpio %u\n"), gpio);
return URJ_STATUS_FAIL;
}
return value == '1';
}
Here gpio is only used for the warning. We could consider taking out
the argument
gpio_clock.c has
for (i = 0; i < n; i++)
{
gpio_set_value (p->fd_gpios[GPIO_TCK], 0);
gpio_set_value (p->fd_gpios[GPIO_TCK], 1);
gpio_set_value (p->fd_gpios[GPIO_TCK], 0);
}
We could move the last set_value out of the loop. This did not result
in a measurable performance gain.
gpio_current_signals:
this says:
int sigs = p->signals & ~(URJ_POD_CS_TMS | URJ_POD_CS_TDI | URJ_POD_CS_TCK);
if (p->lastout & URJ_POD_CS_TCK) sigs |= URJ_POD_CS_TCK;
if (p->lastout & URJ_POD_CS_TDI) sigs |= URJ_POD_CS_TDI;
if (p->lastout & URJ_POD_CS_TMS) sigs |= URJ_POD_CS_TMS;
The last three ifs are probably better written as:
sigs |= (p->lastout & (URJ_POD_CS_TMS | URJ_POD_CS_TDI | URJ_POD_CS_TCK));
Looks somewhat more efficient, but probably will not give a substantial gain.
gpio_set_signal:
personally I would remove
if (mask != 0)
Guess it will be very rare that mask is 0, but if that is the case the
next 3 if statements will not execute the then branch so the behaviour
is the same.
gpio_set_tdo:
this one has:
gpio_set_value(p->fd_gpios[GPIO_TCK], 0);
gpio_set_value(p->fd_gpios[GPIO_TDI], 0);
gpio_set_value(p->fd_gpios[GPIO_TMS], 0);
p->lastout &= ~(URJ_POD_CS_TMS | URJ_POD_CS_TDI | URJ_POD_CS_TCK);
there are no other bits in lastout than these three, so this can be
simplified to p->lastout = 0;
(I expect no performance gain from this, the compiler will fold the
expression to a nice constant)
Actually maybe there are some more optimisations to lastout possible.
It can also be discussed if the first line (setting TCK to 0) is
actually needed. After set_clock it is 0, but not sure what set_signal
might do, so perhaps it is better to save it.
What could still help in improving the performance is to cache the
values in gpio_set_value and, upon the next invocation, see if the
value has changed and if not, simply return. This will avoid some I/O
overhead.
Btw: some test data:
writing a 2.4 MB file to an ep3c40 via usbblaster:
real 0m 35.15s
user 0m 28.94s
sys 0m 0.04s
and via gpio with file descriptors
real 1m 12.76s
user 0m 33.12s
sys 0m 33.59s
and via gpio with file pointers (the original code)
real 1m 59.65s
user 1m 12.95s
sys 0m 36.68s
Guess most of the usbblaster timeis to read and process the svf file.
Btw this is all on a 1.2 Ghz PPC (e500v2 core).
On my quad core Xeon writing the same image using usbblaster takes
real 5.32
user 1.85
sys 0.10
(hm: 7 times as fast as PPC, hadn't expected that).
Best regards Frans.
------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users
worldwide. Take advantage of special opportunities to increase revenue and
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development
