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

Fix FlexIO register mistakes on 106x #41

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

Finomnis
Copy link
Contributor

@Finomnis Finomnis commented Jun 20, 2023

Fixes #40 and #42.

Note that svdtools had to be updated to 0.2.8 to support the dim patches.

@Finomnis
Copy link
Contributor Author

Finomnis commented Jun 20, 2023

I can confirm that this made Teensy 4.0 Pin7 functional (FLEXIO2, PINSEL=17).

@mciantyre This should probably get reported to NXP, too.

@mciantyre
Copy link
Member

There might be a few other fields that could increase their bit width. A fast way to find these is to diff against the 1051 FlexIO register block. Have you noticed these in your prototyping, and should we patch these fields now?

If we patch the FlexIO peripheral in 1060 SVDs to look just like the FlexIO in the 1050 SVD, I'm expecting raltool to collapse the blocks together.

diff src/blocks/imxrt1051/flexio.rs src/blocks/imxrt1061/flexio.rs -C5
*** src/blocks/imxrt1051/flexio.rs	Tue Jun 20 11:47:03 2023
--- src/blocks/imxrt1061/flexio.rs	Tue Jun 20 11:52:28 2023
***************
*** 203,213 ****
  #[doc = "Pin State Register"]
  pub mod PIN {
      #[doc = "Pin Data Input"]
      pub mod PDI {
          pub const offset: u32 = 0;
!         pub const mask: u32 = 0xffff_ffff << offset;
          pub mod R {}
          pub mod W {}
          pub mod RW {}
      }
  }
--- 203,213 ----
  #[doc = "Pin State Register"]
  pub mod PIN {
      #[doc = "Pin Data Input"]
      pub mod PDI {
          pub const offset: u32 = 0;
!         pub const mask: u32 = 0xffff << offset;
          pub mod R {}
          pub mod W {}
          pub mod RW {}
      }
  }
***************
*** 432,442 ****
          }
      }
      #[doc = "Parallel Width"]
      pub mod PWIDTH {
          pub const offset: u32 = 16;
!         pub const mask: u32 = 0x1f << offset;
          pub mod R {}
          pub mod W {}
          pub mod RW {}
      }
  }
--- 432,442 ----
          }
      }
      #[doc = "Parallel Width"]
      pub mod PWIDTH {
          pub const offset: u32 = 16;
!         pub const mask: u32 = 0x0f << offset;
          pub mod R {}
          pub mod W {}
          pub mod RW {}
      }
  }
***************
*** 568,578 ****
          }
      }
      #[doc = "Trigger Select"]
      pub mod TRGSEL {
          pub const offset: u32 = 24;
!         pub const mask: u32 = 0x3f << offset;
          pub mod R {}
          pub mod W {}
          pub mod RW {}
      }
  }
--- 568,578 ----
          }
      }
      #[doc = "Trigger Select"]
      pub mod TRGSEL {
          pub const offset: u32 = 24;
!         pub const mask: u32 = 0x1f << offset;
          pub mod R {}
          pub mod W {}
          pub mod RW {}
      }
  }

@Finomnis
Copy link
Contributor Author

Finomnis commented Jun 20, 2023

@mciantyre Good call! Fixed.

@Finomnis Finomnis changed the title Make FlexIO PINSEL 5 bits on 106x Fix FlexIO register widths on 106x Jun 21, 2023
@Finomnis
Copy link
Contributor Author

Finomnis commented Jun 21, 2023

@mciantyre They did merge with 1051; however, I realized that they shouldn't. 1051 only has 4 shifters and 4 timers, while 106x have 8 shifters and 8 timers. I feel like they screwed up everything in the SVD that they possibly could. See #42.

@Finomnis Finomnis changed the title Fix FlexIO register widths on 106x Fix FlexIO register mistakes on 106x Jun 21, 2023
@Finomnis
Copy link
Contributor Author

Finomnis commented Jun 21, 2023

@mciantyre I fixed the register dimensions. Note that I had to update svdtools to 0.2.8, as 0.2.6 doesn't support dim patches yet.

The 1060 SVDs suggest that FlexIO2 and 3 have 16 pins. They should have
32 pins. This commit updates select field widths to five bits to express
the additional pins. This change affects FlexIO1, and downstream users
will need to handle the fact that these extra pins are invalid.
The committer cross-checked all increased field widths against the 1060
RM.

This commit also increases the timer and shifter count for the FlexIO
peripherals. This implementation has eight timers and shifters, not
four.
Copy link
Member

@mciantyre mciantyre left a comment

Choose a reason for hiding this comment

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

Thanks for all these fixes. I checked the increased field widths against the 1060 RM; they LGTM. Note that I didn't scan the entire peripheral to make sure all fields are fixed.

I'll include this in a 0.5 release. I'm playing with that v0.5 release branch in my remote.

@Finomnis
Copy link
Contributor Author

Finomnis commented Jun 27, 2023

@mciantyre I used all the timers and shifters with ids >4 and <4, and even with DMA, and pins >16, and it worked so far. So while I also can't be 100% sure that I didn't overlook something (like I did with TIMSEL before), so far I didn't encounter any issues any more.

@Finomnis
Copy link
Contributor Author

@mciantyre How is your testing going? :) I'm starting to get increasingly interested in getting this merged, my driver is finished now, and I'd love to get it published :)

Sadly I still have this line in its Cargo.toml:

[patch.crates-io]
imxrt-ral = { git = "https://github.com/Finomnis/imxrt-ral.git", branch = "pinsel_5bit" }

@mciantyre mciantyre merged commit 99ff2bd into imxrt-rs:master Jul 3, 2023
20 checks passed
@Finomnis Finomnis deleted the pinsel_5bit branch July 3, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants