[pinmux/padctrl] First round of (mostly CSR facing) pinmux updates #5128

msfschaffner · 2021-02-05T23:29:16Z

Note that this is a WIP PR and there are still several things I need to change.

Putting this up to enable early feedback only.

msfschaffner · 2021-02-05T23:39:50Z

Please do not review just yet - I will ping you once this is (sort of) stable.

hw/top_earlgrey/rtl/autogen/top_earlgrey.sv

hw/top_earlgrey/data/top_earlgrey.sv.tpl

hw/top_earlgrey/rtl/top_earlgrey_asic.sv

hw/ip/pinmux/data/pinmux.hjson.tpl

tjaychen · 2021-02-08T19:48:03Z

hw/ip/pinmux/rtl/pinmux.sv

+  logic [NDioPads-1:0] dio_out_retreg_q, dio_oe_retreg_q;
+
+  // Sleep entry trigger
+  assign sleep_trig = sleep_en_i & ~sleep_en_q;


can you file an issue on me to figure out exactly which pwrmgr signal to connect? There isn't anything existing that fits..because the first thing pwrmgr does is shut off the clocks, which would mess up this logic unless you want to put it on the clk_aon_i domain.

jep, see here: #5198

hw/ip/pinmux/rtl/pinmux.sv

msfschaffner · 2021-02-12T02:40:17Z

I'll double check - however apart from the compact/noncompact multireg change, the pinmux configuration should not have to be different after this change (there are no changes to the #MIOs and pinout in this PR). I've adapted the pinmux initialization function to reflect the compact -> noncompact change.

msfschaffner · 2021-02-12T03:13:24Z

Good call @tjaychen!
Looks like I optimized too much away when I removed the IO_POK signals:

- assign mio_data_mux = AlignedMuxSize'({(&io_pok_i) ? mio_in_i : '0, 1'b1, 1'b0});
+ assign mio_data_mux = AlignedMuxSize'(mio_in_i);

Find the error ;)...

tjaychen · 2021-02-12T06:04:54Z

ahhhh.. .the whole thing shifted down by 2? yeah that would definitely mess things up :)

…

On Thu, Feb 11, 2021 at 7:13 PM Michael Schaffner ***@***.***> wrote: Good call @tjaychen <https://github.com/tjaychen>! Looks like I optimized too much away when I removed the IO_POK signals: - assign mio_data_mux = AlignedMuxSize'({(&io_pok_i) ? mio_in_i : '0, 1'b1, 1'b0});+ assign mio_data_mux = AlignedMuxSize'(mio_in_i); Find the error ;)... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH2RSSTWKFEYNKDD42CJN3S6SMGDANCNFSM4XFTEOLA> .

tjaychen · 2021-02-12T06:07:21Z

@msfschaffner if you haven't done so already, could you update the spreadsheet containing the memory map to remove padctrl?

msfschaffner · 2021-02-12T06:38:29Z

Sure will do!

If this looks good to you, I will squash most of this into one commit to make this giant thing atomic.

This also aligns the pinmux initialization code accordingly. I.e., due to the multibit REGWENs, the pinmux insel/outsel CSRs have to be non-compact, and therefore the initialization function needs to be adapted for this to work. Signed-off-by: Michael Schaffner <[email protected]>

In particular, this adds a per pad sleep status bit that stays asserted when coming back from deep sleep until SW clears it. The sleep behavior of the corresponding pad will only be disabled if SW clears that status bit. Signed-off-by: Michael Schaffner <[email protected]>

Note that hw/ip/padctrl still contains some legacy documentation material and design sources that need to be moved or merged with the pinmux docs. This will be done in a separate PR. Signed-off-by: Michael Schaffner <[email protected]>

Signed-off-by: Michael Schaffner <[email protected]>

msfschaffner · 2021-02-12T21:17:25Z

The latest force push squashes some of the commits together to make them atomic.

tjaychen · 2021-02-12T21:21:28Z

it looks good to me. sorry i forgot to respond earlier.

…

On Fri, Feb 12, 2021 at 1:17 PM Michael Schaffner ***@***.***> wrote: The latest force push squashes some of the commits together to make them atomic. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH2RSVV3SPZ54UOPVUKQ7TS6WLHHANCNFSM4XFTEOLA> .

msfschaffner · 2021-02-12T21:24:30Z

No worries. Can you push the green button ;)?

tjaychen · 2021-02-12T21:47:54Z

hw/ip/pinmux/rtl/pinmux_wkup.sv

        HighTimed: begin
          aon_cnt_en = aon_filter_out_d;
          aon_wkup_pulse = aon_cnt_eq_th;
        end
-        default: ; // also covers "Disabled"
+        LowTimed: begin


i forgot to ask this earlier. but if someone didn't want any debounce, and were okay with this anytime this signal is high or low, would they just a really low threshold count? (0 or 1?)

The pulse counter is separate from the debouncing logic.
In fact, we've got a prim_filter in each wakeup detector, which can be enabled or desabled via a CSR (see separate comment).

o sorry, i should correct myself then. I guess I meant if someone wanted any length of a pulse to trigger.
Would they basically set this to 1?

Yeah that's right, you could do that or just use the edge detector feature instead.

msfschaffner · 2021-02-12T21:57:11Z

hw/ip/pinmux/data/pinmux.hjson.tpl

                            of always-on clock cycles as configured in !!WKUP_DETECTOR_CNT_TH.
                            '''
                        },
+
                      ]
                    }
                    { bits: "3",


FYI: @tjaychen this is the register I have been talking about in the other comment

msfschaffner · 2021-02-12T21:58:54Z

hw/ip/pinmux/rtl/pinmux_wkup.sv

@@ -51,7 +51,7 @@ module pinmux_wkup import pinmux_pkg::*; import pinmux_reg_pkg::*; #(
  always_ff @(posedge clk_aon_i or negedge rst_aon_ni) begin : p_sync
    if (!rst_aon_ni) begin
      aon_wkup_en_q     <= 1'b0;
-      aon_wkup_mode_q   <= Disabled;
+      aon_wkup_mode_q   <= Posedge;
      aon_filter_en_q   <= 1'b0;
      aon_wkup_cnt_th_q <= '0;
    end else begin


FYI @tjaychen: see on line 75ff below

msfschaffner · 2021-02-12T22:22:08Z

Thanks for the review, pulling this in now!

This PR implements the DV support for PR lowRISC#5128, which allows an enable register to be a multi-reg. This enable register will have many fields, each field locks a certain set of registers. This PR support it by moving the `locked_reg_q` to dv_base_reg_field, and copied enable register related methods to dv_base_reg_field. If the enable register has only one field, normal methods still work (which means we do not need to change current testbench). If the enable register has more than one field, CSR automation test will work, but if user want to access the `locked_reg_q`, they have to call from dv_base_reg_field rather than dv_base_reg. This PR also updated the UVM RAL register generate script. Signed-off-by: Cindy Chen <[email protected]>

This PR implements the DV support for PR #5128, which allows an enable register to be a multi-reg. This enable register will have many fields, each field locks a certain set of registers. This PR support it by moving the `locked_reg_q` to dv_base_reg_field, and copied enable register related methods to dv_base_reg_field. If the enable register has only one field, normal methods still work (which means we do not need to change current testbench). If the enable register has more than one field, CSR automation test will work, but if user want to access the `locked_reg_q`, they have to call from dv_base_reg_field rather than dv_base_reg. This PR also updated the UVM RAL register generate script. Signed-off-by: Cindy Chen <[email protected]>

msfschaffner added the Component:RTL label Feb 5, 2021

msfschaffner requested a review from tjaychen February 5, 2021 23:29

msfschaffner self-assigned this Feb 5, 2021

msfschaffner requested a review from eunchan February 5, 2021 23:29

msfschaffner force-pushed the pinmux-updates branch 2 times, most recently from 8a99adf to a1459fc Compare February 6, 2021 02:44

msfschaffner requested review from cindychip, weicaiyang and sriyerg February 6, 2021 02:45

msfschaffner force-pushed the pinmux-updates branch 4 times, most recently from 6e0e841 to 95e393e Compare February 6, 2021 03:41

cindychip reviewed Feb 8, 2021

View reviewed changes

hw/top_earlgrey/rtl/autogen/top_earlgrey.sv Outdated Show resolved Hide resolved

cindychip reviewed Feb 8, 2021

View reviewed changes

hw/top_earlgrey/data/top_earlgrey.sv.tpl Show resolved Hide resolved

msfschaffner force-pushed the pinmux-updates branch from 95e393e to 1070f46 Compare February 8, 2021 19:04

cindychip reviewed Feb 8, 2021

View reviewed changes

hw/top_earlgrey/rtl/top_earlgrey_asic.sv Show resolved Hide resolved

tjaychen reviewed Feb 8, 2021

View reviewed changes

hw/ip/pinmux/data/pinmux.hjson.tpl Outdated Show resolved Hide resolved

tjaychen reviewed Feb 8, 2021

View reviewed changes

hw/ip/pinmux/data/pinmux.hjson.tpl Show resolved Hide resolved

tjaychen reviewed Feb 8, 2021

View reviewed changes

hw/ip/pinmux/data/pinmux.hjson.tpl Show resolved Hide resolved

tjaychen reviewed Feb 8, 2021

View reviewed changes

hw/ip/pinmux/rtl/pinmux.sv Outdated Show resolved Hide resolved

cindychip mentioned this pull request Feb 9, 2021

[dv/enable_regs] Support enable registers have more than one field #5150

Merged

msfschaffner mentioned this pull request Feb 10, 2021

[pwrmgr] Figure out which signal to connect to the pinmux sleep enable input #5198

Closed

msfschaffner force-pushed the pinmux-updates branch from 658b899 to c6a0f23 Compare February 12, 2021 03:11

msfschaffner added 5 commits February 12, 2021 13:14

[pinmux] Remove IO pok signal as it will be merged into the padring

aff1f1b

Signed-off-by: Michael Schaffner <[email protected]>

[pinmux] Rework wakeup detectors

7aec627

Signed-off-by: Michael Schaffner <[email protected]>

msfschaffner force-pushed the pinmux-updates branch from c6a0f23 to 7aec627 Compare February 12, 2021 21:15

tjaychen reviewed Feb 12, 2021

View reviewed changes

tjaychen approved these changes Feb 12, 2021

View reviewed changes

msfschaffner commented Feb 12, 2021

View reviewed changes

msfschaffner merged commit 390251b into lowRISC:master Feb 12, 2021

msfschaffner deleted the pinmux-updates branch February 18, 2022 00:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pinmux/padctrl] First round of (mostly CSR facing) pinmux updates #5128

[pinmux/padctrl] First round of (mostly CSR facing) pinmux updates #5128

msfschaffner commented Feb 5, 2021

msfschaffner commented Feb 5, 2021

tjaychen Feb 8, 2021

msfschaffner Feb 10, 2021

msfschaffner commented Feb 12, 2021

msfschaffner commented Feb 12, 2021

tjaychen commented Feb 12, 2021 via email

tjaychen commented Feb 12, 2021

msfschaffner commented Feb 12, 2021

msfschaffner commented Feb 12, 2021

tjaychen commented Feb 12, 2021 via email

msfschaffner commented Feb 12, 2021

tjaychen Feb 12, 2021

msfschaffner Feb 12, 2021

tjaychen Feb 12, 2021

msfschaffner Feb 13, 2021

msfschaffner Feb 12, 2021

msfschaffner Feb 12, 2021

msfschaffner commented Feb 12, 2021

[pinmux/padctrl] First round of (mostly CSR facing) pinmux updates #5128

[pinmux/padctrl] First round of (mostly CSR facing) pinmux updates #5128

Conversation

msfschaffner commented Feb 5, 2021

msfschaffner commented Feb 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msfschaffner commented Feb 12, 2021

msfschaffner commented Feb 12, 2021

tjaychen commented Feb 12, 2021 via email

tjaychen commented Feb 12, 2021

msfschaffner commented Feb 12, 2021

msfschaffner commented Feb 12, 2021

tjaychen commented Feb 12, 2021 via email

msfschaffner commented Feb 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msfschaffner commented Feb 12, 2021