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.

Sign up for GitHub

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

Jump to bottom

Add I3C Controller IP + Doc #1267

Merged
merged 3 commits into from
Apr 12, 2024
Merged

Add I3C Controller IP + Doc #1267

merged 3 commits into from
Apr 12, 2024
+5,217 −0

Conversation

gastmaier
Copy link
Contributor

@gastmaier gastmaier commented Jan 31, 2024 •
edited
Loading

PR Description

Adds a I3C Controller with the required by specification features.
The interface and instruction set are explained at docs/library/i3c_controller/interface.
The specification can be downloaded at mipi i3c basic.

Note: Our guideline checker doesn't allow verilog files without modules, however, I created header verilog files to define values used across multiple files and the testbench. Those files are:

./library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap_defs.v
./library/i3c_controller/i3c_controller_core/i3c_controller_word_cmd.v
./library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod_cmd.v

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

This was referenced Feb 5, 2024
Closed
Merged
@sarpadi sarpadi self-requested a review March 7, 2024 09:06
sarpadi
sarpadi reviewed Mar 8, 2024
View reviewed changes
docs/regmap/adi_regmap_i3c_controller.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

there are inconsistencies in the way in which registers are broken down into fields. sometimes the top field is the MSB and sometimes it is the LSB.

for example the IRQ register fields start with [0] CMD_ALMOST_EMPTY and descend to [7] DAA_PENDING. it should be the other way arround. the same goes for FIFO_STATUS, OPS, IBI_CONFIG etc

library/i3c_controller/i3c_controller_core/i3c_controller_bit_mod.v Outdated
end
end

if (!cmd_ready) begin
if (!cmdb_ready) begin
count <= count + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong indentation

docs/regmap/adi_regmap_i3c_controller.txt Outdated Show resolved Hide resolved
docs/regmap/adi_regmap_i3c_controller.txt Outdated Show resolved Hide resolved
@sarpadi
Copy link
Contributor

sarpadi commented Mar 8, 2024

Please implement the new rule regarding localparam location as described here f127a62

@gastmaier
Copy link
Contributor Author

V3
Moved localparam to satisfy new guideline f127a62
Regmap:

  • Replaced ?? with proper initial value where applicable.
  • Reordered fields to MSB.
    Fixup if_offload_trigger on tcl targeting Altera

@gastmaier gastmaier requested a review from sarpadi March 10, 2024 23:07
@gastmaier
Copy link
Contributor Author

gastmaier commented Mar 11, 2024 •
edited
Loading

V4
Fixup interfaces warnings:

  • xilinx.com:interface:axis must be upper case
  • sdio was accidentally suffixed by ,

Rename instances to make intel<->xilinx more homogeneous.
Fixup i3c host interface hide/show signals in Vivado.

@gastmaier
Copy link
Contributor Author

V5
Minor doc fix

sarpadi
sarpadi reviewed Mar 14, 2024
View reviewed changes
library/i3c_controller/i3c_controller_core/i3c_controller_word.v Outdated Show resolved Hide resolved
library/i3c_controller/i3c_controller_core/i3c_controller_word.v Outdated Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_host_interface.v Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Outdated Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Outdated Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Outdated Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Outdated Show resolved Hide resolved
library/i3c_controller/i3c_controller_host_interface/i3c_controller_regmap.v Outdated Show resolved Hide resolved
sarpadi
sarpadi reviewed Mar 15, 2024
View reviewed changes
docs/regmap/adi_regmap_i3c_controller.txt Outdated Show resolved Hide resolved
docs/regmap/adi_regmap_i3c_controller.txt Outdated Show resolved Hide resolved
@gastmaier gastmaier force-pushed the i3c branch 2 times, most recently from d0cb3bd to b80f1a6 Compare March 19, 2024 12:54
@gastmaier
Copy link
Contributor Author

V6
Code formatting (indentation, declaration location)
Regmap format fixes
Sync information on regmap and i3c_controller/interfaces.rst

@gastmaier gastmaier requested a review from sarpadi March 19, 2024 19:42
@gastmaier
Copy link
Contributor Author

V7
Add CLK_MOD parameter to i3c_controller_bit_mod, to allow the module to be driven by a clock of either 100MHz or 50MHz, still achieving the nominal speed of 12.5MHz. on the SCL lane.
On the 50MHz mode, after a word (cmdw) finishes the next one is fetched, the bus safely stalls for 2 system clock cycles.

@gastmaier
Copy link
Contributor Author

gastmaier commented Apr 1, 2024 •
edited
Loading

V8
Rebase to solve docs/library/index.rst conflict after #1280 merge.

gastmaier added 3 commits April 1, 2024 17:16
@gastmaier
common: Add ad_mem_dual
0776107 
Dual access memory abstraction.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
@gastmaier
i3c_controller: Add I3C Controller IP
f8087ba 
Add I3C Controller IP with required I3C features support.
Uses IRQ based DAA.
Supports speeds at 100MHz clk: 12.50MHz, 6.25MHz, 3.12MHz, 1.56MHz
Basic IBI support with/without MDB.
Compatible with AMD Xilinx and Altera FPGAs.

Signed-off-by: Jorge Marques <jorge.marques@analog.com>
@gastmaier
docs: i3c_controller: Add documentation
efa48a3 
Signed-off-by: Jorge Marques <jorge.marques@analog.com>
@gastmaier gastmaier merged commit 15ff99a into main Apr 12, 2024
1 of 3 checks passed
@gastmaier gastmaier deleted the i3c branch April 12, 2024 12:19
@gastmaier gastmaier mentioned this pull request Apr 16, 2024
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acostina acostina acostina approved these changes

@sarpadi sarpadi sarpadi approved these changes

@IuliaCMoldovan IuliaCMoldovan Awaiting requested review from IuliaCMoldovan IuliaCMoldovan is a code owner

@StancaPop StancaPop Awaiting requested review from StancaPop StancaPop is a code owner

@LBFFilho LBFFilho Awaiting requested review from LBFFilho LBFFilho is a code owner

@AndreiGrozav AndreiGrozav Awaiting requested review from AndreiGrozav AndreiGrozav is a code owner

@PopPaul2021 PopPaul2021 Awaiting requested review from PopPaul2021 PopPaul2021 is a code owner

Assignees
No one assigned
Labels
new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gastmaier @sarpadi @acostina