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

[pinmux,top_earlgrey] Add ScanClock role to SPI_DEV_CLK and filter from flop inputs during DFT #23791

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

a-will
Copy link
Contributor

@a-will a-will commented Jun 25, 2024

Add a ScanClock role to the scan_role_e enum, then add the scan roles of all pinmux-connected DIOs and MIOs to the pinmux. Filter any DIO or MIO with the ScanClock role from connecting to the wakeup detector flops when scanmode is active.

Use SPI_DEV_CLK as the scan clock.

@a-will a-will marked this pull request as ready for review June 25, 2024 04:27
@a-will a-will force-pushed the pinmux-scan branch 2 times, most recently from 819dfe8 to 56bbef2 Compare June 25, 2024 05:00
Copy link
Contributor

@msfschaffner msfschaffner left a comment

Choose a reason for hiding this comment

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

LGTM

for (int k = 0; k < NDioPads; k++) begin
dio_wkup_no_scan[k] = dio_in_i[k];
if (prim_mubi_pkg::mubi4_test_true_strict(scanmode_i) &&
TargetCfg.dio_scan_role[k] == ScanClock) begin
Copy link
Contributor

@msfschaffner msfschaffner Jun 25, 2024

Choose a reason for hiding this comment

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

nit: to make it easier for DV to cover, we may want to formulate this as a generate loop for the static stuff, and then use a ternary or an always_comb if inside that for scanmode_i.

for (int k = 0; k < NDioPads; k++) begin : gen_miodio
  if (TargetCfg.dio_scan_role[k] == ScanClock) begin : gen_dio_scan
    dio_wkup_no_scan[k] = prim_mubi_pkg::mubi4_test_true_strict(scanmode_i) ? 1'b0 : dio_in_i[k];   
  end else begin : gen_no_dio_scan
    dio_wkup_no_scan[k] = dio_in_i[k];   
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

@a-will
Copy link
Contributor Author

a-will commented Jun 25, 2024

@meisnere Note this could need changes in partner-provided scan_role_pkg and prim_pad_wrapper. scan_role_pkg would need the desired pad to be set to ScanClock, and I assume ScanClock would want to be handled like ScanIn in the pad wrapper.

The scan clock cannot be connected to flop inputs. Filter any DIO or MIO
that is designated as a scan clock.

Signed-off-by: Alexander Williams <[email protected]>
Select SPI_DEV_CLK as scan clock input. Because it is originally a clock
anyway, the DIO connects to a minimal amount of flops.

An alternate choice might be the EXT_CLK input, which would avoid some
of the complex timing constraints already on the SPI_DEV_CLK pad.

Signed-off-by: Alexander Williams <[email protected]>
@vogelpi
Copy link
Contributor

vogelpi commented Jun 25, 2024

CHANGE AUTHORIZED: hw/ip/pinmux/rtl/pinmux.sv
CHANGE AUTHORIZED: hw/ip/pinmux/rtl/pinmux_pkg.sv
CHANGE AUTHORIZED: hw/ip/prim/rtl/prim_pad_wrapper_pkg.sv

1 similar comment
@moidx
Copy link
Contributor

moidx commented Jun 25, 2024

CHANGE AUTHORIZED: hw/ip/pinmux/rtl/pinmux.sv
CHANGE AUTHORIZED: hw/ip/pinmux/rtl/pinmux_pkg.sv
CHANGE AUTHORIZED: hw/ip/prim/rtl/prim_pad_wrapper_pkg.sv

@moidx
Copy link
Contributor

moidx commented Jun 25, 2024

Hi @meisnere, please sync with @a-will in case there is help needed to synchronize these changes in your instantiation of the prim_pad_wrapper.

@a-will
Copy link
Contributor Author

a-will commented Jun 25, 2024

The RV_DM block-level test failures are unrelated. They should be fixed in #23803

@andreaskurth andreaskurth merged commit 44b2f81 into lowRISC:master Jun 26, 2024
30 of 32 checks passed
@a-will a-will deleted the pinmux-scan branch June 26, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants