Marlin: SPI Pins for SD on AT90USB1286 uses Teensy pin mapping instead of marlin pin mapping

[StephS]

I found the issue while adding support for the Printrboard Rev F. The pin mapping for the SDSS utilizes the Teensy++ pin mapping, while the rest of the pins def uses arduino/marlin pin mapping. this creates confusion as pin 20 is used for bed heater in arduino, and pin 20 is used for SDSS in Teensy. I have tested this by setting the pins in my own branch and have working SD and heater.
See Sd2PinMap.h section for AT90USB1286
working printrboard rev F pins file on my branch: https://github.com/StephS/Marlin

[Roxy-3D]

I have a PrintRboard! This is helpful for me.

[StephS]

Why is sd2pinmap used? Especially in this use case it messes things up.

[thinkyhead]

Why is sd2pinmap used? Especially in this use case it messes things up.

I came late to this party, so I don't know why this was originally so. Perhaps if you dig into the commit history you can find the original author. Seems the same thing is true for some other pins. For example, in pins_5DPRINT.h we find:

// Microstepping pins
// Note that the pin mapping is not from fastio.h
// See Sd2PinMap.h for the pin configurations
.  .  .
#define X_MS1_PIN 25
#define X_MS2_PIN 26
#define Y_MS1_PIN 9
#define Y_MS2_PIN 8
#define Z_MS1_PIN 7
#define Z_MS2_PIN 6
#define E0_MS1_PIN 5
#define E0_MS2_PIN 4

So… Now that we've integrated support for this board, have we got everything correct? If so, can we close this issue?

[StephS]

well, support still isn't fully integrated considering that the Printrboard and Teensylu use the sd2pinmap which utilizes teensyduino pin mapping, and the board pin config utilizes marlin pin mapping. Ideally it should be the same across the board, so we need to determine why it exists in the first place, then make the necessary changes (if even possible) to use the same pin mapping across the board. as-is the printrboard uses 2 different pin mappings in the same pin config file, which is a bad idea.
See in pins_PRINTRBOARD_REVF.h

37 #define HEATER_BED_PIN     20
64 #define SDSS               20 // Teensylu pin mapping

it appears that sd2pinmap duplicates the pin mapping for whatever reason, when it should just use whatever pin mapping the rest of the code uses. In fact I probably have the other SPI pins wrong in the pins_printrboard_revf, considering that they are only use if SD support is disabled (if enabled, SD2PinMap sets)
I don't even understand why there are two SS pins defined.
Ether SDSS in pins_printrboard_revf (line 64) or SS_PIN in sd2pinmap (line 226)

[thinkyhead]

This is still on my radar – just busy with other issues. There's also a lot of pins labeled "ANALOG PIN MAPPING" because they follow their own scheme too. I presume there's some way to still get the same high performance of Marlin, but to refer to pins in some better way. I haven't delved into where these pin numbers come from, but perhaps the pin numbers we're using now can be replaced by pin names defined in headers for the hardware….

[Blue-Marlin]

As far i can see the root cause for the different mapping here are the differences between fastio.h and Sd2PinMap.h. The one does an extra mapping for Teensy (on request) the other does not.

To solve the problem i suggest to eliminate Sd2PinMap.h. In SD2Card.cpp there are about 7 calls to fastDigitalWrite() and one to fastDigitalRead(). If these can be replaced by the normal WRITE()/READ() from fastio.h, ...

[jbrazio]

@Blue-Marlin will you submit a PR implementing what you have suggested ?

[Blue-Marlin]

@jbrazio
I'll try. But likely it will last until mid of next week until i can begin with this. Alone reading, understanding and investigating today's Marlin mail will last two days.

[thinkyhead]

I wish I was more Arduino-savvy in general to make sense of pins and things. Can anyone recommend a good link on the subject (that is perhaps clearer or more helpful than Arduino's own documentation)?

[Blue-Marlin]

@thinkyhead
A short summary:

The traditional way to program pins of an AVR is to decide on a register, look up the processor-pin-number in the processor's data sheet, then look up the board connector connected to the processor-pin in the board's description. Or the other way around.

One of the design targets of Arduino was to simplify that. They printed nice pin-numbers on their board's connectors and put a translator from the connector number to the registers for each of their boards (pins_arduino.h in ...\Arduino\hardware\arduino\avr\variants).

For us this has two problems. We are using additional boards not supported by Arduino (https://github.com/MarlinFirmware/Marlin/tree/RC/ArduinoAddons/Arduino_1.6.x/hardware/marlin/avr/variants). And the way they map the connectors to the registers is, compared to other ways, awfully slow. They look up the registers in big arrays at runtime. The big plus for their system is - it always works, even if the pin-number is a variable.

There are several speedups out there, in the wild, to make this translation faster. Marlin uses mainly fastio.h (https://github.com/MarlinFirmware/Marlin/blob/RC/Marlin/fastio.h). The speedup here is based on the knowledge of the exact pin number at compile time, (variables do not work), so the lookup can be done at compile time - not at runtime. fastio.h can only work if it knows about all the boards we use. As far as I know it does, including Teensy.

With the SD2library we imported https://github.com/MarlinFirmware/Marlin/blob/RC/Marlin/Sd2PinMap.h. That's another technique to speed up pin access. It also uses big arrays - faster than original Arduino but slower than fastio.h (I guess). Sd2PinMap.h also has to know about all the boards we use, but as far as I can see support for Teensy is not included. Sd2PinMap.h can also work with pin-numbers as variables.

To solve the problem we have two options:

• Integrate Teensy into Sd2PinMap.h. That means maintaining two speedup-systems.
• Change SD2lib to use fastio.h. That seems to be the better option - if it works.

To find the 8 relevant places, all in SD2Card.cpp, grep for fastDigitalWrite( and fastDigitalRead(. All these places look as if defined values should work. Sadly SPI_MISO_PIN,SPI_MOSI_PIN and SPI_SCK_PIN are defined from MISO_PIN, MOSI_PIN and SCK_PIN in Sd2Card.h. Those are #undef'ed and redefined in Sd2PinMap.h. For me it's not obvious if the definitions in some pin/board-files for this are valid/used or not So testing is required.

[thinkyhead]

That is a very helpful summary, thank you. I happen to have a Teensy, but no pin headers, no oscilloscope… so I'm hesitant to make any changes without having a tester by my side.

[jbrazio]

Change SD2lib to use fastio.h. That seems to be the better option - if it works.

Upvote.

Those are #undefined and redefined in Sd2PinMap.h

But why ? Was this code from a Marlin dev or it just came like that from the SD lib ?

[esenapaj]

@thinkyhead
I offer simple oscilloscope and pin headers to you.
Those were already shipped from Secaucus, NJ by UPS, and Amazon.com said that it's arriving Wednesday(May 11).

I wrote a e-mail that it has tracking number and other infomation, and sent to [email protected].
But the mail might have been thrown into spam folder because the mail include hyperlink.

[jbrazio]

@thinkyhead I'm marking this one as a bug.
Do you plan to proceed with the required changes ?

[thinkyhead]

@jbrazio I don't mind working on solving this. I will give it a go now…

[AnHardt]

Maybe you'd like to have a jumpstart. https://github.com/AnHardt/Marlin/tree/sd2pinmap
Removing the functions is not the difficult part. I had problems to replace the pin definitions.

[thinkyhead]

@AnHardt This also looks interesting:
https://github.com/mmarchetti/DirectIO#user-content-why-use-directio

[Roxy-3D]

This also looks interesting:
https://github.com/mmarchetti/DirectIO#user-content-why-use-directio

That link is interesting! I'm not sure this is related but I'm starting to think it might be. I've got a lot of 'twitch' on my ABL Servo. The problem is much worse when I have it controlled by an 8-bit port with something else that is switching. If it is on it's own port where no other bits are switching it is not perfect, but it is much better behaved.

I wonder if the DigitalWrite() routines are causing glitches on the port's I/O pins?

[AnHardt]

I guess i can see now where the actual title problem can come from.

https://github.com/MarlinFirmware/Marlin/blob/RC/Marlin/fastio.h#L2049
https://github.com/MarlinFirmware/Marlin/blob/RC/Marlin/fastio.h#L2699

defines the SPI pins in common for AT90USB and Teensy while they should be different.

Likely the changes in fastio.h in https://github.com/AnHardt/Marlin/pull/61/files are enough to solve the problem.

Sorry untested. I have neither the one nor the other processor at hand.

I have stopped the attempt to remove SD2cardLib for now. The problem in fastio.h seems to be to logical to be not the reason for this issue.

[boelle]

is this one still being worked on or has it even been fixed but not marked as such?