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

added spi 1-bit mode, code from litex #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danielkucera
Copy link
Contributor

as suggested in #87

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There mus be a way to support this without duplicating the entire code.

@danielkucera
Copy link
Contributor Author

There might be but is it worth? This solution is cleaner IMHO.

@sbourdeauducq
Copy link
Member

Thanks for submitting this PR.
Like @jordens I am concerned about the code duplication. What is this solution cleaner than?

self.specials.dq = dq.get_tristate(pads.dq)
else:
dq = SpiIF(pads.miso, pads.mosi, Signal())
with_bitbang = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling with_bitbang silently like this doesn't sound like a good idea.

Copy link
Contributor Author

@danielkucera danielkucera Jul 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, what about:

assert with_bitbang == False, "Bitbang not supported with 1-bit SPI flash."

?

else:
dq = SpiIF(pads.miso, pads.mosi, Signal())
assert with_bitbang == False, \
"Bitbang not supported with 1-bit SPI flash."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it was not working out of the box because of this line:
https://github.com/m-labs/misoc/blob/master/misoc/cores/spi_flash.py#L85
so I didn't implement it.
If you can explain the code, I can try to implement it.

@jordens
Copy link
Member

jordens commented Oct 9, 2018

@danielkucera ping

@danielkucera
Copy link
Contributor Author

@jordens what ping? I don't understand the bitbang code and with bitbang enabled, the code was failing. So I added an assert. Take it, leave it or explain the code.

@jordens
Copy link
Member

jordens commented Oct 9, 2018

@danielkucera The line you refer to does exactly what you are doing (take miso for 1-wide SPI) just in bitbang mode. The bigbang code is originally by @fallen but has gone through a few hands.

@danielkucera
Copy link
Contributor Author

@jordens , I've read it about 50 times but still I don't understand. Moreover I cannot test the code in 2,4-bit mode because I don't have such hardware so I'd rather not do any additional changes to the code.

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.

3 participants