Multiple, simultaneous, SPI buses handling and Smart SD mounting

[LinoBarreca]

We will use this place to have a feedback from testers.
I am reopening this because last one was closed by github as I rebased the changes.

SKR-PRO ONLY (other boards are broken at the moment, don't waste your time trying to build it)

How to test:
Take a clean folder.
Pull the sources.
Add your config files.
Compile.
Put the firmware in the onboard sd.
Turn on the printer.
Let it flash the firmware.
Wait 20 seconds, then plug into an USB port and on the computer open serial monitor.

What will you gain with this PR:

1. you can access the onboard sd card (it wasn't possible before)
2. you can use SPI3 port even if you are using drivers in SPI mode (it wasn't possible before)
3. (future) you can have more than one card and more than one display

SD? great. How does it work?

• On-board SD is on bus 0. There's a detect switch: card in? Marlin sees it. Card out? Marlin doesn't.
• Screen SD is on bus 1. There's a detect switch: card in? Marlin sees it. Card out? Marlin doesn't. If the opposite happens flip the SD_DETECTED_INVERTED, in configuration.h
• External SD can be attached to bus 2. There isn't a detect switch in that port. if Marlin searches the card in there it will always find it and try to initialize it. If you don't use it, remove the probing of SD in bus 2. how? #define SD_SEARCH_ORDER { 1, 0 } in configuration.h

What will it happen when I flash it?
It depends on your current situation.

• If you have a display with an SD card slot and that slot contains a card you will not see any change. You will be able to access your files. Like you did without this. If you pull out the SD card from the display (unmount it from menu, before) and ask to "mount card" again you should see (magically) the files in the on-board SD.
• If your display doesn't have a card slot or the slot is empty you will see the files in your on board sd card.... and you will be able to save/restore EEPROM to/from it

Things which are known to be NOT WORKING (will be addressed later). In order of priority:

• Hardware CRC on SPI bus. DONE! New feature: makes the program smaller and talking with the card faster..by the way...did you know that the on-board sd card is blazing fast? (double the speed of the LCD card) if your card doesn't work properly lower the SPI SPEED by #define SPI_SPEED SPI_HALF_SPEED in Configuration.h
• Actually SD-card calls are not using transactions. When this will be complete the definition of the device parameters (speed/msb/polarity etc) will be "per device" (you shouldn't care anyway...it will be internal in marlin). Actually only the speed is still to be handled in pins file. Partial support added.
• SPI drivers when TMC_USE_SW_SPI is undefined (that's because tmcstepper library might need an update to support more than one bus, like we are doing here) if we address them by pins through marlin, however with "TMC_USE_SW_SPI", everything works fine. so, at the moment #define TMC_USE_SW_SPI
• DMA!!!!
• Multiple cards at once. Actually Marlin can only handle one card at once. Since we can now have N working all together, we need to modify some commands to accept a "card number". That's maybe why
• ESP8266 can't upload files to the LCD card.
• Messages in Chittagonian. English only at the moment, sorry. We need to extract new strings into resource files and localize them.
• User stupidity checks. Actually if you fuck up some configuration on the new features you get no warning from SanityCheck. We will add them later... anyway if you are here you are an expert which reads the instructions and knows what he's doing...by the way DO NOT FU**ING EDIT PINS file unless you are told so. 😆 There are other ways to do what you need in configuration/configuration_adv.h
• Other boards. This required a modification on the HAL interface with the core. it means that to make this working other hals must accept the calls and route them properly to their inner methods.

[PGrave]

Reporting:
"Recv: SPI 1 State: 1, Error: 0
Recv: echo:Receive
Recv: SPI 1 State: 1, Error: 0
Recv: Before init: frequency=42000000SPI 1 State: 0, Error: 0
Recv: echo:After init
Recv: SPI 1 State: 1, Error: 0
Recv: echo:Receive"

[LinoBarreca]

nothing else?

[PGrave]

Nothing else, flooded octoprint serial monitor with this lines, repeating. The only different lines where those mentioning frequency.

[LinoBarreca]

take out the SD card from the screen.
(and reset the board)

[PGrave]

SD Card from screen: out

[LinoBarreca]

was it out before?

[PGrave]

No, it was in

[LinoBarreca]

okay. since you are using octoprint you can disconnect and reconnect (through octoprint)
it should reset the board and probe on SPI 0

[PGrave]

Ok, will do

[PGrave]

Recv: SPI 1 State: 1, Error: 0
Recv: echo:Receive
Recv: SPI 1 State: 1, Error: 0
Recv: Before init: frequency=42000000SPI 1 State: 0, Error: 0
Recv: echo:After init
Recv: SPI 1 State: 1, Error: 0
Recv: echo:Receive

[PGrave]

(buffer full)

[LinoBarreca]

Is the display working?

[PGrave]

Both displays are working, visually at least. Shall I issue some commands? I' noticed some changes in the pins file, drivers section

[PGrave]

Axis seem to work fine, issuing commands from TFT35 display. I'll try the Ender display now

[PGrave]

This message from SPI 1 is just repeating continuously, but I'm almost sure I've seen SPI 2 related lines and an "SD card found"

[PGrave]

I'm recording a serial log from octoprint, maybe that can give further light

[LinoBarreca]

Okay. I am not at the computer. The buffer is flooded because most likely you have a spi display and the calls are dumped. I need to filter those out but it's gonna be tomorrow. Meanwhile can you search for "spi 0" in the buffer (at the very start) or do a connect and disconnect after a couple of seconds so you preserve lines at the start.

[PGrave]

Ok, understood, I'll try. But I've never seen any related SPI 0 messages, just 1 and 2. I'll check also the log for info.

[LinoBarreca]

They should be at the very first start. When marlin boots.

[PGrave]

Ok, I've got logs with more info.
serial(1).log
Serial_Edited.txt

[LinoBarreca]

I understood :D

[Patag]

Hi @LinoBarreca . I'm definitively unable to find out how to use STlink to upload the firmware.
Can help me ?

[LinoBarreca]

@Patag good morning. Yes I can, as I said you need to specify the upload address to do the upload. I don't remember it because I don't have the board but it should be around 0x8000 if I remember correctly. to check (before doing damages) just take a firmware.cur from the card and search the first binary bytes (let's say 10 bytes) in the memory chip. the starting point is the upload address. then go to platformio guide to see how to specify it.

[LinoBarreca]

@Evg33 @Patag https://community.platformio.org/t/jlink-uploading-to-incorrect-address/6085/3

I think that option only applies to jlink. use a custom command.
https://docs.platformio.org/en/latest/projectconf/section_env_upload.html#upload-command
there's the example right for stm32

Opening a bug to platformio. It seems that they knew they were uploading it in the wrong place but they modified only jtag and not stlink...also.. I strongly believe the user shouldn't care about forcing the address...the default configuration should work out of the box without "destroying" the hardware.

[LinoBarreca]

@Evg33 & @Patag analyzing the code upload.offset_address=0x08000000 should make it work.

EDIT: analized openocd. The address (so the above option) is used only when you flash a bin. when you flash the ELF it should analyze the elf and program it as expected.
I have no clue why it messes up everything...but just to be sure use the custom command to flash the bin at the correct address

[Patag]

Hi @LinoBarreca and @Evg33
The address to upload Marlin firmware on SKR Pro is always 0x8008000. The bootloader has been reserved in the first 0x8000 bytes for its own usage. This can't be found in any file (elf or bin), so you have to force it in the upload command. In fact, the bootloader is always launch at reset, whatever there is a card or not, a firmware.bin file or not, then it "jumps" to Marlin according the vector table located at 0x8008000. This is why we can see -DVECT_TAB_OFFSET=0x8000 option in platformio.ini, and explains why the debugger is able to manage this.
You can test it with STLink Utility. I'm still trying to integrate it in Platforimo without success but it's another thing....

[Patag]

Ok
I added
upload_command = ST-LINK_CLI -P $BUILD_DIR/firmware.bin 0x8008000 -Rst
in platformio.ini.

[Patag]

Hi @LinoBarreca
I just found out why I've never been able to activate TMC_USE_SW_SPI !!!
My configuration is Dual Z stepper and is not manage by the current pins_BTT_SKR_PRO_V1_1.h. So I added

#if AXIS_HAS_SPI(Z2)
  {DEVTYPE_DRIVER ,   2,      NC,      NC,      NC, SPI_FULL_SPEED,     PG15, DRIVER_AXIS    , 12}, //Index 12 is Z2
#endif

Not tested with actual stepper, but it makes the trick at least for booting. According to my debugger session, it was previously catch by a unexpected interruption handler coming from TMC5160 init and pointing to an infinite loop.
I guess it will have to be extended to all other multiple steppers configurations.

Next step, I'm still not able to use my TFT35V3 to get my LCD SD working.

[LinoBarreca]

Hi @LinoBarreca
I just found out why I've never been able to activate TMC_USE_SW_SPI !!!
My configuration is Dual Z stepper and is not manage by the current pins_BTT_SKR_PRO_V1_1.h. So I added

#if AXIS_HAS_SPI(Z2)
  {DEVTYPE_DRIVER ,   2,      NC,      NC,      NC, SPI_FULL_SPEED,     PG15, DRIVER_AXIS    , 12}, //Index 12 is Z2
#endif

Not tested with actual stepper, but it makes the trick at least for booting. According to my debugger session, it was previously catch by a unexpected interruption handler coming from TMC5160 init and pointing to an infinite loop.
I guess it will have to be extended to all other multiple steppers configurations.

Next step, I'm still not able to use my TFT35V3 to get my LCD SD working.

Nice catch bro!
Since it's your merit, wanna make a pull request?

[LinoBarreca]

@Patag... wait... there's something which doesn't make sense.
PG15 is for E1 is your Z2 using E1? I think this should be handled differently.

[Patag]

Marlin automatically uses E1 for Z2 when you set a dualZ stepper config.

[LinoBarreca]

then it should use the already defined E1...no? gotta troubleshoot it better

[Patag]

Don't think so, because it has to swap from DRIVER_EXTRUDER to DRIVER_AXIS in this case.

[Patag]

And for the PR, I let you make it for more consistency. I'm too old for this kind of glory ;-)

[LinoBarreca]

you are right. Actual logic needs one more logic layer of separation.
We gotta differentiate the drivers (correctly attached to the SPI) from the usage that marlin does of them
Something like
"for X use driver 1, for y use driver 0, for Z use driver 2, for E0 use driver 3, for Z2 use driver 4...." and so on

[LinoBarreca]

but now I've run into another problem.
Before making the PR to marlin everything was working... now, after their merge, eeprom doesn't work anymore...they just merged something broken. and now we are broken too.

[Patag]

And I was wrong. It's not E1 but E2 used for Z2 in my conf Dual Z / 2 extruders (tested with a stepper with official Marlin branch). So my change becomes

#if AXIS_HAS_SPI(Z2)
  {DEVTYPE_DRIVER ,   2,      NC,      NC,      NC, SPI_FULL_SPEED,     PG12, DRIVER_AXIS    , 12}, //Index 12 is Z2
#endif

and tested with your branch, it works. We definitely have to find out the Marlin logic when affecting an axis to a stepper....

[LinoBarreca]

@Patag at the moment I'm trying to un-fuck-up what Scott did to my code. I created a "branch" MultiSPImio which is MultiSPI BEFORE his changes.

[Patag]

OK, let me know if you need some more tests

[LinoBarreca]

I am pausing all the development on this until saturday 28.
I wanna enjoy christmas without banging my head on this. I will return on this on saturday or, more likely on monday.
Merry christmas you all.

[Patag]

Merry Christmas and happy new year eve also !

[Patag]

you are right. Actual logic needs one more logic layer of separation.
We gotta differentiate the drivers (correctly attached to the SPI) from the usage that marlin does of them
Something like
"for X use driver 1, for y use driver 0, for Z use driver 2, for E0 use driver 3, for Z2 use driver 4...." and so on

Hi @LinoBarreca I think I've found out the Marlin multi stepper logic. I just reverse engineered the pin.h file where it's described. Not a priority but it will help for the next steps.
pins_BTT_SKR_PRO_V1_1.txt

[LinoBarreca]

Hi @LinoBarreca I think I've found out the Marlin multi stepper logic. I just reverse engineered the pin.h file where it's described. Not a priority but it will help for the next steps.
pins_BTT_SKR_PRO_V1_1.txt

Hi @Patag . It can't be done this way 'cause E##p##_##q##_PIN will be undefined since it's defined in the array.

[Patag]

Humm not sure to understand. E##p##_##q##_PIN is defined outside of the SPI_Devices array ?! (or it's my decreasing view). It is used to determine X2_CS_PIN, Y2_CS_PIN...Z3_CS_PIN that are used later in the array. It has been successfully tested on X/X2/Y/Z/E0/E1, X/X2/Y/Y2/Z/E0, X/Y/Z/Z2/E0/E1 and X/Y/Z/Z2/Z3/E0 configurations with real steppers.

BTW, I'm not able to compile your latest commit :

Compiling .pio\build\BIGTREE_SKR_PRO\src\src\sd\Sd2Card.cpp.o
Marlin\src\sd\Sd2Card.cpp: In member function 'bool Sd2Card::readData(uint8_t*, uint16_t)':
Marlin\src\sd\Sd2Card.cpp:473:35: warning: right operand of comma operator has no effect [-Wunused-value]
       spiRecDevice(dev_num), dst, count);
                                   ^~~~~
Marlin\src\sd\Sd2Card.cpp:473:40: error: expected ';' before ')' token
       spiRecDevice(dev_num), dst, count);

[LinoBarreca]

@Patag let me explain better. point of this is to avoid use of "hardcoded" X2* X3*, E2*, E1* and so on.
so you should have in configuration.adv something like

#define NumAxis 3 //my printer has 3 axis (optional, we could leave it dynamic)

const int AxisSteppers[NumAxis][] = {{1,2}, {4}, {5,7}} //axis 0 uses steppers 1 and 2, axis 1 uses stepper 4, axis 2 uses stepper 5 and 7. what does it mean? dual x, single y, dual z
const int ExtrudersSteppers[] = { 3, 6, 8} //three extruders, use steppers 3, 6 and 8

I'm not able to compile your latest commit

I fixed it.

[Patag]

OK, I definitely don't have the "big picture" of what you want to achieve...

[LinoBarreca]

@Patag it isn't about the big picture. I just want this to be closed as soon as possible but it's logical that this new layout can (and sooner or later will) led to multiple axis, multiple cards, multiple devices.
this would allow marlin to run on any kind of machine (from printer to CNCs) and implement, in future a real non planar 3d printing (for example adding another axis which turns clockwise, anti-clockwise the extruder).

Actually it's only X, Y, Z and some "non-planar" 3d printing is achieved by changing Z
(for some info look here)

[LinoBarreca]

This can't be found in any file (elf or bin), so you have to force it in the upload command.

Not really.
The elf file already contains correct regions (look at buildroot\share\PlatformIO\variants\BIGTREE_SKR_PRO_1v1\ldscript.ld)
You can also see it from a verbose build:

MethodWrapper(["checkprogsize"], [".pio\build\BIGTREE_SKR_PRO\firmware.elf"])
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
DATA:    [=         ]  14.9% (used 29216 bytes from 196608 bytes)
PROGRAM: [==        ]  23.0% (used 240732 bytes from 1048576 bytes)
.pio\build\BIGTREE_SKR_PRO\firmware.elf  :

section                 size        addr
.isr_vector              392   134250496
.text                 216712   134250888
.rodata                22952   134467600
.ARM                       8   134490552
.init_array               84   134490560
.fini_array               20   134490644
.data                   1068   536870912
.ccmram                    0   268435456
.bss                   28148   536871980
._user_heap_stack       1536   536900128
.ARM.attributes           48           0
.debug_info         17175852           0
.debug_abbrev         544413           0
.debug_aranges         28336           0
.debug_line           744117           0
.debug_str            288802           0
.comment                 126           0
.debug_ranges          73816           0
.debug_frame           85960           0
.debug_loc            374382           0

Total               19586772
<lambda>(["upload"], [".pio\build\BIGTREE_SKR_PRO\firmware.elf"])
AVAILABLE: dfu, jlink, stlink
CURRENT: upload_protocol = stlink

addr 134250496 = 0x8008000 (in the .isr vector)
problem is that the default upload in openocd doesn't take it into account when uploading but it does when (uploading and) debugging

upload_command = ST-LINK_CLI -P $BUILD_DIR/firmware.bin 0x8008000 -Rst

the correct upload command should be
upload_command = "$PROJECT_PACKAGES_DIR/tool-stm32duino/stlink/ST-LINK_CLI.exe" -c SWD -P "$BUILD_DIR/firmware.bin" 0x8008000 -Rst -Run

but it won't still work because at the "upload stage" you still don't have the .bin but just the .elf

[Patag]

Nope, can't be agreed with this.
ISR vectors (Stack Pointer and Program Counter start address, ...) are always located at 0X8000000 according to ST specs and elf just tells it was moved elsewhere but not where the code has to be uploaded to.. BTT decided to reserve the first 32Kb for it's proprietary(?) boot-loader. As far I understand, after RST, CPU start-up on the boot-loader code which test firmware.bin presence, if so, copies it at 0x8008000, and in the end (whatever the presence of a new file to be copied) loads SP and PC with values found at 0x8008000 (so jumps actually to Marlin).
This is why I bricked my board after downloading Marlin to 0x8000000 (BTW, thanks again for your trick).
The debugger is not so smart and jumps also on boot-loader, but shows you the first address where it finds the symbols in elf.

but it won't still work because at the "upload stage" you still don't have the .bin but just the .elf

Same here. I've to build then upload to make it working