M-Labs
/
artiq-zynq
10
1
Fork 19
Code Issues 13 Pull Requests Releases Wiki Activity

change io_expander not to borrow i2c indefinitely and create io_expander0..1 to store out_current data #255

Merged
sb10q merged 1 commits from morgan/artiq-zynq:refactor_expander into release-7 2023-08-31 15:50:23 +08:00
Conversation 3 Commits 1 Files Changed 3 +77 -58
morgan commented 2023-08-31 15:12:43 +08:00
Owner
Copy Link

  • IoExpander at io_expander.rs now does not borrow i2c indefinitely

    • IoExpander does not store i2c anymore
    • IoExpander functions now take in i2c

  • IoExpander functions are modified to takes in i2c parameter at runtime & satman main.rs

    • io_expander0 and io_expander1 are created to store out_current data

  • compilation check

    • run nix build <variant> -L for kasli-soc master, satellite & standalone and all built successfully

  • functional check

    • veritied with a multimeter that the TX_DISABLE pin of SFP0..3 were driven to false after startup for all 3 variants

Checklist of backporting SFP0..3 LED indication

  • change write_rustc_cfg_file(soc, filename) to follow artiq repo (PR #254)
  • change io_expander not to borrow i2c indefinitely and create io_expander0..1 to store out_current data (This PR)
  • update io_expander due to pin swap on Kasli-soc v1.1 #228 (PR #257)
  • implement SFP0..3 LED indication (PR #258)
- `IoExpander` at `io_expander.rs` now does not borrow i2c indefinitely - `IoExpander` does not store `i2c` anymore - `IoExpander` functions now take in `i2c` - `IoExpander` functions are modified to takes in `i2c` parameter at runtime & satman `main.rs` - `io_expander0` and `io_expander1` are created to store `out_current` data - compilation check - run `nix build <variant> -L` for kasli-soc master, satellite & standalone and all built successfully - functional check - veritied with a multimeter that the TX_DISABLE pin of SFP0..3 were driven to false after startup for all 3 variants ## Checklist of backporting SFP0..3 LED indication - [X] change write_rustc_cfg_file(soc, filename) to follow artiq repo (PR #254) - [X] change io_expander not to borrow i2c indefinitely and create io_expander0..1 to store out_current data (This PR) - [X] update io_expander due to pin swap on Kasli-soc v1.1 #228 (PR #257) - [x] implement SFP0..3 LED indication (PR #258)
morgan added 1 commit 2023-08-31 15:12:44 +08:00
sb10q commented 2023-08-31 15:24:51 +08:00
Owner
Copy Link

Not sure if that's an improvement. TBH I find the original code borderline.

Not sure if that's an improvement. TBH I find the original code borderline.
sb10q commented 2023-08-31 15:25:35 +08:00
Owner
Copy Link

Also we generally only fix bugs on release-7. There is no bug associated with this as far as I can tell.

Also we generally only fix bugs on release-7. There is no bug associated with this as far as I can tell.
sb10q reviewed 2023-08-31 15:30:54 +08:00
morgan commented 2023-08-31 15:38:16 +08:00
Author
Owner
Copy Link

Also we generally only fix bugs on release-7. There is no bug associated with this as far as I can tell.

For the SFP0..3 LED, I need to create ioexpander0 and ioexpander1 which before the patch, one will borrow the i2c indefinitely and fail the borrow checker. Also, if the ioexpander is created like in the for loop, there will be a bug where Kasli-soc cannot turn off all its LED (self.out_current = [0; 2]). Due to self.out_current reset to [0; 2] each time an ioexpander is created. The self.out_target != self.out_current check will fail which won't allow all LED to turn off.

> Also we generally only fix bugs on release-7. There is no bug associated with this as far as I can tell. For the SFP0..3 LED, I need to create `ioexpander0` and `ioexpander1` which before the patch, one will borrow the `i2c` indefinitely and fail the borrow checker. Also, if the `ioexpander` is created like in the for loop, there will be a bug where Kasli-soc cannot turn off all its LED `(self.out_current = [0; 2])`. Due to `self.out_current` reset to `[0; 2]` each time an `ioexpander` is created. The [`self.out_target != self.out_current`](https://git.m-labs.hk/M-Labs/artiq-zynq/src/commit/37df7fd45b329fd6b9c7555180c399301f0ea882/src/libboard_artiq/src/io_expander.rs#L161) check will fail which won't allow all LED to turn off.
morgan changed title from change io_expander not to borrow i2c indefinitely to change io_expander not to borrow i2c indefinitely and create io_expander0..1 to store out_current data 2023-08-31 15:43:23 +08:00
sb10q merged commit 5597927df9 into release-7 2023-08-31 15:50:23 +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#255
No description provided.
Delete Branch "morgan/artiq-zynq:refactor_expander"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?