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

Sonata v1.0 SPI block driver #357

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

HU90m
Copy link
Collaborator

@HU90m HU90m commented Nov 20, 2024

The main changes:

  1. Chip select (CS) lines are now controlled by a register in the SPI controller
    itself.
  2. FIFO depths are now available in the 'info' register, and the FIFOs
    are reduced to 16 entries apiece.
  3. Ensure that zero-byte transfer requests do not result in a deadlock
    between software and hardware; the controller will attempt to start
    but cannot perform a zero-byte transfer.
    • Zero-byte blocking_write/read can be used instead to synchronise
      with the controller core.
  4. Interrupts for the SPI block added.
  5. Added internal loopback. Internal loopback within the SPI controller
    is trivial and very useful for testing; simultaneous transmit and
    receive shall result in all transmitted bytes being replicated
    exactly in the receive FIFO; we can test permutations of polarity and
    phase settings.
  6. Added software reset. Software reset is required to recover in the
    event of an error or indeed just a re-download of the software
    without an intervening IP block reset presently. Clear the TX FIFO,
    reset the controller core and then clear the RX FIFO.
  7. Adapted ethernet driver to use of the builtin SPI chip select

@HU90m
Copy link
Collaborator Author

HU90m commented Nov 20, 2024

It looks like the coding conventions check is failing due to a name collision between the old SPI driver v0.2/platform-spi.hh and the new platform-spi.hh.

Fixed in a commit with the message:

lints: Removed v0.2 versions of the sonata drivers

There are name collisions between the old v0.2 versions of the of sonata
system drivers and the new v1.0 versions. These cause clang-tidy lints
to fail.

The v0.2 drivers are frozen; they should not change before being
depreciated and removed from the repository. It is therefore safe to
remove them from the lints.

HU90m and others added 6 commits November 20, 2024 18:31
1. CS lines are now controlled by a register in the SPI controller
   itself.
2. FIFO depths are now available in the 'info' register, and the FIFOs
   are reduced to 16 entries apiece.
3. Ensure that zero-byte transfer requests do not result in a deadlock
   between software and hardware; the controller will attempt to start
   but cannot perform a zero-byte transfer.
    *  Zero-byte blocking_write/read can be used instead to synchronise
       with the controller core.
4. Interrupts for the SPI block added.
5. Added internal loopback. Internal loopback within the SPI controller
   is trivial and very useful for testing; simultaneous transmit and
   receive shall result in all transmitted bytes being replicated
   exactly in the receive FIFO; we can test permutations of polarity and
   phase settings.
6. Added software reset. Software reset is required to recover in the
   event of an error or indeed just a re-download of the software
   without an intervening IP block reset presently. Clear the TX FIFO,
   reset the controller core and then clear the RX FIFO.

Co-authored-by: Adrian Lees <[email protected]>
The SPI block now has built-in chip select outputs; one no longer has to
rely on a seperate, and possibly shared, GPIO block.

Note, the lock around the CS line changes is likely superflous now, but
has been left in for the time being.

Co-authored-by: Alex Jones <[email protected]>
There are name collisions between the old v0.2 versions of the of sonata
system drivers and the new v1.0 versions. These cause clang-tidy lints
to fail.

The v0.2 drivers are frozen; they should not change before being
depreciated and removed from the repository. It is therefore safe to
remove them from the lints.
Copy link
Member

@nwf nwf left a comment

Choose a reason for hiding this comment

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

Some nits, but generally LGTM!

InterruptReceiveWatermark = 1 << 1,
/// Asserted whilst the receive FIFO is full.
InterruptReceiveFull = 1 << 0,
} Interrupt;
Copy link
Member

Choose a reason for hiding this comment

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

Er, perhaps this is just a polarity thing, but I'd expect the InterruptTransmitWatermark to fire when the transmit FIFO is below its watermark (that is, it has room to accept a positive natural number of bytes before overflow) and, dually, InterruptReceiveWatermakr to fire when the receive FIFO is above its watermark (that is, it can source at least watermark-many bytes).

/**
* The Sonata SPI block doesn't currently have support for interrupts.
* The following registers are reserved for future use.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Stale comment, given the code below?

/**
* Chip Select lines; clear a bit to transmit/receive to/from the
* corresponding peripheral.
*/
Copy link
Member

Choose a reason for hiding this comment

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

What happens if this has a population count other than 1?

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.

2 participants