Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ftdi_write_data, ftdi_set_bitmode, not used bits must be set to 1 #8

Open
rhasst opened this issue Dec 25, 2018 · 7 comments
Open

Comments

@rhasst
Copy link

rhasst commented Dec 25, 2018

Hi,

I was testing your code with the following setup and it did not work.

  • raspbery PI 2B,
  • vivado 2018.2
  • arty35T FPGA with FTDI (Bus 001 Device 007: ID 0403:6010 Future Technology Devices International, Ltd FT2232C Dual USB-UART/FIFO IC)

Vivado did connect to xvcd server but it did not recognize the FPGA. Vivado error:

ERROR: [Labtools 27-2269] No devices detected on target localhost:3121/xilinx_tcf/Xilinx/192.168.0.107:2542.
Check cable connectivity and that the target board is powered up then
use the disconnect_hw_server and connect_hw_server to re-register this hardware target.
ERROR: [Common 17-39] 'open_hw_target' failed due to earlier errors.

I connected scope to JTAG TDI pin and it stayed high for the whole time. Although the TDI (from verbose) was saying that it did set TDI to some value. verbose TDO was 1 all the time.

I applied the following fix to io_ftdi.c:

--- a/src/io_ftdi.c
+++ b/src/io_ftdi.c
@@ -14,6 +14,7 @@
#define PORT_TDO 0x04
#define PORT_TMS 0x08
#define IO_OUTPUT (PORT_TCK|PORT_TDI|PORT_TMS)
+#define IO_MASK 0xF0

// NOTE: Moved definitions to Makefile to make it easier to
// build for different platforms.
@@ -127,7 +128,7 @@ int io_init(int vendor, int product, const char* serial, unsigned int index, uns
}

- res = ftdi_set_bitmode(&ftdi, IO_OUTPUT, BITMODE_SYNCBB);
+ res = ftdi_set_bitmode(&ftdi, IO_MASK | IO_OUTPUT, BITMODE_SYNCBB);

@@ -214,6 +215,9 @@ int io_scan(const unsigned char *TMS, const unsigned char *TDI, unsigned char *T
v |= PORT_TMS;
if (TDI[i/8] & (1<<(i&7)))
v |= PORT_TDI;
+
+ v |= IO_MASK;
+
buffer[i * 2 + 0] = v;
buffer[i * 2 + 1] = v | PORT_TCK;
}

When using ftdi_write_data, buf must have top 4 bits set to 1 and you must not mask those same bit out when using ftdi_set_bitmode.

With the above fix, I am now able to program FPGA and connect to it with chipscope with Vivado.

@tmbinc
Copy link
Owner

tmbinc commented Feb 10, 2022

This should be fixed with 0e03eca (#7). It sets slightly different default values though.

@SuibianP
Copy link

@tmbinc Current HEAD (3de438b) still does not work with my Nexus 4 DDR board with exactly the same symptoms, and the patch fixes it.

--- a/src/io_ftdi.c
+++ b/src/io_ftdi.c
@@ -15,6 +15,7 @@
 #define PORT_TMS            0x08
 #define PORT_MISC           0x90
 #define IO_OUTPUT (PORT_MISC|PORT_TCK|PORT_TDI|PORT_TMS)
+#define IO_MASK 0xF0

 #define IO_DEFAULT_OUT     (0xe0)               /* Found to work best for some FTDI implementations */

@@ -131,7 +132,7 @@
 	}

 	ftdi_set_bitmode(&ftdi, 0xFF, BITMODE_CBUS);
-	res = ftdi_set_bitmode(&ftdi, IO_OUTPUT, BITMODE_SYNCBB);
+	res = ftdi_set_bitmode(&ftdi, IO_MASK | IO_OUTPUT, BITMODE_SYNCBB);

 	if (res < 0)
 	{
@@ -226,6 +227,7 @@
 			v |= PORT_TMS;
 		if (TDI[i/8] & (1<<(i&7)))
 			v |= PORT_TDI;
+		v |= IO_MASK;
 		buffer[i * 2 + 0] = v;
 		buffer[i * 2 + 1] = v | PORT_TCK;
 	}

@davidchisnall
Copy link

I can confirm that the version from the above diff works with an Arty A7 100, but the version in the repo does not.

@tmbinc
Copy link
Owner

tmbinc commented Nov 10, 2023

@davidchisnall @SuibianP thanks for testing the change.

I unfortunately don't have a board that requires this so I can't test it. What the change essentially is doing is to configure more pins as output (bitmask argument to ftdi_set_bitmode - old mask was IO_OUTPUT, 'b10011111, new mask is now 'b11111111), and also setting bit 4 (v |= IO_MASK), which previously was not driven (IO_DEFAULT_OUT is 'b11100000). So effectively:

  • Bit 6 is now driven to high, instead of leaving it floating.
  • Bit 5 is now driven to high, instead of leaving it floating.
  • Bit 4 is now driven to high instead of being driven to low.

This should be equivalent to the following change:

diff --git a/src/io_ftdi.c b/src/io_ftdi.c
index 881c2c4..b5a65e6 100644
--- a/src/io_ftdi.c
+++ b/src/io_ftdi.c
@@ -13,10 +13,9 @@
 #define PORT_TDI            0x02
 #define PORT_TDO            0x04
 #define PORT_TMS            0x08
-#define PORT_MISC           0x90
+#define PORT_MISC           0xf0 /* Additional pins to drive as output */
 #define IO_OUTPUT (PORT_MISC|PORT_TCK|PORT_TDI|PORT_TMS)
-
-#define IO_DEFAULT_OUT     (0xe0)               /* Found to work best for some FTDI implementations */
+#define IO_DEFAULT_OUT     (0xf0)               /* Constant pin values to drive, additionally to the JTAG pins*/

 // NOTE: Moved definitions to Makefile to make it easier to
 // build for different platforms.

@davidchisnall any chance you can see if this is indeed equivalent or whether I messed up something in my head?

Unfortunately, Sheet #10 in the arty schematics (https://digilent.com/reference/_media/programmable-logic/arty-a7/arty-a7-e2-sch.pdf), which contains the relevant circuit ("USB PROG/UART") is "intentionally left blank", and I couldn't find the relevant snippet in any other official schematics.

The risk is that we're driving an IO pin that is an input. But honestly that risk is already there for the other bits, so I'm tempted to merge this if it solves a problem. Best action is probably to make this dynamic per config/command line, as it's really board-specific.

E.g. mithro's digilent_arty.cfg for openOCD (https://gist.github.com/mithro/9f60cbd59c91b015c6491a0b2a091500 ) uses

ftdi_layout_init 0x0088 0x008b

where 0x88 is the initial output value (our "IO_DEFAULT_OUT"), and 0x8b the default direction ("IO_OUTPUT").

Is the issue that we're driving bit 4 low, whereas it is floating and should be left floating or driven high? Would #define PORT_MISC 0x80 already fix this, maybe?

@davidchisnall
Copy link

Thanks (hi @tmbinc , it's been a while!), that also works for me.

@tmbinc
Copy link
Owner

tmbinc commented Nov 10, 2023

(Hi as well!) Thank you so much for testing! To clarify - did you test PORT_MISC=0xF0/IO_DEFAULT_OUT=0xF0 (the diff above) or also PORT_MISC=0x80 DEFAULT_OUT=0x80 (or 0x8B, shouldn't matter here), i.e. the digilent_arty.cfg-like setting?

@davidchisnall
Copy link

I tested your diff, not the arty cfg change. I can do the latter this morning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants