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

[top_earlgrey,pinmux] Enable pull-down for TAP strap 0 at reset #24604

Merged
merged 1 commit into from
Sep 23, 2024
Merged

[top_earlgrey,pinmux] Enable pull-down for TAP strap 0 at reset #24604

merged 1 commit into from
Sep 23, 2024

Conversation

andreaskurth
Copy link
Contributor

@andreaskurth andreaskurth commented Sep 19, 2024 •
edited
Loading

TAP strap 0 is sampled after reset (and only once for life cycle states
that are not TEST_UNLOCKED* or RMA). To ensure it gets sampled as 0
unless driven to 1 from an external source (and specifically that it
gets sampled as 0 when left floating / not connected), this enables the
pull-down of the pad at reset.

This resolves issue #24576 on master.

@andreaskurth andreaskurth self-assigned this Sep 19, 2024
vogelpi
vogelpi approved these changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks @andreaskurth, this looks good.

The default pull_select is 0, meaning this indeed results in a pull down after reset.

@vogelpi
Copy link
Contributor

vogelpi commented Sep 19, 2024

@andreaskurth , I just noted that this PR is directly into the release branch. Do first want to fix this on master and then just do a cherry pick (including reference to master commit)?

a-will
a-will reviewed Sep 19, 2024
View reviewed changes
hw/ip/pinmux/rtl/pinmux.sv Outdated Show resolved Hide resolved
@andreaskurth
Copy link
Contributor Author

Pinmux-related TLTs chip_padctrl_attributes and chip_sw_spi_device_pinmux_sleep_retention still pass 100% after this change (even with -rx 10).

I'm now running pinmux block-level FPV to complete confirmation of this change.

@andreaskurth
Copy link
Contributor Author

@andreaskurth , I just noted that this PR is directly into the release branch. Do first want to fix this on master and then just do a cherry pick (including reference to master commit)?

Yes, I'll retarget this PR to master.

@andreaskurth andreaskurth changed the base branch from earlgrey_1.0.0 to master September 19, 2024 13:56
@andreaskurth andreaskurth changed the title [top_earlgrey,pinmux] Enable pull-down for IOC8 at reset [top_earlgrey,pinmux] Enable pull-down for TAP strap 0 at reset Sep 19, 2024
@vogelpi
Copy link
Contributor

vogelpi commented Sep 19, 2024

CHANGE AUTHORIZED: hw/ip/pinmux/rtl/pinmux.sv

This PR touches one single RTL file to implement a confirmed ECO. This is okay.

@vogelpi
Copy link
Contributor

vogelpi commented Sep 19, 2024

@andreaskurth this PR currently fails lint:

E   RESET_ONLY:   pinmux.sv:170   'mio_pad_attr_q[0]' is asynchronously reset but has no other assignments in this block

Can you please update it once more?

@andreaskurth
Copy link
Contributor Author

@andreaskurth this PR currently fails lint:

E   RESET_ONLY:   pinmux.sv:170   'mio_pad_attr_q[0]' is asynchronously reset but has no other assignments in this block

Can you please update it once more?

Thx @vogelpi, as we just discussed this gets waived

@andreaskurth
Copy link
Contributor Author

andreaskurth commented Sep 19, 2024 •
edited
Loading

CHANGE AUTHORIZED: hw/ip/pinmux/data/pinmux.hjson.tpl
CHANGE AUTHORIZED: hw/ip/pinmux/rtl/pinmux.sv

This PR touches one single RTL file and an Hjson template to implement a confirmed ECO. This is okay.

@andreaskurth
Copy link
Contributor Author

The chip_csr_hw_reset TLT fails because we changed the reset value of one register but the DV environment isn't aware of this. To be fixed.

@andreaskurth
Copy link
Contributor Author

andreaskurth commented Sep 23, 2024 •
edited
Loading

We need to move forward with this change, so the automated test that fails because the DV env isn't aware of the changed reset value will have to be excluded for the moment. I created issue #24621 to track this.

@andreaskurth
[top_earlgrey,pinmux] Enable pull-down for TAP strap 0 at reset
17dcc1d 
TAP strap 0 is sampled after reset (and only once for life cycle states
that are not TEST_UNLOCKED* or RMA).  To ensure it gets sampled as 0
unless driven to 1 from an external source (and specifically that it
gets sampled as 0 when left floating / not connected), this enables the
pull-down of the pad at reset.

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

vogelpi commented Sep 23, 2024

CHANGE AUTHORIZED: hw/ip/pinmux/data/pinmux.hjson.tpl

@andreaskurth andreaskurth merged commit 78ad89d into lowRISC:master Sep 23, 2024
38 checks passed
@andreaskurth andreaskurth deleted the ioc8-pull-down-at-reset branch September 23, 2024 10:56
@andreaskurth andreaskurth mentioned this pull request Sep 23, 2024
Merged
@vogelpi vogelpi added the ECO Accepted as an ECO label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-will a-will a-will left review comments

@vogelpi vogelpi vogelpi approved these changes

@rswarbrick rswarbrick Awaiting requested review from rswarbrick

@meisnere meisnere Awaiting requested review from meisnere

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

Assignees

@andreaskurth andreaskurth

Labels
ECO Accepted as an ECO IP:pinmux TOP:earlgrey
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andreaskurth @vogelpi @a-will