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

Expose TL node(s) in periphery #169

Merged
merged 1 commit into from
Jan 6, 2021

Conversation

abejgonzalez
Copy link
Contributor

@abejgonzalez abejgonzalez commented Oct 27, 2020

This exposes the TL SPI nodes so that the MMCDevice/FlashDevice can be attached to them in the system that has the corresponding mixin (i.e. HasPeripherySPI / HasPeripherySPIFlash).

Background Info:

Currently sifive-blocks has both an MMCDevice and FlashDevice that can be bound to a TL SPI node to append to the DTS/DTB (

class MMCDevice(spi: Device, maxMHz: Double = 20) extends SimpleDevice("mmc", Seq("mmc-spi-slot")) {
override def parent = Some(spi)
override def describe(resources: ResourceBindings): Description = {
val Description(name, mapping) = super.describe(resources)
val extra = Map(
"voltage-ranges" -> Seq(ResourceInt(3300), ResourceInt(3300)),
"disable-wp" -> Nil,
"spi-max-frequency" -> Seq(ResourceInt(maxMHz * 1000000)))
Description(name, mapping ++ extra)
}
}
class FlashDevice(spi: Device, bits: Int = 4, maxMHz: Double = 50, compat: Seq[String] = Nil) extends SimpleDevice("flash", compat :+ "jedec,spi-nor") {
require (bits == 1 || bits == 2 || bits == 4)
override def parent = Some(spi)
override def describe(resources: ResourceBindings): Description = {
val Description(name, mapping) = super.describe(resources)
val extra = Map(
"m25p,fast-read" -> Nil,
"spi-tx-bus-width" -> Seq(ResourceInt(bits)),
"spi-rx-bus-width" -> Seq(ResourceInt(bits)),
"spi-max-frequency" -> Seq(ResourceInt(maxMHz * 1000000)))
Description(name, mapping ++ extra)
}
}
). So far as I know, the main way to add one of these devices is to do something like the following:

ResourceBinding {
  Resource(new MMCDevice(tlSpiNode.device, 1), "reg").bind(ResourceAddress(0))
}

where the tlSpiNode is the TL SPI node that you want to add the DTS snippet to. This method of adding to the DTS was done in the Freedom platform (https://github.com/sifive/freedom/blob/943ab4ac2cefbbabdeda9447ec0f6231f6235f1e/src/main/scala/unleashed/DevKitFPGADesign.scala#L88-L91).

However, in newer versions of sifive-blocks this functionality was removed since the TL node is never exposed (only the IO's are exposed). This PR re-exposes the TL node like before while keeping the default functionality.

@abejgonzalez
Copy link
Contributor Author

abejgonzalez commented Dec 1, 2020

What's the timeline to merge/reject this PR?

@tamood
Copy link

tamood commented Dec 10, 2020

The benefits of HasPeriphery mix-ins cannot be exploited when they expose sinks instead of actual IORegisterRouter instances. Therefore, this patch must be applied everywhere, similar to the state prior to "spi: converted to PeripheralPuncher" commit.

@tamood
Copy link

tamood commented Dec 11, 2020

To be more clearer, one can do things like this, if TLSPI instance from attachto(Subsystem) is available

trait HasCustomImp extends LazyModuleImp
{
val outer : RocketSubsystem with HasPeripherySPI
val spicontrol = Wire(outer.spis(0).module.ctrl)
// good things follow
}

@abejgonzalez
Copy link
Contributor Author

Chipyard (https://github.com/ucb-bar/chipyard/) is looking to do a release with this change within the next week or two (ucb-bar/chipyard#747). Would it be possible to get this merged in (or re-reviewed) by then? Thanks.

@mwachs5
Copy link
Collaborator

mwachs5 commented Dec 28, 2020

Hi @abejgonzalez we are taking a look

@alonamid
Copy link

alonamid commented Jan 6, 2021

Is there any update on the timeline on this? It has some implications on the Chipyard release @abejgonzalez mentioned.

@mwachs5
Copy link
Collaborator

mwachs5 commented Jan 6, 2021

Hi @alonamid , thanks for your patience as things were slow over the holidays. Merging now!

@mwachs5 mwachs5 merged commit 545a396 into sifive:master Jan 6, 2021
@abejgonzalez
Copy link
Contributor Author

Thanks!

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.

5 participants