MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.14k stars 19.21k forks source link

[QUESTION] SPI clocks...HAL differences #14685

Closed GMagician closed 5 years ago

GMagician commented 5 years ago

I'm back again with question on SD.... this is what I read in HAL_SPI.h (that should be base for all HALs):

  • Approximate rates : *
    • 0 : 8 - 10 MHz
    • 1 : 4 - 5 MHz
    • 2 : 2 - 2.5 MHz
    • 3 : 1 - 1.25 MHz
    • 4 : 500 - 625 kHz
    • 5 : 250 - 312 kHz
    • 6 : 125 - 156 kHz
    • On AVR, actual speed is F_CPU/2^(1 + index).
    • On other platforms, speed should be in range given above where possible.

but this seems to be disregarded by STM32 (0, 4, 5, 6) and teensy (4, 5, 6) platforms.

Even if SPI devices clock may span through other freqs, shouldn't be used above reported for all boards?

tpruvot commented 5 years ago

To note the STM32 has a dedicated SDIO port for the sdcard. Tested ok here at 20MHz (18MHz default) with a cheap 8GB card given with the printer...

An SD/SDIO/MMC host interface is available, that supports MultiMediaCard System Specification Version 4.2 in three different databus modes: 1-bit (default), 4-bit and 8-bit. The interface allows data transfer at up to 48 MHz in 8-bit mode, and is compliant with SD Memory Card Specifications Version 2.0. The SDIO Card Specification Version 2.0 is also supported with two different databus modes: 1-bit (default) and 4-bit. The current version supports only one SD/SDIO/MMC4.2 card at any one time and a stack of MMC4.1 or previous. In addition to SD/SDIO/MMC, this interface is also fully compliant with the CE-ATA digital protocol Rev1.1.

GMagician commented 5 years ago

@tpruvot that's ok AGCM4 has also a dedicated SD port (onboard). Base clock is 48MHz (24MHz max programmable), but what about such clock used in shared SPI?

tpruvot commented 5 years ago

from the F1 datasheet: SPI clock frequency up to 18 MHz

GMagician commented 5 years ago

I understand f1 can raise spi freq, but since Marlin requires some predefined clock I'm wondering why these are not sadisfied, does this means that every board can use different frequencies?

tpruvot commented 5 years ago

We are talking about that today in #14679 but yep, depends on the board and devices too... if using unshielded wires... their length also

GMagician commented 5 years ago

I'm going to read that. By the way my question is more generic, I know that properly shielded cables may let you to use high freqs, but since .h explicitly say that boards must use defined freqs, here comes my question

tpruvot commented 5 years ago

well, as you can see in current HAL devices ... https://github.com/MarlinFirmware/Marlin/tree/bugfix-2.0.x#current-hals

AVR clocks are limited... compared to all others

GMagician commented 5 years ago

Since new devices may increase freq, maybe correct solution is to remove or change HAL_SPI.h comment. Everyone that will read it may think that these freqs match are mandatory..

tpruvot commented 5 years ago

oh, right its from Marlin/src/HAL/shared/HAL_SPI.h .. missed that

gloomyandy commented 5 years ago

The LPC176x HAL implements the defined clock rates for both hardware and software SPI (or at least it attempts to, I've not checked the actual resultant speeds). The defined range is a little limited and arguably the wrong way around as there is no easy/obvious way to define faster speeds (I suppose you could use negative values, but that's not nice).

tpruvot commented 5 years ago

Maybe this kind of function could be implemented in our shared HAL to set speeds in MHz https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/master/STM32F1/libraries/SPI/src/SPI.cpp#L776 Or to define maximum tested freq, maybe..

GMagician commented 5 years ago

SAMD51 already works passing clock, max supported freq is 24MHz, then really usable depends on what is connected

GMagician commented 5 years ago

@gloomyandy and @tpruvot do you think is a good option to change Marlin/src/HAL/shared/HAL_SPI.h comment to something different?

tpruvot commented 5 years ago

At least these speeds should be in AVR hal... not shared one

gloomyandy commented 5 years ago

I think that either the HALs should support the mechanism as defined, or the mechanism for selecting the speed should be changed (and all HALs should support that new mechanism). Having HALs do different things for something like this seems just wrong to me.

tpruvot commented 5 years ago

its not possible to match speeds from all SoC families.. for example the F1 "top" speed is 18 (or 36MHz), so you cant get a 10MHz...18/2 = 9

gloomyandy commented 5 years ago

Well the header file does say 0 is 8-10MHz so 9MHz would be fine. It also says "On other platforms, speed should be in range given above where possible". Seems to me that a HAL should make a best effort to match the requested speed, probably going low if it can't match the actual range.

It could all be reworked, but honestly I think there are more important things to do in Marlin at the moment and any change to this API will mean making changes to all of the HALs and testing them. But that's just my thoughts on it.

tpruvot commented 5 years ago

I dont see why a 32Bit board should limit it's SDcard speed just because some 8bit AVR cant reach it

gloomyandy commented 5 years ago

The thing is Marlin 2.0 is to some degree all about handling the move from 8bit to 32bit. So if for instance you decide that speed 0 means 10MHz on an 8bit board but means 20MHz on some 32bit board then guess what, you will get a bunch of folks posting issues saying that when they moved from their 8 bit board to a new 32bit the SD card suddenly became flakey even though they are using the same speed setting.... Sure it would be nice to have that high speed access on 32bit boards (though many folks will not be able to use it because of long cables etc.), I just think that at the moment there are more important things to get sorted in Marlin if there is ever going to be a 2.0 release.

Also does Marlin actually need faster SD card access? What for? I doubt if SD reading speed is ever an issue when printing, File transfers via gcode are limited by the protocol used to way below SD card speed. That pretty much leaves file transfers via say USB, on the LPC176x HAL we already use 20MHz SD card access from the USB code, nothing to stop other HALs doing the same.

Anyway as I said above that's my take on it, if anyone wants to go ahead and change things so there is a better API then good luck to them.

GMagician commented 5 years ago

Ok, I'l opt to fulfill default requirements in my AGCM4

github-actions[bot] commented 4 years ago

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.