Dear Pantelis,
in message 20061129172539.24638.52486.stgit@pantathon.hol.gr you wrote:
Add support for Lattice FPGA parts programmed using JTAG.
Again, the previous comments on necessary coding style cleanup apply.
Also:
common/Makefile | 4 common/ec.c | 473 ++++++
^^^^^^^^^^^^^^
Huh? What's "ec"? Please use a more descriptive name.
--- /dev/null +++ b/common/lattice_ivm_core.c
...
+* JTAG state machine transistion tragetory table.
Please fix typos ;-)
+struct {
- unsigned char CurState; /* From this state */
- unsigned char NextState; /* Step to this state */
- unsigned char Pattern; /* The tragetory of TMS */
- unsigned char Pulses; /* The number of steps */
+} g_JTAGTransistions[24] = {
- {
- RESET, RESET, 0xFC, 6}, /* Transitions from RESET */
- {
- RESET, IDLE, 0x00, 1}, {
- RESET, DRPAUSE, 0x50, 5}, {
- RESET, IRPAUSE, 0x68, 6}, {
- IDLE, RESET, 0xE0, 3}, /* Transitions from IDLE */
- {
- IDLE, DRPAUSE, 0xA0, 4}, {
- IDLE, IRPAUSE, 0xD0, 5}, {
- DRPAUSE, RESET, 0xF8, 5}, /* Transitions from DRPAUSE */
- {
- DRPAUSE, IDLE, 0xC0, 3}, {
- DRPAUSE, IRPAUSE, 0xF4, 7}, {
- IRPAUSE, RESET, 0xF8, 5}, /* Transitions from IRPAUSE */
- {
- IRPAUSE, IDLE, 0xC0, 3}, {
- IRPAUSE, DRPAUSE, 0xE8, 6}, {
- DRPAUSE, SHIFTDR, 0x80, 2}, /* Extra transitions using SHIFTDR */
- {
- IRPAUSE, SHIFTDR, 0xE0, 5}, {
- SHIFTDR, DRPAUSE, 0x80, 2}, {
- SHIFTDR, IDLE, 0xC0, 3}, {
- IRPAUSE, SHIFTIR, 0x80, 2}, /* Extra transitions using SHIFTIR */
- {
- SHIFTIR, IRPAUSE, 0x80, 2}, {
- SHIFTIR, IDLE, 0xC0, 3}, {
- DRPAUSE, DRCAPTURE, 0xE0, 4}, /* 11/15/05 Nguyen changed to support DRCAPTURE */
- {
- DRCAPTURE, DRPAUSE, 0x80, 2}, {
- IDLE, DRCAPTURE, 0x80, 2}, {
- IRPAUSE, DRCAPTURE, 0xE0, 4}
+};
Please fix indentation.
+#ifdef VME_DEBUG +/*************************************************************** +* +* vme_print +* +* Prints VME to SVF file or terminal, depending on the +* pre-compiler definitions. This function may be compiler or OS +* specific. Please see your compiler's documentation regarding +* va_list, va_arg, and va_end. +* +***************************************************************/
Is this really needed?
+void vme_print(const char *a_pszString, ...) +{
- va_list list;
- unsigned short usStringIndex;
- static char szString[5000] = { 0 };
- static char szTempString[5000] = { 0 };
Do you really need static stings here?
- va_start(list, a_pszString);
- for (usStringIndex = 0; usStringIndex < strlen(a_pszString);
usStringIndex++) {if (a_pszString[usStringIndex] == '%') {szString[strlen(szString)] = a_pszString[usStringIndex];usStringIndex++;
Please note that the variable names are basicly a violation of the Coding Style (see Chapter 4: Naming). This code, even though it's not really complicated, is more or less unreadable as is.
...
+signed char ispVMRead(unsigned short a_usiDataSize)
...
- for (usDataSizeIndex = 0; usDataSizeIndex < a_usiDataSize;
usDataSizeIndex++) {if (cByteIndex == 0) {/***************************************************************** Grab byte from TDO buffer.****************************************************************/if (g_usDataType & TDO_DATA) {cDataByte = g_pucOutData[usBufferIndex];}/***************************************************************** Grab byte from MASK buffer.****************************************************************/
Comments don't line up here and elsewhere.
+/*************************************************************** +* +* internal_udelay +* +* Users must devise their own timing procedure to ensure the +* specified minimum delay is observed when using different +* platform. The timing function used here is for PC only by +* hocking the clock chip. +* +***************************************************************/
+void internal_udelay(unsigned short a_usMicroSecondDelay) +{
- int i, msecs;
- if (a_usMicroSecondDelay & 0x8000) {
msecs = a_usMicroSecondDelay & ~0x8000;for (i = 0; i < msecs; i++)udelay(1000);return;- }
- udelay(a_usMicroSecondDelay);
+}
Comment and implementation don't match. Also: are you sure this is needed in U-Boot?
Best regards,
Wolfgang Denk