Bug 694 - Wishlist: an output type to cover the entire SPI transfer
Summary: Wishlist: an output type to cover the entire SPI transfer
Status: RESOLVED FIXED
Alias: None
Product: libsigrokdecode
Classification: Unclassified
Component: PD: spi (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-03 16:53 CET by leonerd
Modified: 2018-02-04 11:03 CET (History)
1 user (show)



Attachments
Code simplification to 'max7219' PD (2.58 KB, patch)
2015-11-08 02:16 CET, leonerd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description leonerd 2015-11-03 16:53:26 CET
When writing an SPI protocol decoder, almost every higher-level PD has to internally keep track of the CS status, buffer up the individual bytes as they are received, and then interpret them when CS becomes deasserted. I feel that writing SPI-attached PDs would be simpler if the SPI decoder itself would perform this basic step for them.

Currently it has two types of data output:

  BITS [list of [bitvalue, ss, es] * 2] * 2
  DATA bytevalue * 2

I'd like to propose a new type, called perhaps "TRANSFER", whose contents are the full byte-wise collection of DATA bytes to cover the entire CS transaction.

Thus, where currently it outputs e.g.

  ['CS-CHANGE', 1, 0]
  ['DATA', 0x10, 0xff] at times ss=100, es=110
  ['DATA', 0x23, 0xff] at times ss=111, es=121
  ['CS-CHANGE', 0, 1]

I'd like to add a new single extra item that looks like:

  ['TRANSFER',
     [[0x10, 100, 110], [0x23, 111, 121]],
     [[0xff, 100, 110], [0xff, 111, 121]]]

That is, data1 relates to MISO and data2 relates to MOSI as with DATA, but the value in each is a list of elements, each element being a [bytevalue, ss, es] triple.

This structure would make it much simpler to write most SPI-based PDs.
Comment 1 leonerd 2015-11-03 16:58:50 CET
(FTR I'm quite happy to implement this and provide a patch; I'm just asking first if that's a good idea to :) )
Comment 2 leonerd 2015-11-08 02:08:57 CET
An initial attempt at implementing this can now be found at:

  https://github.com/leonerd/libsigrokdecode/compare/spi-transfer
Comment 3 leonerd 2015-11-08 02:16:21 CET
Created attachment 183 [details]
Code simplification to 'max7219' PD
Comment 4 leonerd 2015-11-08 02:18:24 CET
Attached above is the patch of simplification to my (proposed) max7219 PD that having a TRANSFER packet type allows. It removes a few lines of logic, but moreover requires a lot less "stateful" handling within the PD; it just gets invoked once on the TRANSFER packet and inspects things entirely through local variables. Crucially, it removes having to store stateful things on the PD itself. This alone is surely a useful improvement.
Comment 5 Gerhard Sittig 2017-09-17 14:11:40 CEST
The feature got implemented in 8c90d7bdf21f.  Can the report get closed?
Comment 6 Gerhard Sittig 2018-02-04 11:03:39 CET
The feature has been in mainline since 2015-11-22.