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

SPI issue with CPOL=1 CPHS=1 #15

Open
darkstar007 opened this issue Apr 10, 2023 · 7 comments
Open

SPI issue with CPOL=1 CPHS=1 #15

darkstar007 opened this issue Apr 10, 2023 · 7 comments

Comments

@darkstar007
Copy link

Hi, I'm using this to talk to the ADC on a ULX3S dev board - and it doesn't seem to work. My hand crafted SPI module does work. The issue is that the comms returning from the ADC are garbage - not sure if that's because tx or rx or both are being mangled. My suspicion is that its caused by the sub-SPI-clk glitch that can be seen before and after the CS line transitions.

I noticed this on my logic analyser (the values from which agree with the values the fpga returns), and they can be seen in the attached test script.

import math

from amaranth import *
from amaranth.sim import *
from amlib.io import SPIControllerInterface

if __name__ == '__main__':
    word_size = 16
    divisor = 4
    clock_polarity = 1
    clock_phase = 1
    msb_first = True
    cs_idles_high = True
    
    spi = SPIControllerInterface(word_size=word_size, divisor=divisor,
                                 clock_polarity=clock_polarity, clock_phase=clock_phase,
                                 msb_first=msb_first, cs_idles_high=cs_idles_high)

    m = Module()
    m.submodules.spi = spi

    sim = Simulator(m)

    def process():
        yield spi.word_out.eq(0x1234)
        yield
        yield spi.start_transfer.eq(1)
        for x in range(4*16+12):
            yield

    sim.add_sync_process(process)
    sim.add_clock(1.0/50e6)

    with sim.write_vcd("test.vcd"):
        sim.run()

glitch

@hansfbaier
Copy link
Collaborator

hansfbaier commented Apr 10, 2023

It would need a MultiReg to make it work out of the box, I'll add that tomorrow.

@hansfbaier
Copy link
Collaborator

@darkstar007 I just pushed a fix and the SPI unit tests still pass. So please give it a try.

@darkstar007
Copy link
Author

Hi - thanks @hansfbaier

Unfortunately that doesn't appear to quite work - although it has improved things. If I look at the values that my Siglent logic analyser thinks are being sent they are off by a factor of two (i.e. I send 0x0040 and the LA says that's 0x0020). My suspicion is that the glitch (arrowed) at the start of the transmission is making an extra zero in the MOSI payload (taken on low-high transition) - i.e. its taking bits 0:15 rather than 1:16. As the ADC isn't happy, its not echoing back commands - you have to turn that on with software - so its hard to give much to say about MISO at the moment.

It you want further debug let me know.

The pictures are from my test script above - but look very similar to what's observed on my LA.

Thanks,
Matt
glitch2
glitch2_markup

@hansfbaier
Copy link
Collaborator

Ah I see. OK, will look into this.
You also might be able to fix this in principle, if you
look at the source code.

@darkstar007
Copy link
Author

OK - but this is my first time touching amaranth so its going to take a while. (Also, pretty much my first dealings with FPGAs......).

@hansfbaier
Copy link
Collaborator

Yes SPI is actually a great first project, because it is one of the most simple protocols.

@darkstar007
Copy link
Author

Yes, SPI is a very simple protocol - the fact that spi.py is already over 1000 lines of code raises alarm bells with me.

One of the errors is with the sync domain SPIControllerInterface when CPOL=1 requires a complete clock cycle to occur before it will start to output. According to the 'documentation': "Signals in synchronous control domains change whenever a specific transition (positive or negative edge) occurs on the clock of the synchronous domain." - but I can't see how the sync domain decides what signal is going to be its clock. I also can't find out how to make it change when it make an assignment - so that changing

       if self.clock_phase == 0:
            with m.If(Rose(chip_selected, domain="sync")):
                output_one_bit(m)

to be something like

       if self.clock_phase == 0:
            with m.If(Rose(chip_selected, domain="sync")):
                output_one_bit(m)
       else:
            with m.If(Rose(chip_selected, domain="sync")):
                 output_one_bit_on_negative_edge(m))

to try and force the value of sdo to be the correct value before the incoming positive edge transition of the clock.

So, yeah...........

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

No branches or pull requests

2 participants