On Sunday 23 September 2012 22:43:16 Jie Zhang wrote: > This patch adds Analog Devices BF609 support to UrJTAG. BF609 has an > SDU (System Debug Unit), which does not exist in previous Blackfin > processors. This patch treats SDU as a new part type. > > Because the special design of TAPs in BF609 processors, new functions > are added to manually add/remove Blackfin core TAPs in/from scan > chain, like urj_part_parts_add_part_at and > urj_part_parts_remove_part_at in src/part/part.c. > > This patch also add a bus driver for BF609 EZ-BOARD.
the sdu.h header needs a comment block at the top like other headers. it
probably should also be named "bfin-sdu.h" since it's entirely ADI-specific.
similarly, i'd move src/sdu/ to src/bfin/sdu/ and rename "sdu" in user visible
strings to "bfin-sdu" or something.
it should also include parts.h and stdint.h since it uses things from them.
it would simplify things if we always initialized part->params->data. is that
not possible ?
i generally over looked the API ugliness in the bfin.h because it was such a
large blob and only Blackfin-specific stuff would include it. but you can't
add
that stuff to part.h w/out a URJ_xxx prefix. i'm not sure why even bother
putting PART_XXX() helpers in the header in the first place. update the funcs
like so:
bool
urj_part_get_bypassed (urj_part_t *part)
{
if (urj_part_has_data (part))
{
urj_part_data_t *part_data = part->params->data;
return part_data->bypass;
}
else
return true;
}
i would convert the urj_part_bypass to a single set func:
void
urj_part_set_bypass (urj_part_t *part, bool bypass)
{
if (urj_part_has_data (part))
{
urj_part_data_t *part_data = part->params->data;
part_data->bypass = bypass;
}
}
the help string in src/cmd/cmd_part.c needs to be line wrapped.
urj_tap_manual_add_at should take a const char *name. the strncpy in there
probably mishandles NUL if someone passes in a really long name, so i'd make
sure the last byte is always set to '\0'.
in urj_tap_manual_init, pretty sure you don't need to line wrap the calls to
urj_part_find_instruction. also, why do you have to construct the full path to
the data_file ? doesn't the default parse func search the urjtag data dir ? i
think the error handling in that func is also incorrect. pretty sure it
should just be:
urj_log_error_describe (URJ_LOG_LEVEL_ERROR);
also, all calls to assert() are wrong. return an error instead.
in src/part/part.c:urj_part_parts_add_part_at, rather than a for loop, why not
use memmove() ? you should also do checking on the "n" variable to make sure
it isn't <0 or >p->len. similarly, urj_part_parts_remove_part_at should use
memmove().
the changes to src/bus/blackfin.c should be using the new
urj_part_set_signal_high() helper funcs.
i didn't look too closely at the new sdu.c file. i did see some assert()'s
which need fixing, and sdu_part_data_initializer should get marked static.
the header files in src/cmd/cmd_sdu.c need trimming. no need for wait.h. it
also needs to wrap the error messages with _(...) for translation. and drop
all calls to assert(). when it comes to checking params, you wouldn't have to
indent so much if you changed the logic. instead of doing:
if (check param count) {
... normal code ...
} else {
... show error ...
}
you just had it return early:
if (check param count) {
... show error & return ...
}
... normal code ...
there's one place where you use "reseting" when it's spelled "resetting"
the signals in data/analog/sdu/sdu (and lack there of in
data/analog/bf609/bf609) make me suspicious. shouldn't the pins be wired up
to the bf609 cores ? or is everything routed through the sdu ? if the
latter, then there should be a "sdu-bf609" file with the common sdu stuff
remaining in the "sdu" file. also, how come bf609/bf609 isn't including
bfin/bfin like all the other blackfin parts ?
in the src/bfin/bfin.c:bfin_set_scan() change, you deleted the major if check,
but didn't re-indent the body.
-mike
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________ UrJTAG-development mailing list UrJTAG-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/urjtag-development
