Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add SPI daisy chain capability #66

Merged
merged 5 commits into from
Sep 6, 2019
Merged

add SPI daisy chain capability #66

merged 5 commits into from
Sep 6, 2019

Conversation

Bob-the-Kuhn
Copy link
Contributor

@Bob-the-Kuhn Bob-the-Kuhn commented Aug 28, 2019

This PR adds the ability to use SPI daisy chaining. This allows the use of just four pins no matter how many drivers are in the chain.

The daisy chain routines are based on the L6470 daisy chain routines in Marlin.

The code changes have been tested on a modified RAMPS-FD V2 using TMC2130 and TMC5160 drivers in one and four driver chains. Only the soft SPI mode was tested.

See PR #15081 for the Marlin side of the changes.


The array TMC_chain[] is used to hold the information as to the order in which the drivers are chained.

  [0] - number of drivers in chain
  [1] - axis_index for first device in the chain (closest to MOSI)
  [2] - axis_index for next device in the chain

axis index is an unique arbitrary number assigned by the application to the axis that a driver powers.

void set_chain_info(const uint8_t axis_index, const uint8_t chain_position); is used by the application to populate TMC_chain[] and to save the axis_index in the stepper object. This must be done after the steppers objects have all been created and before the drivers are initialized.

Wrappers were added to the read and write functions to implement the daisy chain functionality. All the wrappers do is:

  1. Scan the TMC_chain[] backwards (data for the driver furthest from MOSI is sent first).
  2. If the contents of TMC_chain[i] matches the axis_index of the stepper object then the data is sent using the appropriate (standard) TMC SW or HW SPI driver. If there isn't a match then five dummy bytes are sent (contents are 0x00).

The TMC2130 SPI read and SPI write routines were re-arranged and broken out into subroutines to make it easier to understand the logic flow.


TO DO

  • Look at adding a TMC_CHAIN_CS define so that the rule of using just a single chip select is automatically enforced.
  • Look at moving all SPI daisy chain class items in the TMC2130 area.

@Bob-the-Kuhn
Copy link
Contributor Author

Let me know if this PR should be written against another branch. If that's the case then I'll kill this PR and create a new one.

@teemuatlut
Copy link
Owner

Thanks.
I had a bit different interface and a simpler implementation in mind but I never got around to writing it out, so I may just merge this in as-is just to have it as an included feature. Then later when I have the time I can fine tune it further to better suit my own vision.
But if you don't mind, could you separate the "read/write break down into functions" into a separate commit. It's a bit hard to see what was actually changed when large chunks of code just gets moved while also receiving modifications.

master is fine.

1. TMCStepper.cpp - rearrange logic to match layout in current database.  Moved logic from subroutines back into the main logic flow.
2. Moved all SPI daisy chain items into TMC2130Stepper
Missed a couple of items when rearranging TMC2130Stepper.cpp:
1. adding blank lines back in
2. added "status_response = " back in
remove trailing spaces and remove added/extra line
@Bob-the-Kuhn
Copy link
Contributor Author

I've rearranged the logic back to the current database's logic flow. Definitely easier to see what is changing.

I did run across two logic errors in the original PR.

  • Passed too few arguments into TMC_SW_SPI_transfer_40 and TMC_HW_SPI_transfer_40 in some places. The config parameter isn't always used and should have been set to zero in those cases.
  • status_response = was missing in a couple of cases.

@Bob-the-Kuhn
Copy link
Contributor Author

If you can describe your vision & interface I'll see if I can write the code & test it.

@teemuatlut
Copy link
Owner

teemuatlut commented Sep 4, 2019

3f90a6a

Drivers are indexed starting from 1. The highest given index determines the chain length.
Write operation starts by pushing the valid data into the first link, then shifting it to the correct place as given by the link index.
Read starts the same as write but then we also shift the response data first into the last link and then once more into the MCU.
Link index and max chain length are not exposed outside of the constructor argument.
SRAM use only 1 + 1*n bytes.

Obvious disclaimer: not tested, only for SW SPI, supported constructors for TMC2130 only for now, needs clean up to remove all the repetition.
Overall all the reads and writes are in need of clean up atm...
After PoC it's easily expanded for the other SPI drivers as well.

@Bob-the-Kuhn
Copy link
Contributor Author

The only bug I found is the lack of int8_t i = 1; in TMC2130Stepper::write.

I did a quick sanity check on my 5160 six device chain. All stepper motors worked correctly.

Tomorrow I'll move my chain over to a test rig and make sure that the correct device in the chain and only that device is being accessed. My printer has 5 identical 5160 settings so it's possible to be sending the Y2 settings to Z and be perfectly happy.

Also tomorrow I'll do a Marlin PR to match your code.


Might as well include the changes in the HW SPI sections. Yes, it's untested but it's also low risk.

@Bob-the-Kuhn
Copy link
Contributor Author

I remember why I always pushed nulls to every unused device in a chain. The 5160 puts non-zero data on SDO when the very first 40 bits is being clocked in. I wanted to be sure the "extra" data is shifted out of the chain before the CS goes high.

Apparently the non-zero data doesn't affect anything.

@Bob-the-Kuhn
Copy link
Contributor Author

Here's the TMC library code I used. This works properly in a mixed 5160/2130 system.

I included the hardware SPI.

I did not do any changes to the 2260. Since it has a 20 bit transfer then doing a mixed chain would be complicated.

@Bob-the-Kuhn
Copy link
Contributor Author

Marlin PR 15179 has been created to implement the Marlin side of the code proposed by @teemuatlut.

@teemuatlut teemuatlut merged commit 5863f6d into teemuatlut:master Sep 6, 2019
teemuatlut pushed a commit that referenced this pull request Sep 6, 2019
SPI drivers can be controlled by shifting data through a driver into the next one, creating a chain. Dummy data will be pushed into the bus to shift data to the correct target, and likewise to get data out of the target driver into the MCU.
Drivers are indexed from 1. Highest link index determines the chain length.
teemuatlut pushed a commit that referenced this pull request Sep 7, 2019
Add support for SPI daisy chaining (#66)

SPI drivers can be controlled by shifting data through a driver into the next one, creating a chain. Dummy data will be pushed into the bus to shift data to the correct target, and likewise to get data out of the target driver into the MCU.
Drivers are indexed from 1. Highest link index determines the chain length.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants