Bummer, I added a big bug in that last one. By attempting to use a high-OUT, for 15kR pull-up of the rx data pin (MISO), in place of the default low-OUT-with-inversion, I didn't notice I already had a low-OUT as part of the "select" function.
The bug manifested, for me, in a not commonly tested way. It would function fine if the SD card was already in SPI mode. Say from an alternative test run using an older routine. Only after a power down would the SD card fail to init from this bug.
All fixed hopefully this time. Rather than changing to that idea of setting OUT high, I've reverted back to having an inverted pin drive with OUT left low.
I stumbled into the C pitfall, again. I thought that
bool ok;
...
ok&= CheckCondition();
is the same as
if (ok) ok= CheckCondition();
like it is defined in C++, e.g. if ok is false in the first place then CheckCondition() is not executed at all. But my assumption was wrong. In (pure) C bool is just an alias for byte. So CheckCondition() is always executed and the result is ANDed with the previous state of ok.
I have to look it up in the Stroustrup book but I'm pretty sure that assignments to a bool variable in C++ are always expanded, e.g. bool ok= 1 is compiled to bool= 0xff. And boolean expressions are always "optimized", e.g. functions on the right side are not evaluated or executed if the term on the left side is already true for OR or already false for AND. This is the difference of boolean AND vs. bitwise AND (&& vs. &). And &= behaves like &&= for booleans.
@ManAtWork said:
I have to look it up in the Stroustrup book but I'm pretty sure that assignments to a bool variable in C++ are always expanded, e.g. bool ok= 1 is compiled to bool= 0xff. And boolean expressions are always "optimized", e.g. functions on the right side are not evaluated or executed if the term on the left side is already true for OR or already false for AND. This is the difference of boolean AND vs. bitwise AND (&& vs. &). And &= behaves like &&= for booleans.
No, &= never short circuits, only &&= does. Also, the "true" value in C++ is 1, not 0xff, although you are correct that bool does do simplification (e.g. bool ok = 2 compiles to bool ok = true which means that ok is really set to 1, not 2).
Sample program (I tried with g++ and clang++ under Linux):
#include <iostream>
bool
checkit()
{
std::cout << "checkit called\n";
return false;
}
int main()
{
bool f = false;
bool t = true;
f &= checkit();
std::cout << "done: t = " << int(t) << "\n";
return 0;
}
Never mind. I found out that moving the strings out of main(), making them globals, solved it.
I suspect I've got a C question here too. When I compile the following two snippets, the first snippet builds a binary smaller than the second snippet. When I look at the p2asm file of the first snippet I see each string duplicated a few times as per their uses. And checking the binary file they're there too, yet that is the smaller binary!
I guess I'm wondering if I'm doing something wrong in my attempt at hand optimising. I can see there is a hidden strcpy() occurring for the second snippet. I presume this consumes a chunk of heap space which in turn fills out the binary.
I kind of figured those two strings would be referenced directly for each use but they aren't.
EDIT: Fixed by making the strings into globals. Figures in hindsight, the scope of a string local wasn't suited to parameter passing.
Eric,
I can't figure out how to split off a second SD card driver under FAT control in include/filesystem/fatfs. I've just come back to trying to make it happen after revamping the tester program.
The 4-bit mode of operation is such a major rewrite, mainly from the alternative SD command sequencing but also requires streamer ops, that I wanted to start afresh while at the same time be able to quickly compare against the existing driver as I develop the new one.
BTW: I was trying to use both together - One driver talking on the regular 4 boot pins, the other driver on Roger's accessory board using 8 pins. Both using FAT FS. Maybe I'm expecting too much.
PS: Here's the terminal output with debug messages turned on:
umount(/sd1) called
umount /sd1: ENOENT
umount(/sd2) called
umount /sd2: ENOENT
open sdcardx: using pins: 61 60 59 58
&_pin_clk=10a90, _pin_clk = 61
sd card get_vfs: returning 10cf0
mount(/sd1, 10cf0) called
mount: using slot 0 for /sd1
_getvfsforfile(/sd1/): trying [/sd1]
_getvfsforfile: slot 0 returning 10cf0 for
disk_initialize: PINS=61 60 59 58
smartpin modes: 61=00064048 59=02000078 58=0300527a
idle OK
SDHC 10 MHz selected
ty = 12
SPI clock ratio = sysclock/4
disk_read: PINS=61 60 59 58
disk_read: PINS=61 60 59 58
disk_read: PINS=61 60 59 58
mount sd1: OK
open sdcard4b: using pins: 21 20 16
&_pin_clk=10d60, _pin_clk = 21
sd card get_vfs: returning 10fc0
mount(/sd2, 10fc0) called
mount: using slot 1 for /sd2
_getvfsforfile(/sd2/): trying [/sd1]
_getvfsforfile(/sd2/): trying [/sd2]
_getvfsforfile: slot 1 returning 10fc0 for
disk_initialize: PINS=21 20 16 77198
disk_read: PINS=21 20 16 77198
opendir: v->opendir() returned -1
mount sd2: Unknown error
Clean-up ...
errno = -1: Unknown error
umount(/sd1) called
umount looking at 4 chars of (/sd1)
umount: calling deinit
clear pins 61 60 59 58 58
umount(/sd2) called
umount looking at 4 chars of (/sd2)
umount: calling deinit
clear pins 21 20 16 0 77198
All is fine for mounting the first SD card on the boot pins. mount sd1: OK
First oddity for mount of second card is with _getvfsforfile(/sd2/): trying [/sd1]
And of note is disk_initialize: PINS=21 20 16 77198 which is clearly trying to use four pins, but the driver code in file sd4b.cc that I changed has only three pins in it ...
Where are you even trying to change it? There isn't as-is a second virtual layer for instantiating fatfs with a different block device. sdmm.cc is hardcoded in diskio.h
At this stage, I simply copied the whole directory into a second one called sdfatfs. So I guess I've kind of tried to build two FAT filesystems alongside each other. diskio.h and some others in there have been edited.
I added a new entry to include/sys/vfs.h: struct vfs *_vfs_open_sdcard4b(int pclk = 21, int pcmd = 20, int pdat = 16) _IMPL("filesys/sdfatfs/fatfs_vfs.c");
Edited that very referenced file, changing its FAT FS handler: struct __using("filesys/sdfatfs/fatfs.cc") *FFS;
and its debug message: __builtin_printf("open sdcard4b: using pins: %d %d %d\n", pclk, pcmd, pdat);
The diskio.h in there has all references to sdmm.cc changed to sd4b.cc instead:
change disk_initialize and friends to be function pointers that live in fatfs.cc (the VFS glue class). Then in _vfs_open_sdcardx etc, you'd do something like FFS->disk_initialize = &sdmm->disk_initialize; where sdmm would be struct __using("filesys/fatfs/sdmm.cc") *sdmm; Not sure if that'd actually work correctly though, idk.
If you worry that using function pointers would increase bloat, you would be right, but I just figured out how to cut loose about 1.3k of bloat in sdmm.cc, so I think that's fine: https://github.com/totalspectrum/spin2cpp/pull/435/files
@Wuerfel_21 said:
change disk_initialize and friends to be function pointers that live in fatfs.cc (the VFS glue class). Then in _vfs_open_sdcardx etc, you'd do something like FFS->disk_initialize = &sdmm->disk_initialize; where sdmm would be struct __using("filesys/fatfs/sdmm.cc") *sdmm; Not sure if that'd actually work correctly though, idk.
Basically, if you create a local variable and take a pointer to it, flexspin hates that and all variables in that function go on the stack. This is kind-of a spin-ism because Spin has no structures and guaranteed variable order, so lots of code will take a variable address and try to access the next variable through that, so all of them need to be on the stack. This never got fixed to work on per-variable granularity for the C frontend. That would really be a boon.
Flexprop uses the fromfile() to find source code to compile. Having two file with the same name is bad. The compiler will probably try to use both at the same time. The last one wins?
Mike, I got it to appear to work at least, without increasing Cog()'s stack size, by replacing cogstart() with __builtin_cogstart(). Dunno if I'd trust it though.
__builtin_cogstart(Cog(&data), Stack);
EDIT: Err, scratch that. I'd shuffled the placement of Stack[] at the same time.
@evanh : Ideally we would convert all of the public disk_* functions in sdmm.cc to call through a jump table of some kind so that we could layer FatFS on top of different devices, kind of like what the the littlefs file system does. (That's basically what @Wuerfel_21 already suggested.) For now FatFS can only sit on top of one device at a time, but you could use a define to select between them. Move the old sdmm.cc to sdmm_old.cc, and create a new sdmm_new.cc. Then make sdmm.cc look like:
// sdmm.cc: does nothing but include the real code
#ifdef _NEW_SDCARD
#include "sdmm_new.cc"
#else
#include "sdmm_old.cc"
#endif
and at compile time add -D_NEW_SDCARD to the command line to test your new code.
@iseries : Did you figure out why the variable was being printed incorrectly? Having a bigger stack often solves cog issues, and printf is really bad for needing a lot of stack.
@ManAtWork said:
Why? This worked in version 6.5 but doesn't in 6.8.0, now.
Eric changed how bools work. They're more spec-compliant now (all truthy values coerce to 1), but the implementation's kinda scuffed.
Anyways, I think (not confirmed yet) I figured out the aforementioned "SD driver doesn't for work for Chip" issue. Completely unrelated to the actual SD driver of course. Inexplicably, he doesn't have a normal P2EVAL USB breakout with the protection IC, he just hotwired some connector to the P2, which means he doesn't have the power enable pin. So, spot the issue in the USB driver:
' Handle Port protection enable and startup delay
cmp usb_enable_pin, #0 wc
if_ae drvl usb_enable_pin ' disable port
waitx _21ms_ ' Wait a while for everything to turn off
if_ae drvh usb_enable_pin ' Enable the port
waitx _21ms_ ' Hold to let the idle state get settled
(of course the update has been made available in all the usual places)
@ManAtWork : Ada's right, when I updated the bool type to work according to the C spec (it's quite a strange type compared to the others) I missed some places that needed changing. I've pushed an update to the source code to fix this, and also to make the storage format for C booleans 1 byte again. BASIC booleans are also 1 byte on the platforms that can handle loading signed bytes, which now that I've put in some fixes for nucode means everything except the P1 bytecode.
Does that mean there's a problem with signed char in bytecode C, too? That should really work. Though idk, that'd involve messing with, BCCompileMemOpExEx which is the sort of code that should probably be completely trashed and rewritten.
Comments
Bummer, I added a big bug in that last one. By attempting to use a high-OUT, for 15kR pull-up of the rx data pin (MISO), in place of the default low-OUT-with-inversion, I didn't notice I already had a low-OUT as part of the "select" function.
The bug manifested, for me, in a not commonly tested way. It would function fine if the SD card was already in SPI mode. Say from an alternative test run using an older routine. Only after a power down would the SD card fail to init from this bug.
All fixed hopefully this time. Rather than changing to that idea of setting OUT high, I've reverted back to having an inverted pin drive with OUT left low.
I stumbled into the C pitfall, again. I thought that
is the same as
like it is defined in C++, e.g. if ok is false in the first place then CheckCondition() is not executed at all. But my assumption was wrong. In (pure) C bool is just an alias for byte. So CheckCondition() is always executed and the result is ANDed with the previous state of ok.
I don't think that works in C++ either? I don't think the standard & operator short-circuits on bool.
I have to look it up in the Stroustrup book but I'm pretty sure that assignments to a bool variable in C++ are always expanded, e.g. bool ok= 1 is compiled to bool= 0xff. And boolean expressions are always "optimized", e.g. functions on the right side are not evaluated or executed if the term on the left side is already true for OR or already false for AND. This is the difference of boolean AND vs. bitwise AND (&& vs. &). And
&=behaves like&&=for booleans.Yeah, && is the logical comparison operator.
No,
&=never short circuits, only&&=does. Also, the "true" value in C++ is 1, not 0xff, although you are correct that bool does do simplification (e.g.bool ok = 2compiles tobool ok = truewhich means thatokis really set to 1, not 2).Sample program (I tried with g++ and clang++ under Linux):
#include <iostream> bool checkit() { std::cout << "checkit called\n"; return false; } int main() { bool f = false; bool t = true; f &= checkit(); std::cout << "done: t = " << int(t) << "\n"; return 0; }Never mind. I found out that moving the strings out of main(), making them globals, solved it.
I suspect I've got a C question here too. When I compile the following two snippets, the first snippet builds a binary smaller than the second snippet. When I look at the p2asm file of the first snippet I see each string duplicated a few times as per their uses. And checking the binary file they're there too, yet that is the smaller binary!
void main( void ) { uint32_t seed; int i; for(i = 0; i < 3; i++) { rmfile("/sd1/copy1.bin"); rmfile("/sd2/copy1.bin"); seed = _rnd(); printf("data seed = 0x%08x\n", seed); xoro32state = seed; writefile("/sd1/copy1.bin", 4_400_000); xoro32state = seed; checkfile("/sd1/copy1.bin"); copyfile("/sd1/copy1.bin", "/sd2/copy1.bin"); xoro32state = seed; checkfile("/sd2/copy1.bin"); } }void main( void ) { const char filepath1[] = "/sd1/copy1.bin"; const char filepath2[] = "/sd2/copy1.bin"; uint32_t seed; int i; for(i = 0; i < 3; i++) { rmfile(filepath1); rmfile(filepath2); seed = _rnd(); printf("data seed = 0x%08x\n", seed); xoro32state = seed; writefile(filepath1, 4_400_000); xoro32state = seed; checkfile(filepath1); copyfile(filepath1, filepath2); xoro32state = seed; checkfile(filepath2); } }I guess I'm wondering if I'm doing something wrong in my attempt at hand optimising. I can see there is a hidden strcpy() occurring for the second snippet. I presume this consumes a chunk of heap space which in turn fills out the binary.
I kind of figured those two strings would be referenced directly for each use but they aren't.
EDIT: Fixed by making the strings into globals. Figures in hindsight, the scope of a string local wasn't suited to parameter passing.
const char filepath1[] = "/sd1/copy1.bin"; const char filepath2[] = "/sd2/copy1.bin"; void main( void ) { uint32_t seed; int i; for(i = 0; i < 3; i++) { rmfile(filepath1); rmfile(filepath2); seed = _rnd(); printf("data seed = 0x%08x\n", seed); xoro32state = seed; writefile(filepath1, 4_400_000); xoro32state = seed; checkfile(filepath1); copyfile(filepath1, filepath2); xoro32state = seed; checkfile(filepath2); } }Eric,
I can't figure out how to split off a second SD card driver under FAT control in
include/filesystem/fatfs. I've just come back to trying to make it happen after revamping the tester program.The 4-bit mode of operation is such a major rewrite, mainly from the alternative SD command sequencing but also requires streamer ops, that I wanted to start afresh while at the same time be able to quickly compare against the existing driver as I develop the new one.
BTW: I was trying to use both together - One driver talking on the regular 4 boot pins, the other driver on Roger's accessory board using 8 pins. Both using FAT FS. Maybe I'm expecting too much.
@evanh That should not be a problem. When the driver starts up it installs its own virtual functions to access the driver.
If it's not working then your backend driver has an issue with setting the virtual functions for reading and writing to the driver.
Mike
Okay, any details on what I need to do to set these functions? Is there docs?
PS: Here's the terminal output with debug messages turned on:
All is fine for mounting the first SD card on the boot pins.
mount sd1: OKFirst oddity for mount of second card is with
_getvfsforfile(/sd2/): trying [/sd1]And of note is
disk_initialize: PINS=21 20 16 77198which is clearly trying to use four pins, but the driver code in filesd4b.ccthat I changed has only three pins in it ...__builtin_printf("disk_initialize: PINS=%d %d %d\n", PIN_CLK, PIN_CMD, PIN_DAT);Which means my edited driver is not the one being called here.
Where are you even trying to change it? There isn't as-is a second virtual layer for instantiating fatfs with a different block device. sdmm.cc is hardcoded in diskio.h
At this stage, I simply copied the whole directory into a second one called
sdfatfs. So I guess I've kind of tried to build two FAT filesystems alongside each other. diskio.h and some others in there have been edited.I added a new entry to include/sys/vfs.h:
struct vfs *_vfs_open_sdcard4b(int pclk = 21, int pcmd = 20, int pdat = 16) _IMPL("filesys/sdfatfs/fatfs_vfs.c");Edited that very referenced file, changing its FAT FS handler:
struct __using("filesys/sdfatfs/fatfs.cc") *FFS;and its debug message:
__builtin_printf("open sdcard4b: using pins: %d %d %d\n", pclk, pcmd, pdat);The diskio.h in there has all references to sdmm.cc changed to sd4b.cc instead:
DSTATUS disk_initialize (BYTE pdrv) _IMPL("sd4b.cc"); DSTATUS disk_status (BYTE pdrv) _IMPL("sd4b.cc"); DRESULT disk_read (BYTE pdrv, BYTE* buff, LBA_t sector, UINT count) _IMPL("sd4b.cc"); DRESULT disk_write (BYTE pdrv, const BYTE* buff, LBA_t sector, UINT count) _IMPL("sd4b.cc"); DRESULT disk_ioctl (BYTE pdrv, BYTE cmd, void* buff) _IMPL("sd4b.cc"); DRESULT disk_setpins (BYTE pdrv, int pclk, int pcmd, int pdat) _IMPL("sd4b.cc");I suspect that diskio.h is not the one in use. Somehow the original one is being used, resulting in the sdmm.cc driver being called.
Very strange. Though I'd try changing them to be function pointers so you can use the same fatfs class instance.
Also, I just noticed some bug/cruft in fatfs.cc: https://github.com/totalspectrum/spin2cpp/pull/434/files
Example?
change
disk_initializeand friends to be function pointers that live in fatfs.cc (the VFS glue class). Then in_vfs_open_sdcardxetc, you'd do something likeFFS->disk_initialize = &sdmm->disk_initialize;where sdmm would bestruct __using("filesys/fatfs/sdmm.cc") *sdmm;Not sure if that'd actually work correctly though, idk.If you worry that using function pointers would increase bloat, you would be right, but I just figured out how to cut loose about 1.3k of bloat in sdmm.cc, so I think that's fine: https://github.com/totalspectrum/spin2cpp/pull/435/files
Thanks, a tomorrow job for me.
LOL, good stuff. That's the same as I noted above with those strings, just on a more significant scale.
Basically, if you create a local variable and take a pointer to it, flexspin hates that and all variables in that function go on the stack. This is kind-of a spin-ism because Spin has no structures and guaranteed variable order, so lots of code will take a variable address and try to access the next variable through that, so all of them need to be on the stack. This never got fixed to work on per-variable granularity for the C frontend. That would really be a boon.
Flexprop uses the fromfile() to find source code to compile. Having two file with the same name is bad. The compiler will probably try to use both at the same time. The last one wins?
MIke
Eric can you tell me why this doesn't work?
#include <stdio.h> #include <propeller.h> void Cog(void *); int Stack[50]; int x; struct pass { int i; int j; int k; } data; int main(int argc, char** argv) { struct pass *b; b = &data; data.i = 12; data.j = 25; data.k = 45; cogstart(Cog, b, Stack, sizeof(Stack)); _waitms(5000); printf("i: %d, j: %d, k: %d\n", data.i, data.j, data.k); while (1) { _waitms(1000); } } /** * @brief Background cog function * @param *par void pointer to parameters * */ void Cog(void *par) { struct pass *d; d = par; printf("i: %d, j: %d, k: %d\n", d->i, d->j, d->k); while(1) { _pinl(56); _waitms(500); _pinh(56); _waitms(500); } }Mike
Having tested it, I'm assuming your concern is the printed garbage value of
k? Increasing the size of Cog()'s stack appears to fix it for me.Mike,
I got it to appear to work at least, without increasing Cog()'s stack size, by replacing cogstart() with __builtin_cogstart(). Dunno if I'd trust it though.
EDIT: Err, scratch that. I'd shuffled the placement of Stack[] at the same time.
@evanh , Right. I figured it out.
Mike
@evanh : Ideally we would convert all of the public disk_* functions in sdmm.cc to call through a jump table of some kind so that we could layer FatFS on top of different devices, kind of like what the the littlefs file system does. (That's basically what @Wuerfel_21 already suggested.) For now FatFS can only sit on top of one device at a time, but you could use a define to select between them. Move the old sdmm.cc to sdmm_old.cc, and create a new sdmm_new.cc. Then make sdmm.cc look like:
and at compile time add
-D_NEW_SDCARDto the command line to test your new code.@iseries : Did you figure out why the variable was being printed incorrectly? Having a bigger stack often solves cog issues, and printf is really bad for needing a lot of stack.
Thanks, that'll do. And a lot easier on my brain.
throws an error:
Why? This worked in version 6.5 but doesn't in 6.8.0, now.
Eric changed how bools work. They're more spec-compliant now (all truthy values coerce to 1), but the implementation's kinda scuffed.
Anyways, I think (not confirmed yet) I figured out the aforementioned "SD driver doesn't for work for Chip" issue. Completely unrelated to the actual SD driver of course. Inexplicably, he doesn't have a normal P2EVAL USB breakout with the protection IC, he just hotwired some connector to the P2, which means he doesn't have the power enable pin. So, spot the issue in the USB driver:
' Handle Port protection enable and startup delay cmp usb_enable_pin, #0 wc if_ae drvl usb_enable_pin ' disable port waitx _21ms_ ' Wait a while for everything to turn off if_ae drvh usb_enable_pin ' Enable the port waitx _21ms_ ' Hold to let the idle state get settled(of course the update has been made available in all the usual places)
@ManAtWork : Ada's right, when I updated the
booltype to work according to the C spec (it's quite a strange type compared to the others) I missed some places that needed changing. I've pushed an update to the source code to fix this, and also to make the storage format for C booleans 1 byte again. BASIC booleans are also 1 byte on the platforms that can handle loading signed bytes, which now that I've put in some fixes for nucode means everything except the P1 bytecode.Does that mean there's a problem with
signed charin bytecode C, too? That should really work. Though idk, that'd involve messing with,BCCompileMemOpExExwhich is the sort of code that should probably be completely trashed and rewritten.