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

Bit values are annotated on wrong clock edge #2

Open
adamgreig opened this issue Apr 5, 2020 · 6 comments · May be fixed by #4
Open

Bit values are annotated on wrong clock edge #2

adamgreig opened this issue Apr 5, 2020 · 6 comments · May be fixed by #4

Comments

@adamgreig
Copy link

As with #1 if this isn't the best place for bugs please let me know!

The analyzer adds bit value labels to clock edges, which is extremely helpful, but they don't seem to be on the correct clock edge. In this example of reading DRIDR:

image

The host sets the START (1) bit on the clock falling edge, and the target reads it on the subsequent rising edge, and the annotation is placed on the falling edge following that. I think this annotation should probably be on the rising edge where the target samples the value. That way the annotation would be placed in the centre of the time the bit is valid for, instead of right at the end immediately before the next bit is driven.

Conversely for target-to-host bits, such as the 100 ACK sequence here, the target sets the 1/0 on the preceding rising edge, the host samples it at the next falling edge (immediately before the next rising edge), and the annotation is placed on that next rising edge. I think the clearest thing would be to place the annotation on the falling edge where the host samples the value, so the bit annotation appears in the centre of the time the bit is valid for.

I've attached the traces for the screenshot to #1.

@timreyes
Copy link

timreyes commented Apr 8, 2020

Thanks for posting. This is in fact the best place to post bugs for analyzers. For the SWD analyzer, we officially track them all here. At the moment, we're laser focused on the next-gen application, Logic 2: https://discuss.saleae.com/

Once we officially release this version of the software, we'll begin updating our protocol analyzers. Unfortunately, we wouldn't plan on updating our source code until then.

Let me look into these issues in the meantime to see if it is in fact a bug or something less obvious related to settings.

@timreyes
Copy link

Hmm.. relooking at this, you're right. The bits should in fact appear on the edge where the sampling occurs. Once we wrap up development of Logic 2 and move on to analyzers, we'll get this fixed. We'll also look into how the START bit, and the following bits are handled. I'll leave this issue open in the meantime.

@Rob65
Copy link

Rob65 commented Apr 11, 2020

I've been talking to Tim about this and cross checking this with the ARM specs at https://developer.arm.com/docs/101761/0100/debug-and-trace-interface/serial-wire-debug-swd-signals

Data from the debug unit is supposed to change at the falling edge but it may change also before the falling edge. Data from the target will always change after (at least 4ns) the rising edge, with the data being stable at the rising edge (see figure 1-9 and table 1-2 in ARM's documentation)

ARM specifies:

The debug unit:

    Writes data to SWDIO on the falling edge of SWCLK.
    Reads data from SWDIO on the rising edge of SWCLK.

The target:

    Writes data to SWDIO on the rising edge of SWCLK.
    Reads data from SWDIO on the rising edge of SWCLK.

So looking at this, the analyzer should always sample the information on the rising edge of the SWCLK signal. Sampling on the rising edge after the turnaround is already correct.

I've copied a piece of Adam's picture:

image

and compared this with the timing specified by ARM:

image

With the annotations in red shown, this should be correct.

This is confirmed by looking at the code from the ARM DAPLink project:

#define SW_WRITE_BIT(bit)               \
  PIN_SWDIO_OUT(bit);                   \
  PIN_SWCLK_CLR();                      \
  PIN_DELAY();                          \
  PIN_SWCLK_SET();                      \
  PIN_DELAY()

#define SW_READ_BIT(bit)                \
  PIN_SWCLK_CLR();                      \
  PIN_DELAY();                          \
  bit = PIN_SWDIO_IN();                 \
  PIN_SWCLK_SET();                      \
  PIN_DELAY()

Data is written from the debug unit at the falling edge of the clock and samples at (just before) the rising edge.

@adamgreig
Copy link
Author

Thanks for the detailed response! Yep, moving the host-to-target annotations backwards to the rising edge where they are sampled by the target, and leaving the target-to-host annotations on the rising edge where they are valid just before the target changes them, sounds fine to me too.

@alejmrm
Copy link

alejmrm commented Nov 5, 2020

Yeap. We were fool by this issue at the floor test. do you have a timeframe by now?

@timreyes
Copy link

timreyes commented Nov 5, 2020

@alejmrm Unfortunately, no updates on this yet. Our focus still lies on improving and officially release Logic v2.
Download: https://ideas.saleae.com/f/changelog/
Logic 2 Progress: https://discuss.saleae.com/

bernardfitch added a commit to bernardfitch/swd-analyzer that referenced this issue Aug 16, 2022
tweaks the 'makrer' labelling of bits that are 'read' to be just before the falling edge of the clock (where the are actually read) rather than on the rising edge - see issue saleae#2 of the original Saleae analyser
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 a pull request may close this issue.

4 participants