sd package: sdcard package redesign#639
Conversation
|
Is it possible to make this implement BlockDevice so that it is natively compatible with tinyfs? Skimming https://github.com/tinygo-org/tinyfs/blob/release/tinyfs.go#L43 |
|
@bgould Will do, but I'll do it through another type that uses the ReadBlocks/WriteBlocks interface set to implement the tinyfs.BlockDevice interface. Already had this in mind as the next addition to |
|
@bgould I've added a draft of what I mean: https://github.com/tinygo-org/drivers/pull/639/files#diff-92f519b8e01de4c93b473c245b482f67da4e20bfbfbe48aaca799f35e45afb43R38 |
|
@bgould OK, I've finished the sd.BlockDevice type, seems to be working OK. Check it out and let me know what you think :) |
|
@soypat would you perhaps be able to add a README with some small explanation about how this works? For example what is |
|
OK, added a Readme and switched some filenames around to make navigation easier. |
|
@bgould have you had a chance to try this out yet? |
|
Paging Dr. @bgould 😸 any chance to try this out? |
|
I need this for a class I'll be teaching in 1.5 weeks :) |
|
Can we merge this? |
|
You should add something to Thanks! |
| for i := 0; i < numblocks; i++ { | ||
| dataoff := i * blocksize | ||
| d.csEnable(true) | ||
| _, err := d.read_block_single(dst[dataoff:dataoff+blocksize], int64(i)+startBlockIdx) |
There was a problem hiding this comment.
Multi-block read addressing bug for non-SDHC cards.
In ReadBlocks, after startBlockIdx <<= 9 for non-SDHC cards (converting block index to byte address), the multi-block loop passes int64(i) + startBlockIdx to read_block_single. This adds a block count to a byte address. It should be int64(i)*int64(blocksize) + startBlockIdx. SDHC (block-addressed) probably works correctly by accident.
| return (b & (1 << from)) >> from << to | ||
| } | ||
| // See 4.9.5 R6 (Published RCA response) of the SD Simplified Specification. | ||
| s := status(binary.BigEndian.Uint16(r.data[1:5])) |
There was a problem hiding this comment.
binary.BigEndian.Uint16(r.data[1:5]) reads bytes 1–2 (the RCA field), not bytes 3–4 (the card status bits). Should be Uint16(r.data[3:5]).
|
|
||
| // RateKilobits returns the transfer rate in kilobits per second. | ||
| func (t TransferSpeed) RateKilobits() int64 { | ||
| return 100 * log10table[t&0b111] |
There was a problem hiding this comment.
log10table has 7 entries (indices 0–6), but t&0b111 can be 7. Index 7 panics. The rate-unit value 7 is reserved in the spec but could appear on a misbehaving card.
|
|
||
| // AccessTime returns the asynchronous part of the data access time. | ||
| func (t TAAC) AccessTime() (d time.Duration) { | ||
| return time.Duration(log10table[t&0b111]) * time.Nanosecond |
|
|
||
| // ReadAt implements the [io.ReaderAt] interface for an SD card. | ||
| // Reads need not be aligned to block boundaries. | ||
| func (bd *BlockDevice) ReadAt(p []byte, off int64) (n int, err error) { |
There was a problem hiding this comment.
No bounds checking against device size in this function.
| // WriteAt implements the [io.WriterAt] interface for an SD card. Writes need | ||
| // not be aligned to block boundaries: partial blocks are read, modified and | ||
| // written back. | ||
| func (bd *BlockDevice) WriteAt(p []byte, off int64) (n int, err error) { |
There was a problem hiding this comment.
No bounds checking against device size in this function either.
New Features:
Is still a WIP. CRC7 still does not work and I'm getting errors on ever other ReadBlock call.