Skip to content

Add support for FC14 SPI (Untested) and FC15 I2C#321

Merged
tullom merged 1 commit into
OpenDevicePartnership:mainfrom
sukomath:sukomath/fc15I2C
Mar 18, 2025
Merged

Add support for FC14 SPI (Untested) and FC15 I2C#321
tullom merged 1 commit into
OpenDevicePartnership:mainfrom
sukomath:sukomath/fc15I2C

Conversation

@sukomath
Copy link
Copy Markdown
Contributor

@sukomath sukomath commented Feb 19, 2025

@tullom FYI

@sukomath
Copy link
Copy Markdown
Contributor Author

@felipebalbi @jerrysxie @DM777-ms Adding FC15 support changes. For the review comment from #235 - are we planning to adding this in PAC? Let me know if there is anything related to this review comment that needs to be addressed in this PR.

image

@sukomath sukomath marked this pull request as ready for review February 19, 2025 20:06
Comment thread src/iopctl.rs
Comment thread src/i2c/mod.rs Outdated
Comment thread src/iopctl.rs Outdated
Comment thread src/iopctl.rs Outdated
Comment thread src/flexcomm.rs
@jerrysxie jerrysxie added the enhancement New feature or request label Feb 24, 2025
@jerrysxie jerrysxie assigned jerrysxie and sukomath and unassigned jerrysxie Feb 24, 2025
@jerrysxie jerrysxie changed the title Add support for FC15 I2C Add support for FC15 I2C and FC14 SPI (untested) Feb 25, 2025
@jerrysxie jerrysxie changed the title Add support for FC15 I2C and FC14 SPI (untested) Add support for FC15 I2C and FC14 SPI (Untested) Feb 25, 2025
@sukomath sukomath changed the title Add support for FC15 I2C and FC14 SPI (Untested) Add support for FC14 SPI (Untested) and FC15 I2C Feb 25, 2025
@jeffglaum jeffglaum moved this to In review in ODP Backlog Mar 6, 2025
@jerrysxie jerrysxie requested a review from Copilot March 12, 2025 14:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for FC14 SPI (untested) and FC15 I2C by introducing new abstractions for handling FC15 pins and extending flexcomm implementations. Key changes include:

  • Adding a new trait and struct (FC15Pin) in src/iopctl.rs with its corresponding macro to support FC15 pin configuration.
  • Introducing new flexcomm implementations for FLEXCOMM14 (SPI) and FLEXCOMM15 (I2C), updating clock selection and peripheral enable code.
  • Adjusting the I2C instance mapping and count to incorporate FLEXCOMM15.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/iopctl.rs Added new FC15 pin support via trait ToFC15Pin, FC15Pin struct, and a dedicated macro.
src/flexcomm.rs Extended flexcomm support to include new implementations for FLEXCOMM14 and FLEXCOMM15 with updated clock setup.
src/i2c/mod.rs Modified I2C instance handling to map FLEXCOMM15 to a new index and update the instance count.
src/chips/mimxrt633s.rs Updated peripheral list to include FC15 I2C pins.
src/chips/mimxrt685s.rs Updated peripheral list to include FC15 I2C pins.
Comments suppressed due to low confidence (3)

src/iopctl.rs:416

  • [nitpick] The set_function method in the FC15Pin implementation intentionally ignores the provided function value. Consider adding or expanding the inline documentation to clarify that FC15 pins have fixed functionality and thus do not support runtime function configuration.
fn set_function(&self, _function: Function) -> &Self {

src/flexcomm.rs:115

  • FLEXCOMM14 SPI support is marked as untested. Please ensure that adequate tests are added to validate its functionality before this feature is used in production.
// TODO: FLEXCOMM 14 is untested. Enable SPI support on FLEXCOMM14

src/i2c/mod.rs:105

  • [nitpick] Mapping FLEXCOMM15 to index 8 uses a magic number; consider documenting the rationale behind this mapping or replacing it with a named constant to enhance code clarity.
if $n == 15 { info_index = 8; }

@sukomath sukomath requested a review from jerrysxie March 12, 2025 19:16
Comment thread src/flexcomm.rs Outdated
Comment thread src/iopctl.rs Outdated
Comment thread src/i2c/mod.rs Outdated
Comment thread src/i2c/mod.rs Outdated
Comment thread src/iopctl.rs
Comment thread src/iopctl.rs
Comment thread src/iopctl.rs
Comment thread src/iopctl.rs
Comment thread src/iopctl.rs
Comment thread src/iopctl.rs
@jerrysxie jerrysxie linked an issue Mar 17, 2025 that may be closed by this pull request
@sukomath sukomath requested a review from felipebalbi March 17, 2025 20:44
@tullom tullom merged commit 602104e into OpenDevicePartnership:main Mar 18, 2025
@github-project-automation github-project-automation Bot moved this from In review to Done in ODP Backlog Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add support for flexcomm 14 and 15

7 participants