Skip to content
This repository has been archived by the owner on Jul 6, 2019. It is now read-only.

hal::k20: Add DSPI registers #157

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

bgamari
Copy link
Contributor

@bgamari bgamari commented Aug 28, 2014

No description provided.

@bgamari
Copy link
Contributor Author

bgamari commented Aug 28, 2014

This example brings up an interesting limitation of the current ioregs! implementation:

PUSHR is the transmit data register of the DSPI peripheral. It is backed by a FIFO and contains a data field (TXDATA) as well as a bitfield (PCS[5:0]) to specify which chip-select(s) should be targeted by the transaction. I currently represent this register with,

0x34 => reg32 pushr {
  0..15 => txdata, //= Transmitted data
  16..21 => pcs[6], //= Chip select state
}

Unfortunately, reads to this register fetch the values associated with the head of the FIFO. This means that queuing a new transaction would require that we first clear all of the pcs flags, then set the one we intend along with txdata. This is a rather tricky task with the current interface.

Another solution would be mark the pushr register as write-only, since write-only registers are no longer read from on update. This unfortunately means we loose the ability read the TX FIFO head (although I'm frankly not sure how valuable this capability is).

This does suggest, however, that the current scheme isn't quite expressive enough. Perhaps we want a new register type which has the same no-read-on-update semantics as wo but preserves the read accessors? What would such a thing be called?

Thoughts?

@farcaller
Copy link
Member

Do I get it right that you want to write into txdata while preserving pcs value and not doing a read? Then a "cached" register might be a reasonable (although quite tricky to implement) option.

@bgamari
Copy link
Contributor Author

bgamari commented Aug 28, 2014

The opposite actually, we don't want to preserve any of the former contents of the register across the update.

@bharrisau
Copy link
Contributor

So there is no problem with the read? Just that when you write, you don't care about the previous value. So the current implementation will look like

pushr.set_txdata(data)
     .set_pcs(0, false)
     .set_pcs(1, true)
     .set_pcs(2, false)
     .set_pcs(3, false)
     .set_pcs(4, false)
     .set_pcs(5, false);

@bharrisau
Copy link
Contributor

Perhaps add a clear() that initialised with a full mask and a value of 0?

pushr.clear()
     .set_txdata(data)
     .set_pcs(1, true);

Otherwise just live with the slightly verbose language above.

@bgamari
Copy link
Contributor Author

bgamari commented Aug 29, 2014

@bharrisau correct.

clear is one solution, although it feels like a bit of a hack. I should probably just get over it though.

@farcaller
Copy link
Member

clear() sounds like writing zeroes to it.

@bharrisau
Copy link
Contributor

The other solution is a always-clear flag which indicates that there is write only, but still with reads.

@farcaller
Copy link
Member

@bgamari can you please update the state of this or move out from in progress?

@bgamari
Copy link
Contributor Author

bgamari commented Jun 10, 2015

I'll have a look tonight.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants