Marlin: [RFC] Questions about HAL, SPI and Arduino library compatibility.

[marcio-ao]

I have recently been working on integrating UHS3.0 into Marlin (for better USB support over the current UHS2.0 library; and it's more readable, actively maintained code) and this has caused me to dig deeper into how Marlin handles SPI, how to handle multiple SPI devices in Marlin, how the Marlin HAL interacts with Arduino libraries.

As many of you know, I have been involved in two projects: developing a touch UI for Lulzbot as well as adding USB functionality. Both of these devices are native SPI devices and fortunately allow us to get SD cards out of the mix (which, per my understanding, are not well-behaved SPI devices).

When first developing my touch UI code, I came across some routines in the Marlin HAL for accessing the SPI bus, namely spiInit, spiBegin, spiRead, spiSend. I ended up writing my touch UI code to either use bit-banged software SPI or the Marlin HAL layer for the hardware SPI. Everything worked fine until I added UHS3.0 to the mix.

What I found is that UHS3.0, like most Arduino libraries, uses the Arduino SPI class for hardware abstraction. It also uses an interrupt for handling USB requests and as such requires all non-ISR code to use beginTransaction and endTransaction, as this suspends the appropriate interrupts during transfers to avoid interference. Obviously, this presented a problem, since the routines in the Marlin HAL don't respect this convention. I was able to confirm that if I switched my touch UI to use software SPI, everything worked; but if I had my touch UI use the Marlin HAL for hardware SPI, things would crash. I decided to modify my color LCD code to use the Arduino SPI routines, along with beginTransaction andendTransaction and everything worked.

Anyhow, this outcome surprised me a bit, as I expected that since Marlin had SPI stuff in the HAL, it probably was necessary. Yet I now have evidence that using the Arduino SPI library is better, as it does all the work necessary to ensure different libraries play well together, even if some of them use interrupts, which is something the Marlin HAL cannot do.

I also investigated further and found that @teemuatlut TMC stuff is also using the Arduino SPI interface, so there is certainly precedent for doing so. So my question is, why does the Marlin HAL have SPI routines at all? Is it just legacy stuff from the SD card? If so, wouldn't it be best to simply rewrite the SD card stuff to use the SPI library and get rid of the SPI layer in the Marlin HAL? Is there any reason this cannot be done?

Any answers to these questions would be very much appreciated!

-- Marcio

[InsanityAutomation]

My understanding is the initial implementation was intended to break hardware reliance on support in the Arduino and platformio toolstack. However I'm sure others can answer more in-depth and possibly more correctly :)

[teemuatlut]

I think it would be better if all HW SPI would interface through a single implementation. External libraries of course don't have access to Marlin internal code.
A quick search for spiInit shows that it's only called for MAX6675, USB flashdrive and SD cards so best guess is that it sure is all legacy stuff for SD.

[marcio-ao]

My understanding is the initial implementation was intended to break hardware reliance on support in the Arduino and platformio toolstack. However I'm sure others can answer more in-depth and possibly more correctly :)

@InsanityAutomation: This is a topic for another full thread, but I tried to not get into that in this post. But since you mentioned it, I'll make my spiel on that:

I suspect I understand why some folks may think the Arduino library is a bad thing. The reason, at least as I have encountered it, is that the Arduino routines only work well for pins which are wired up in official Arduino boards. For instance, on the Mega 2560, you can use digitalWrite on most pins, but if you need to access PJ4, you are SOL. This is made more frustrating because it works for some of the pins, but not for others, and if you haven't yet learned enough about Marlin, this will frustrate you to no end (as it did frustrate me for the first two years of working with Marlin and not really understanding the subtle difference between Arduino boards and 3D printers boards, which are Arduino-ish).

So, yes, as it stands, the Arduino library is a bit broken, because 3D printer boards often use pins which are inaccessible from the Arduino library. Things work as long as you stick to the subset of pins which would be present on an official Arduino board, but beyond that, it does not.

Despite this, I think there is great value in being able to use Arduino libraries in Marlin. Most of the time, it simply is not feasible to rewrite said libraries to use Marlin HALs. I believe this will become a bigger issue as Marlin starts to support additional touch panels, for which Arduino libraries already exist, and which we will not want to adapt individually.

It seems like the deficit in Arduino libraries is minor. They simply don't have the incentive to add support for all the pins on each microprocessor, but doing so should not be very difficult. Rather than re-inventing the wheel with a incompatible HAL in Marlin, has any thought been put into simply making a special fork of the Arduino libraries for 3D printers? This would be essentially the upstream Arduino libraries, with the following modifications:

• Use Marlin pin numbers, rather than Arduino pin numbers (where there are differences)
• Allow access to all the pins for a given MCU, even when the equivalent Arduino product does not have those pins assigned.
• Maybe improve digitalWrite and digitalRead so that it favors performance over being fool-proof, so it is more like the digitalWriteFast library.
• Patch delay so it properly maintains the heaters.
• Continue to support fastio style routines, for timing critical tasks, like stepper ISR.

Although maintaining such a fork might be a bit more effort than rolling our own HAL, it would have the following advantages:

• Arduino libraries could compiled in and they would behave as expected.
• Marlin would become more readable for folks with a strong background in Arduino.
• We would benefit from all and any work done by the Arduino team.
• The fork would be useful in its own right, as it would allow people to run sketches on 3D printer boards as if they were Arduinos (I often do this already, but I have to limit myself to "compatible" pins, which is a PITA).

Just to clarify: I am not suggesting forking the Arduino IDE and toolchain, or abandoning PlatformIO, for folks who like that. Just forking the library code responsible for SPI, digitalWrite, digitalRead, etc. Just the bits to provide compatibility to Arduino code and that currently are duplicated in the Marlin HAL, using an incompatible API.

Thoughts?

[marcio-ao]

A quick search for spiInit shows that it's only called for MAX6675, ~USB flashdrive~ and SD cards so best guess is that it sure is all legacy stuff for SD.

@teemuatlut: There, I fixed that. Or at least I hope to, with this upgrade to UHS3.0 ;)

[ejtagle]

@marcio-ao : The main problem, as i see it, is that Arduino SPI framework is not general enough.
At some point, trying to target the amount of differing platforms with a unique API would mean no special features of hardware could be used.
I will give you one example where SPI Arduino libraries are lacking: Speed
On 8bit processors, all the digitalPinXXX functions are extremely slow (that is why the fastDigitalXX library appeared... But that library has its own requirements, and thus, its usage could be incompatible with other Arduino libraries.
On SPI and UART, Arduino libraries are not interrupt driven. I agree that if intending to use fast SPI clocks, polling is usually the right choice, but, if using slow clock speeds, sometimes interrupt driven approachs are way better.
On ADC, for example, Arduino uses POLLING to wait for completion on conversions. That is awful, because conversions are slow. That is why Marlin had to roll their own ADC routines.

On 32bit boards, the situation worsens. Most CPUs have DMA capabilities that would make life much easier. But Arduino libraries do not use it at all. Awfull polling on ADC conversions, Software SPI communications that are not optimized at all (that is the reason i wrote SPI routines in Assembler for DUE...) ... No usage of DMA at all...

You see the trend: Generalization makes nearly impossible to take advantage of particular features of each architecture. On 8bit boards, we are nearly using the maximum available amount of CPU... So, quite to the contrary, i will expect the HAL to even grow bigger with optimized implementations of each lib.

And, on the 32bit side, exactly the same will happen. Maybe the usage of a proper RTOS (freeRTOS or something alike) will be the proper move. Reality dictates that the Arduino framework is just not good enough to exploit the platform capabilities to its fullest. It is just an "easy" framework to learn, but that's it.

And, i am sure you will agree that Marlin is, by no means, a "simple" piece of software: Quite to the contrary, it is a quite complex one, with very complex realtime requirements. It is not for the faint of heart to modify and/or add functionality to it.

And, just check the logs of commits: All the people that are actively contributing, most of them are either very good embedded programmers, or very good/excellent c++ programmers... I may say, not your average Arduino user!

[teemuatlut]

The issue here is strictly with Hardware SPI.
AVR Marlin does not make it any faster and as per the comment it might even make it slightly slower.

Marlin HAL_spi_AVR.cpp

  /** SPI send a byte */
  void spiSend(uint8_t b) {
    SPDR = b;
    while (!TEST(SPSR, SPIF)) { /* Intentionally left empty */ }
  }

Arduino AVR SPI

  // Write to the SPI bus (MOSI pin) and also receive (MISO pin)
  inline static uint8_t transfer(uint8_t data) {
    SPDR = data;
    /*
     * The following NOP introduces a small delay that can prevent the wait
     * loop form iterating when running at the maximum speed. This gives
     * about 10% more speed, even if it seems counter-intuitive. At lower
     * speeds it is unnoticed.
     */
    asm volatile("nop");
    while (!(SPSR & _BV(SPIF))) ; // wait
    return SPDR;
  }

[ejtagle]

@teemuatlut : There are ways to make Hardware SPI a little bit faster, but it depends on the exact ratio of main CLK and SPI clk. I did several experiments trying to achieve maximum throughput (to control an SPI TFT LCD) and as far as i remember, it was impossible to get more than 1 byte every 10 clocks on AVR, because 1 extra cycle is required to load the SPDR into the shift register, and an extra cycle is required to load the received result from the SPI shift register to the SPDR.
But that only applies when running SPI at maximum speed. Otherwise, it is mostly pointless.

[Grogyan]

It is my opinion that the SPI library needs to be replaced with something that isn't dependent on Arduino libraries.

Playing with multiple settings to get reliable SPI working on the ReArm is exhausting.
Even with a working set of parameters, and come back the next day, without changing anything, and it stops working.

Noting that the SPI library is also probably the cause of multiple other problems with different displays, and boards.
For example, the Bigtree Facebook group is almost a quarter of, people's boards not working reliably. Which appears to be due to either the SD card they are using, or the SPI interface to the processor.

[marcio-ao]

So it sounds like there are good reasons for the Marlin HAL to exist.

However, is it possible that the HAL could be made in such a way that libraries can continue to use the Arduino SPI class without needing to be rewritten? If at least both could operate side by side, we would have the option to use those libraries in Marlin.

Currently, on the Due platform, I was finding that trying to initialize both Marlin's SPI routines and the Arduino SPI class caused the Arduino SPI code to hang.

[marcio-ao]

And, just check the logs of commits: All the people that are actively contributing, most of them are either very good embedded programmers, or very good/excellent c++ programmers... I may say, not your average Arduino user!

@ejtagle: Well, I am a C++ programmer, and I know Arduino pretty well, but I've spend countless hours trying to get Arduino libraries to work under Marlin. I'm precisely the kind of user who doesn't have enough embedded experience to be able to navigate the complexities on my own and it would be really nice if Marlin made it easier for things to work together :)

Anyhow, I am running into a problem like that right now. The UHS3 library seems to work well on AVR when integrated into Marlin, but it does not work under the Archim 2.0. If I load the library onto a Due, it seems to work fine as well. So, it should work under Marlin on the Archim 2.0, but it does not. My experience tells me that I will probably spend a few weeks of my life before I finally get this to run and it probably will be some odd little thing.

[marcio-ao]

Ok, so this appears to be the problem. If I load the following Arduino sketch into the board:

#include <SPI.h>

void setup() {
  // put your setup code here, to run once:
  SerialUSB.begin(115200);
  delay(3000);
  SerialUSB.println("Hello.");
  pinMode(87, OUTPUT);
  //SPI.begin();
}

void loop() {
  // put your main code here, to run repeatedly:
  delay(100);
  digitalWrite(87, LOW);
  //SPI.beginTransaction(SPISettings(14000000, MSBFIRST, SPI_MODE0));
  //SPI.transfer(0x55);
  //SPI.endTransaction();
  delay(100);
  digitalWrite(87, HIGH);
  delay(100);
  SerialUSB.println("Toggling pin");
}

I see the CS pin toggling. But as soon as I uncomment SPI.begin(). The CS pin no longer toggles. What is going on here?

Incidentally, this seems to be what is happening when the USB is part of Marlin too. The CS pin seems not to toggle.

[ejtagle]

I´d suspect the SPI library taking control of the CS pin, probably because it is using it in hardware control mode. You should check the Arduino SPI library source code... Probably they are setting the hardware SPI device to control it directly, and when that happens, you lose software control of the pin...

[boelle]

should this be keept open?

[turtiustrek]

What would one use then? Is it better off writing you're own routines or using the HAL SPI interface.

[boelle]

its more if this is a bug or question, @thinkyhead what is your take on it?

[marcio-ao]

@boelle: I'll close this, as I no longer need an answer.

[turtiustrek]

@marcio-ao So what did you settle on?

[marcio-ao]

I am using Arduino SPI for all my devices, rather than the Marlin HAL. This is consistent with what the TMC libraries do, for instance. Mixing and matching both styles of routines seems to lead to problems.

[turtiustrek]

@marcio-ao did you face any issues marlin overriding 'beginTransaction'(which crashes the interface)?

[marcio-ao]

@jack001214: Where does Marlin override 'beginTransaction'? I have not had any issues. That said, we only use SD cards on 8-bit AVR boards. It is possible that you could see an issue if you were using a 32-bit card with the SD card. We have not tested that combination.

[turtiustrek]

@marcio-ao Yes i am running marlin 2.0 on Esp32 which is not friendly to the SD Card and i have a hard time calling the SPI functions for use, before something "disables" it.

[marcio-ao]

Yes i am running marlin 2.0 on Esp32 which is not friendly to the SD Card and i have a hard time calling the SPI functions for use, before something "disables" it.

@jack001214: I'm not too surprised by this. I think this is a serious enough issue that warrants its own separate ticket.

[turtiustrek]

@marcio-ao anyways, do you intend on running UHS3.0 on other platforms?

[marcio-ao]

@marcio-ao: On 32-bit, we are still using UHS2.0 because UHS3.0 uses interrupts which interferes with servo motors, which we use on our TAZ Pro. We have been working with the developer of UHS3.0 (@xxxajk) to try and address the issue, but it is still not fully addressed.

For the moment, we find UHS2.0 is the best bet for 32-bit systems, but it will not work on 8-bit systems.

A modified version of UHS3.0 works on 8-bit systems, so long as you use an interrupt-capable pin for SD_DETECT/USB_INT, but it consumes almost all available memory, making the machine potentially unstable.

[antfurn]

@marcio-ao This is maybe a bit off-topic, but on the TAZ Pro are you using the on-board SD card reader of archim2 or an external one? I've not been able to get the on-board to work with marlinv2 :-(

[marcio-ao]

@antfurn: Neither. The TAZ Pro uses an external USB host shield, using the UHS2.0 library.

[boelle]

not sure if it helps but i have changed the sd card settings

// LPC-based boards have on-board SD Card options. Override here or defaults apply.
#ifdef TARGET_LPC1768
#define LPC_SD_LCD // Use the SD drive in the external LCD controller.
#define USB_SD_ONBOARD // Provide the onboard SD card to the host as a USB mass storage device.
//#define LPC_SD_ONBOARD // Use the SD drive on the control board. (No SD_DETECT_PIN. M21 to init.)
//#define LPC_SD_CUSTOM_CABLE // Use a custom cable to access the SD (as defined in a pins file).
//#define USB_SD_DISABLED // Disable SD Card access over USB (for security).
#if ENABLED(LPC_SD_ONBOARD)
//#define USB_SD_ONBOARD // Provide the onboard SD card to the host as a USB mass storage device.
#endif
#endif

i use the re-arm and i want the internal sd card to be availble for my pc or octoprint so i can update firmware remotely if i want, since i got a raspberry pi 3 i stopped using the sd in the lcd, but with this config the sd in lcd is still free

[antfurn]

OK, good to know. Was starting to wonder if anybody had got it working at all.

On the archim, although it's a 32bit board - unlike re-arm, it doesn't use the SD card for it's firmware. Basically the archim is a variation of the arduino Due using the SAM HAL. In theory the on-board reader should work just like an external SPI reader - but looks like there are some major issues with SPI SD card and marlinv2 if I'm reading this thread correctly - shame.

[turtiustrek]

@antfurn i am having issues on a ESP32 which does not use SD for the firmware...

[antfurn]

I'm still having to do a lot of learning about how marlinv2 and the archim2 work together. I managed to customise marlinv1 quite a bit for my AO101 many many years ago. But the HAL changes in v2 are still taking a lot of time to get my head around.

Just reading this issue again. It looks like I need to change to Arduino SPI to have any chance for getting the on-board SD carder to work. Any pointers would be gratefully received :-)

FYI: I'm using a basic I2C OLED screen and there don't have any external SPI modules attached (so far).

[turtiustrek]

@antfurn did you look at the HAL_spi_pins.h file? it has pin definitions but different for each processor.

[xxxajk]

Take a peek at a UHS3, sdcard is simple to do. I managed to implement and
merge it into UHS3 in a day. There's sample code and documentation all over
the net.

On Mon, Jul 22, 2019, 1:24 PM jack001214 [e-mail] wrote:

@antfurn https://github.com/antfurn did you look at the HAL_spi_pins.h
file?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MarlinFirmware/Marlin/issues/14197?email_source=notifications&email_token=AA5SJTGRFY7NXACTI6IWBO3QAXULHA5CNFSM4HR3SXBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2QSAZY#issuecomment-513876071,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5SJTCZ3KLEDNQ77YGVVT3QAXULHANCNFSM4HR3SXBA
.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.