M-Labs
/
artiq-zynq
9
1
Fork 16
Code Issues 14 Pull Requests 5 Releases Wiki Activity

parsed and loaded bitstream from bootimage #12

Merged
sb10q merged 1 commits from pca006132/artiq-zynq:devc-clean into master 2020-06-16 17:47:27 +08:00
Conversation 0 Commits 1 Files Changed 5
pca006132 commented 2020-06-15 17:08:56 +08:00

It can now parse the header and load the bitstream from the bootimage, i.e. boot.bin stored in the SD card.

It can now parse the header and load the bitstream from the bootimage, i.e. `boot.bin` stored in the SD card.
sb10q reviewed 2020-06-15 17:30:09 +08:00
src/runtime/src/main.rs
@ -40,0 +129,4 @@
let buffer = &mut raw_buffer[offset..unencrypted_length as usize * 4 + offset];
file.read_exact(buffer).unwrap();
cache::dcci_slice(buffer);
sb10q commented 2020-06-15 17:30:09 +08:00
Owner

Shouldn't this be moved into the devc driver?

Shouldn't this be moved into the devc driver?
pca006132 commented 2020-06-15 17:42:48 +08:00
Poster

This requires refactoring, as the current implementation just takes a u32 as address... but should be simple to do

This requires refactoring, as the current implementation just takes a `u32` as address... but should be simple to do
sb10q commented 2020-06-15 17:54:56 +08:00
Owner

OK please do it.

OK please do it.
sb10q reviewed 2020-06-15 17:31:35 +08:00
src/runtime/src/main.rs
@ -40,0 +89,4 @@
// read boot header signature
file.seek(SeekFrom::Start(0x24)).unwrap();
if read_u32(file) != BOOT_HEADER_SIGN {
panic!("Invalid header!");
sb10q commented 2020-06-15 17:31:35 +08:00
Owner

I would not use panic! here, instead return a Result.

Suggest moving all this boot.bin and bitstream loading code into its own file, e.g. runtime/src/load_pl.rs

I would not use ``panic!`` here, instead return a ``Result``. Suggest moving all this boot.bin and bitstream loading code into its own file, e.g. ``runtime/src/load_pl.rs``
pca006132 commented 2020-06-15 17:43:52 +08:00
Poster

ok, I would try to turn the panic things and unwrap things into appropriate error handling and refactor them into load_pl.rs.

ok, I would try to turn the `panic` things and `unwrap` things into appropriate error handling and refactor them into `load_pl.rs`.
sb10q reviewed 2020-06-16 16:02:05 +08:00
src/runtime/src/main.rs
@ -60,3 +56,2 @@
} else {
info!("loading gateware");
unimplemented!("gateware loading");
// Load from SD card
sb10q commented 2020-06-16 16:02:05 +08:00
Owner

Move this (i.e. the "Load for SD card" block, but not the "devc.is_done" test) to load_pl as well.

Move this (i.e. the "Load for SD card" block, but not the "devc.is_done" test) to load_pl as well.
sb10q reviewed 2020-06-16 16:09:27 +08:00
src/runtime/src/main.rs
@ -46,20 +47,43 @@ pub fn main_core0() {
ram::init_alloc_linker();
match config::read_str("foo") {
sb10q commented 2020-06-16 16:09:27 +08:00
Owner

Leave this.

Leave this.
sb10q reviewed 2020-06-16 16:09:58 +08:00
src/Cargo.lock
@ -232,3 +232,3 @@
name = "libsupport_zynq"
version = "0.0.0"
source = "git+https://git.m-labs.hk/M-Labs/zc706.git#a17a5d2925e7e521f31320ccefd7bf3cb61c45a7"
source = "git+https://git.m-labs.hk/M-Labs/zc706.git#191da7c959baeb30e31196f2c35b0790ea25cbf6"
sb10q commented 2020-06-16 16:09:58 +08:00
Owner

Changing Cargo.lock requires updating cargoSha256 in default.nix.

Changing Cargo.lock requires updating cargoSha256 in default.nix.
sb10q reviewed 2020-06-16 16:51:20 +08:00
src/runtime/src/main.rs
@ -57,3 +58,1 @@
// Do not load again: assume that the gateware already present is
// what we want (e.g. gateware configured via JTAG before PS
// startup, or by FSBL).
// Do not load again: assume that the gateware already present is
sb10q commented 2020-06-16 16:51:20 +08:00
Owner

This should be indented one more block.

This should be indented one more block.
sb10q reviewed 2020-06-16 16:51:37 +08:00
src/runtime/src/main.rs
@ -23,3 +26,1 @@
mod kernel;
mod moninj;
mod sd_reader;
sb10q commented 2020-06-16 16:51:37 +08:00
Owner

Why change the order of the imports?

Why change the order of the imports?
sb10q reviewed 2020-06-16 16:51:44 +08:00
src/runtime/src/main.rs
@ -9,3 +9,3 @@
use log::info;
use libboard_zynq::{timer::GlobalTimer, logger, devc};
use fatfs::File;
sb10q commented 2020-06-16 16:51:44 +08:00
Owner

Is this used?

Is this used?
sb10q reviewed 2020-06-16 16:54:17 +08:00
src/runtime/src/main.rs
@ -61,2 +62,2 @@
info!("loading gateware");
unimplemented!("gateware loading");
// Load from SD card
match load_pl::load_bitstream_from_sd(&mut devc) {
sb10q commented 2020-06-16 16:54:17 +08:00
Owner

I think the load_pl module should create the DevC object itself.
Release it above using e.g.

let is_done = {
  let devc = devc::DevC::new();
  devc.is_done()
};

or simply if devc::DevC::new().is_done() if that works?

I think the ``load_pl`` module should create the DevC object itself. Release it above using e.g. ```rust let is_done = { let devc = devc::DevC::new(); devc.is_done() }; ``` or simply ``if devc::DevC::new().is_done()`` if that works?
pca006132 commented 2020-06-16 16:57:23 +08:00
Poster

I think it should work. I pass the DevC object as I thought we should treat that as a singleton, I would change that.

I think it should work. I pass the DevC object as I thought we should treat that as a singleton, I would change that.
sb10q closed this pull request 2020-06-16 17:47:27 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
priority:critical
   
priority:high
   
priority:low
   
priority:medium
No Label
priority:critical
priority:high
priority:low
priority:medium
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/artiq-zynq#12
There is no content yet.
Delete Branch "pca006132/artiq-zynq:devc-clean"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?