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

SD card SPI conflicts #1226

Closed
Grogyan opened this issue Dec 22, 2014 · 24 comments
Closed

SD card SPI conflicts #1226

Grogyan opened this issue Dec 22, 2014 · 24 comments

Comments

@Grogyan
Copy link
Contributor

Grogyan commented Dec 22, 2014

The SPI for the SD card assumes no other devices are connected to the same bus and so data conflicts occur because the code doesn't do a sanity check to ensure that the SPI bus to free to communicate over.

Noticeable with;
RAMPS 1.4
RepRap discount full graphic display
Max6675 thermocouple

@boelle
Copy link
Contributor

boelle commented Dec 26, 2014

related to #1227

@boelle boelle added this to the Bug Fixing Round 3 milestone Jan 6, 2015
@boelle
Copy link
Contributor

boelle commented Jan 13, 2015

one way to solve this one is to use the chip select pin.... if master do not use SPI it will not pull the chip select lines high... one line is needed for each device...

when master wants a temp reading it will pull high the chip select line a and talk to the thermocouple... when done it puts the line low...

http://en.wikipedia.org/wiki/Serial_Peripheral_Interface-bus

@Grogyan
Copy link
Contributor Author

Grogyan commented Jan 14, 2015

Yes, this can be done, however, it will need careful implementation to not screw up in progress communication.
I did a very rough hack to see if it could resolve the issue, it did not, meaning that there are some underlying problems withing the SD card code implementation

@Grogyan
Copy link
Contributor Author

Grogyan commented Jan 14, 2015

http://elm-chan.org/docs/mmc/mmc_e.html
Explains that during the initialization method, the SD card needs to be set to SPI MODE, default on power up is SD MODE. The above document also explains that this MODE selection is CS dependent during initialization.
I don't know how it is implemented in Marlin, however, during power up, consideration for this step is essential.

@barneycg
Copy link

would this explain the times I've been seeing the sd card as showing files but when I try going into a sub directory it shows the files briefly and then they disappear ? Using a thermocouple hooked up with one of these http://reprap.org/wiki/ExtThermoCouple_1.0 boards

@boelle
Copy link
Contributor

boelle commented Jan 15, 2015

its a problem in general... SPI needs one line for each device that the master can use when it needs to talk to say the thermocouple... lets say it can talk to the LCD in general but then the thermocouple sends out data at the same time... why does this happen? because the CS (chipselect) line is not driven right

@thinkyhead
Copy link
Member

If this is related to code in the SDFat library, we should have a look at the SDFat Beta repo and see if the latest library is doing the right thing, then incorporate those changes. (Assuming there is a "right thing" it can even do!)

@Grogyan
Copy link
Contributor Author

Grogyan commented Jan 19, 2015

I just got home, and doing a quick catch up, before I head out again in 20 minutes.

It's the whole SD card implementation, it's just a mess.

I just had a look at that repo, briefly, and looking through the code, it is doing some right thing I expect in SdSpiCard.cpp
However, it does not check to see if any other devices are on the SPI bus.
A fix could be applied to the function, bool SdSpiCard::isBusy() but this is just a guess.

I have been racking my head for a couple of weeks now, of some way, this/similar function could be adapted to read all the SPI devices automagically. I only know some very basic elements in code, not in any way would I consider myself adept at coding.

@thinkyhead
Copy link
Member

The readme on SDFat beta seems optimistic with its mention of "Allow[ing] simultaneous use of hardware and software SPI with multiple cards. See the ThreeCard example." But I suppose it would be good to query the author @greiman and see what he thinks is possible.

@daid
Copy link
Contributor

daid commented Jan 20, 2015

You need this patch to fix the MAX6675 code with SD cards:
Ultimaker/Ultimaker2Marlin@91c64e3

As this changes the MAX6675 read from interrupt to main loop. And as the SD is also called from the main loop the conflict is suddenly gone.

@thinkyhead
Copy link
Member

I will roll a patch from that soon.

@thinkyhead thinkyhead self-assigned this Jan 23, 2015
thinkyhead added a commit to thinkyhead/Marlin that referenced this issue Jan 23, 2015
Changes to temperature.cpp from Ultimaker fork, intended to address
MarlinFirmware#1226 and MarlinFirmware#1227
thinkyhead added a commit to thinkyhead/Marlin that referenced this issue Jan 24, 2015
Changes to temperature.cpp from Ultimaker fork, intended to address
MarlinFirmware#1226 and MarlinFirmware#1227
@Grogyan
Copy link
Contributor Author

Grogyan commented Jan 27, 2015

Still haven't got around to checking this merge out.
We've just had a long weekend here in NZ and I happened to be away for it.

@fmalpartida
Copy link
Contributor

Just a comment question. ISP devices assume that they can share a bus but there is a chip select (or Slave Select signal in the spec) that determines which device has been granted access to the bus. In general, the master controller arbitrates and uses the SS to use the different peripherals.
In your configuration, do you have the same pin/signal driving the Chip Select of two devices?
If so, this is something that you should review.

The SPI bus is not like the I2C where you can address devices in the bus, they require their unique CS signal.

@fmalpartida
Copy link
Contributor

The Max6675 does not support being connected on a multi-slave shared CS configuration. As soon as the CS is asserted low and it sees a clock signal in the its SCK pin, it will start clocking data out. This will short anything on the MISO line.

Please indicate how the MAX6675 is connected and if it shares the CS with the SD card-reader.

@boelle
Copy link
Contributor

boelle commented Apr 25, 2015

yes i have said the same a few times.... i will try and find the schematics for the 2...

@fmalpartida
Copy link
Contributor

In the patch shared by daid it is using a different pin for the chip select.

@boelle
Copy link
Contributor

boelle commented Apr 25, 2015

oh... yes... so this just needs testing then

@boelle
Copy link
Contributor

boelle commented Apr 25, 2015

@Grogyan had time to test if the included patch worked?

but basicly the issue here is that every SPI device must have an individual CS pin

@AnHardt
Copy link
Member

AnHardt commented Apr 25, 2015

@fmalpartida
No. The patch does not use another pin. MAX code used to be in an interrupt - so could interrupt the communication with the sd-card an mess it up.. Now the Code for the MAX is execute in the main-loop. So they should interleave nicely.
Yes. The MAX always had its own CS Pin.

@fmalpartida
Copy link
Contributor

So we can close this one if someone can try it out. I don't have the needed setup.

@boelle
Copy link
Contributor

boelle commented Apr 25, 2015

@fmalpartida excatly... someone with the setup needs to test...

so if we dont see any activity in say 7 days i will close it

issues can always be reopened if the issue is still there

@Grogyan
Copy link
Contributor Author

Grogyan commented Apr 26, 2015

When I last checked, this problem was resolved.
But there have been a number of changes which I will need to check out next
weekend, probably, when I have time free.
On 26 Apr 2015 11:21, "Bo Herrmannsen" notifications@github.com wrote:

@fmalpartida https://github.com/fmalpartida excatly... someone with the
setup needs to test...

so if we dont see any activity in say 7 days i will close it

issues can always be reopened if the issue is still there


Reply to this email directly or view it on GitHub
#1226 (comment)
.

@boelle
Copy link
Contributor

boelle commented Apr 26, 2015

will close this one since it has been resolved... we can always open a new issue if the the problem comes back

@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants