Prefix SD SPI pins (SCK, MISO, MOSI, SS)

[rhapsodyv]

Description

Marlin born with AVR, that have only one HW SPI. So, the name chose for the SD SPI pins have no prefix. It lead to some confusion and don't make sense on boards with multiple SPI, and when we support dozens of different SPI devices.

As others SPI pins (TFT, touch, etc), this PR add prefix the the SD SPI pins. This is part of the on going effort to modernise and fix marlin spi code.

Benefits

More clear usage of the SD SPI pins.

Make clear where SD SPI pins are being used with another devices....

Related Issues

[thinkyhead]

For much of the long history of Marlin the SPI bus for your LCD and your SD Card were shared, so I assume they shared the same pins and speed settings. Are they well separated now, so that we can be sure these SPI settings only pertain to SD? Are these SPI settings pertinent at all when SDIO_SUPPORT is in use?

[rhapsodyv]

For much of the long history of Marlin the SPI bus for your LCD and your SD Card were shared, so I assume they shared the same pins and speed settings.

Yes and no. Some (minor, in fact) code reconfigure the spi before use. The sd code always reconfigure the spi for the sd settings before the use.

Are they well separated now, so that we can be sure these SPI settings only pertain to SD?

Yes, but not yet. The idea is that each peripheral define your own spi pins and settings. All settings separated from each other.

But to avoid breaking everything, I'm trying to do these changes in "isolated" tasks.

For example: this renaming will give us a better understanding for what is relying on SD spi settings to work.

Are these SPI settings pertinent at all when SDIO_SUPPORT is in use?

Yes and no again hahaha. Currently, some boards define another spi port on pins when using sdio, they normally set the spi of the exp ports, so the spi can be used with a lcd.
In the near future, the idea is that the board define what it has, and the user chose what he will use.
For example: if I want a SD onboard at same time of a SD on my LCD, the board should define the two configs (if it can) and not exclude one or the other.

Even if lcd or tft or whatever share the same spi pins with SD, having this prefix will be necessary for board that don't share. And make everything more clear.

I changed the SD_SPI_SPEED to advanced config too, because it should not be something "common" to change. As board already does with SDIO, soon they will provide their "supported" SD spi speed and sd spi data mode.

[rhapsodyv]

I didn't changed the lcd or the others places using SD spi pins/speed because this is a planned task: I'm reviewing all SPI usages.

I don't know if worth start part of this task now, as the issue is not only speed, but data mode, port, etc...

[rhapsodyv]

The objective is exactly the opposite... remove “generic” and “un-prefixed” spi configs... not add one more...

[thinkyhead]

Things inside a .cpp file are distinct from the rest of the codebase, and this fits what is going on here, while also following a convention.

In general, calling the first exposed SPI port on all boards the "SD SPI Port" is slightly problematic, because it doesn't really apply as an "SD port" unless SDSUPPORT is enabled. I expect the vast majority of users will be using this SPI for SD connection primarily, but I'm hesitant to label it as "the SD port."

[rhapsodyv]

We aren't calling the first spi port as SD spi... we are making the board set the right names for what they mean to.

There's no such a thing as "THE" SPI. This is only valid for old avr that we have only one single spi...

Each device should define its own spi config and pins, and the SD too.

This un-prefixed pins is one of the reasons that all over the code it's wrongly used.

[rhapsodyv]

@thinkyhead Re-reading your comments, I think here began the misunderstanding of this PR. You said this:

In general, calling the first exposed SPI port on all boards the "SD SPI Port" is slightly problematic, because it doesn't really apply as an "SD port" unless SDSUPPORT is enabled.

But this is not true. The boards only set SPI pins for specific usages. If you look carefully at the board pins, lots of board doesn't even define SCK/MOSI/MISO when SD is not SPI (lcd or onboard).

If some DOGM (or another) are sharing SPI configs with SD, this is an error that we are starting to fix!!

Random boards that doesn't declare MISO/MOSI/SCK pins when the SD is not SPI: pins_BTT_GTR_V1_0.h, pins_MKS_ROBIN_NANO_V2.h (doesn't declare it at all, because the SD is SDIO), pins_BTT_SKR_common.h (all SKR)...

It's more than clear the (MOSI/MISO/SCK/SS)_PIN are 100% related with SD.

[rhapsodyv]

We have now:
TFT:

  #define TFT_CS_PIN                        PD11
  #define TFT_SCK_PIN                       PA5
  #define TFT_MISO_PIN                      PA6
  #define TFT_MOSI_PIN                      PA7

TOUCH:

  #define TOUCH_CS_PIN                      PE14  // SPI1_NSS
  #define TOUCH_SCK_PIN                     PA5   // SPI1_SCK
  #define TOUCH_MISO_PIN                    PA6   // SPI1_MISO
  #define TOUCH_MOSI_PIN                    PA7   // SPI1_MOSI

SPI_FLASH:

  #define W25QXX_CS_PIN                     PB12
  #define W25QXX_MOSI_PIN                   PC3
  #define W25QXX_MISO_PIN                   PC2
  #define W25QXX_SCK_PIN                    PB13

All following the same standard DEVICE_(CS/MOSI/MISO/SCK)_PIN. This was done for one reason: these devices don't share the same SPI pins, config or whenever with each other....

Why SD would be different?? Why it is called only (CS/MOSI/MISO/SCK)_PIN when we have each time more and more SPI devices?!

[thinkyhead]

The handling of SPI pins for LCD devices has evolved, but there is still the need for a 'generic' first SPI port. It seems to me it would be preferable to add this to pins.h …

#if ENABLED(SDSUPPORT)
  #define SD_MISO_PIN MISO_PIN
  #define SD_MOSI_PIN MOSI_PIN
  #define SD_CLK_PIN CLK_PIN
  #define SD_SS_PIN SS_PIN
#endif

… and then only use the SD-specific pin names when the SD device connected to this bus is being specifically referenced in the code.

[rhapsodyv]

So it’s better the other way around. Board define the devices spi.
And we fallback to the generic as SD.

The board don’t define a generic spi. It’s only exist on avr. Board define how devices uses some of their spi. Lcd spi is the spi of the exp port. Sd is the spi for onboard sd, or the same as the lcd spi if using a sd on lcd.

The marlin code must not use a generic spi because it don’t make sense. All the spi usages is an device using it, with its own configs.

That “global” spi must be removed in sometime. It’s prone and lead to error.
This is of the reasons that most of marlin spi usages are flawed.

[rhapsodyv]

Worst yet: if a common 32 bit board would define a “global/generic/default” SPI, it would be the SPI on the exp slot, because it is intended to expansion, not the hard wired spi on the SD slot...

[rhapsodyv]

One more case: when supporting multiple medias at same time, one may want to use an onboard SD + a LCD SD. So, we will need the definitions for two SD SPI, not only one, at same time.

#if HAS_ONBOARD_SD
  #define SD_ONBOARD_MISO_PIN 1
  #define SD_ONBOARD_MOSI_PIN 2
  #define SD_ONBOARD_CLK_PIN 3
  #define SD_ONBOARD_SS_PIN 4
#endif

#if HAS_LCD_SD
  #define SD_LCD_MISO_PIN 5
  #define SD_LCD_MOSI_PIN 6
  #define SD_LCD_CLK_PIN 7
  #define SD_LCD_SS_PIN 8
#endif