search

enjoy-digital / litesdcard

Small footprint and configurable SDCard core
Other
107 stars 35 forks source link

litesdcard: fix handling LONG responses #7

Closed mateusz-holenko closed 4 years ago

mateusz-holenko commented 4 years ago

This also disables CRC checks as they fail for some commands. This must be investigated further.

This will be followed by two more PRs:

mateusz-holenko commented 4 years ago

CC @kgugala

gsomlo commented 4 years ago

for one thing, this patch removes sdclk_mmcm_* CSRs, whose accessors are called in the example bios code (here and in the main LiteX sdcard.c files. Could you maybe also update at least the demo C code in this repo so that the example software would still build?

enjoy-digital commented 4 years ago

@gsomlo: indeed, i was also going to ask that to be able to be able to test the changes.

gsomlo commented 4 years ago

the clocker and data.py changes are now upstream, which leaves just the litesdcard/core.py changes (the actual "meat" of this PR, afaict -- maybe a rebase/force-push would be worth doing at this point).

I did try the litesdcard/core.py changes with rocket and nexys4ddr (see https://github.com/enjoy-digital/litex/pull/408), but it stopped even the one sdcard I have that used to work from passing sdinit, at any frequency. There's always the chance I typo-ed the core.py changes when typing them in manually, which is why a force-push would be helpful :)

mateusz-holenko commented 4 years ago

Thanks for the info - we'll rebase this and verify how it works against the current upstream.

enjoy-digital commented 4 years ago

@gsomlo, @mateusz-holenko: for info, i merged manually the clocker and data changes because i was not able to get things working using the full PR, so i started validating/merging things progressively.

With the changes already merged, i've been able to get the sdcard tests working: https://github.com/enjoy-digital/litex/blob/master/litex/soc/software/bios/main.c#L386-L390

But not with the changes in core.py. I was planning to investigate on that today.

mateusz-holenko commented 4 years ago

We rebased our changes on top of the current master and split it into two separate commits to make debugging/merging easier.

because i was not able to get things working using the full PR

@enjoy-digital What problems did you encounter?

enjoy-digital commented 4 years ago

@mateusz-holenko: the SDCard was not initializing correctly. I'll test your updated PR in the afternoon.

mateusz-holenko commented 4 years ago

@enjoy-digital - what are you testing it on? We had to introduce some changes to LiteX (including BIOS) in order to make it work. Currently they are available on the branch https://github.com/antmicro/litex/commit/179b725ff4aaa57038b0886a7c0c7820db89a773, but once we clean them up we are planning to create a PR to mainline.

enjoy-digital commented 4 years ago

I'm testing on the Nexys4DDR. I'll look at your changes.

mateusz-holenko commented 4 years ago

@enjoy-digital Can we help you somehow with testing the changes?

enjoy-digital commented 4 years ago

@mateusz-holenko: sorry i forgot to test it. I'll do that today.

kgugala commented 4 years ago

@enjoy-digital did you have time to review this? This is slowly becoming a blocker for us

enjoy-digital commented 4 years ago

@kgugala: sorry, i'm going to test.

enjoy-digital commented 4 years ago

@mateusz-holenko, @kgugala: i've been trying to test the code but i'm having trouble reproducing my previous results. Can you tell me how you are testing it? We could merge if this is blocking you, but i 'd like to able to reproduce your results to look at the code that is now commented.

mateusz-holenko commented 4 years ago

@enjoy-digital: We test it on arty A7 using the modified LiteX BIOS (https://github.com/antmicro/litex/commit/179b725ff4aaa57038b0886a7c0c7820db89a773) using the following commands:

sdclk 25
sdinit
sdtest 10

The initialization of the SD Card works for us and we get the following output:

litex> sdclk 25
litex> sdinit
CMD0: GO_IDLE
CMD8: SEND_EXT_CSD, arg: 0x000001aa
00000000 00000000 00000000 00000008 000001aa 
Accepted voltage: 2.7-3.6V
CMD55: APP_CMD
00000000 00000000 00000000 00000037 00000120 
ACMD41: APP_SEND_OP_COND, arg: 70ff8000
00000000 00000000 00000000 0000003f 00ff8000 
CMD55: APP_CMD
00000000 00000000 00000000 00000037 00000120 
ACMD41: APP_SEND_OP_COND, arg: 70ff8000
00000000 00000000 00000000 0000003f c0ff8000 
CMD2: ALL_SEND_CID
0000003f 02544d53 41313647 43001af4 4c0129e9 
CID Register: 0x02544d534131364743001af44c0129e9
Manufacturer ID: 0x254
Application ID 0x4d53
Product name: A16GC
CRC: e9
Production date(m/yy): 9/18
PSN: 001af44c
OID: TM
CMD3: SET_RELATIVE_ADDRESS
00000000 00000000 00000000 00000003 12340500 
CMD10: SEND_CID
0000003f 02544d53 41313647 43001af4 4c0129e9 
CMD9: SEND_CSD
0000003f 400e0032 5b590000 734f7f80 0a40001b 
CSD Register: 0x400e00325b590000734f7f800a40001b
Max data transfer rate: 64 MB/s
Max read block length: 512 bytes
Device size: 14 GB
CMD7: SELECT_CARD
00000000 00000000 00000000 00000007 00000700 
CMD55: APP_CMD
00000000 00000000 00000000 00000037 00000920 
ACMD6: SET_BUS_WIDTH
00000000 00000000 00000000 00000006 00000920 
CMD6: SWITCH_FUNC
00000000 00000000 00000000 00000006 00000900 
CMD6: SWITCH_FUNC
00000000 00000000 00000000 00000006 00000900 
CMD55: APP_CMD
00000000 00000000 00000000 00000037 00000920 
CMD51: APP_SEND_SCR
00000000 00000000 00000000 00000033 00000920 
CMD16: SET_BLOCKLEN
00000000 00000000 00000000 00000010 00000900 
litex> sdtest 10
Writing block 0 (1/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 1 (2/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 2 (3/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 3 (4/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 4 (5/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 5 (6/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 6 (7/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 7 (8/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 8 (9/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing block 9 (10/10); crc errors: 0; timeouts: 0; datawcrc: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD24: WRITE_SINGLE_BLOCK
00000000 00000000 00000000 00000018 00000900 
CMD12: STOP_TRANSMISSION
Writing crc errors: 0; timeouts: 0; datawcrc: 0
Reading and checking block 0 (1/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 1 (2/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 2 (3/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 3 (4/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 4 (5/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 5 (6/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 6 (7/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 7 (8/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 8 (9/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading and checking block 9 (10/10); errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
CMD23: SET_BLOCK_COUNT
00000000 00000000 00000000 00000017 00000900 
CMD17: READ_SINGLE_BLOCK
Reading errors: 0 in 0 blocks; crc errors: 0; timeouts: 0
mateusz-holenko commented 4 years ago

@enjoy-digital - as @kgugala mentioned this becomes a blocker for us. Would it be possible for you to merge this and we will continue the discussion/testing in the meantime?

enjoy-digital commented 4 years ago

I've been able to test it successfully on the Nexys4DDR by integrating your gateware and software changes. So we can merge it. Could you do a PR to LiteX with the software changes once you think it's ready?

mateusz-holenko commented 4 years ago

That's great, thanks! Yes, I'll now work on a PR to LiteX.