Bug 8 - Pinmux needed
Summary: Pinmux needed
Status: CONFIRMED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Specification (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- enhancement
Assignee: Luke Kenneth Casson Leighton
URL:
Depends on: 24 25 26 27 30 31 33 34 35 50 20 28
Blocks: 2
  Show dependency treegraph
 
Reported: 2018-02-26 01:28 GMT by Luke Kenneth Casson Leighton
Modified: 2020-06-25 15:32 BST (History)
1 user (show)

See Also:
NLnet milestone: NLnet.2019.02
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for completion of task (excludes budget allocated to subtasks): 0
parent task for budget allocation:
child tasks for budget allocation:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2018-02-26 01:28:10 GMT
http://libre-riscv.org/shakti/m_class/pinmux/
Comment 1 Luke Kenneth Casson Leighton 2018-03-11 07:01:50 GMT
https://bitbucket.org/casl/pinmux.git - first implementation of an
auto-generator which takes a formal spec and generates BSV which
generates verilog
Comment 2 Luke Kenneth Casson Leighton 2018-03-11 07:29:27 GMT
the general consensus appears to be that rather than complicate
the routing by allowing arbitrary register setting and then
preventing and prohibiting certain combinations from actually
being allowed, instead the pinmux shall be greatly simplified,
and the REGISTERS prevented and prohibited from being set in
ways that would result in a short-circuit.

the only thing is that certain pins can be changed to inputs
under the influence / control of the *function* (e.g. SD-DAT0-3
can be changed to output or input).  so when writing a
"prohibitor" state-machine that stops certain inputs from
being routed from multiple I/O pads, these functions
(SD-DAT0-3, ULPI data lines, FlexBus data lines, ONFI
data lines etc. etc.) all need to also be on the exclusion
list as well.
Comment 3 Luke Kenneth Casson Leighton 2018-03-31 10:53:58 BST
TODO: add auto-generator for many-to-one signal enablers (inout)

import BUtils::*;

Bit#(1) enable;
Bit#(N) many_enable = duplicate(enable);

To declare:
 Wire#(Bit#(1)) enable <- mkDWire(0);  // wire will default value of 0

 or just

Bit#(1) enable = 0; // need to give a default value

to assign:

rule assign_enable;
{sd_d0_en, sd_d1_en, sd_d2_en, sd_d3_en} = duplicate(enable);
endrule

or 

{sd_d0_en, sd_d1_en, sd_d2_en, sd_d3_en} = duplicate(enable);
Comment 4 Luke Kenneth Casson Leighton 2018-04-14 11:44:14 BST
Most of the inputs are declared as 2 bits:

  method  Action cell23_mux (Bit#(2) in);

  Wire#(Bit#(2)) wrcell23_mux<-mkDWire(0);

But some of the inputs are declared as 1 bit:

  method  Action cell24_mux (Bit#(1) in);

  Wire#(Bit#(1)) wrcell24_mux<-mkDWire(0);

This is OK when the value is only ever compared to 0 or 1.  It becomes a problem when (for a dozen or so inputs) you try to compare it against a value that is larger than 1 bit:

  if(wrcell24_mux==2)

I don't know what the code is doing, so I can't say what the right fix is.  One option is to change the size to be 2 bits, by changing "#(1)" to "#(2)" (in three places).  Alternatively, you can delete all the if-statements that compare to a value greater than 1.  (That's what I did, I deleted code, just to get it to compile.)

The next issue I ran into is that the rule "assign_inputs_from_io_to_wires" has a number of if-statements which write to the same wire:

  if(wrcell87_mux==3)
    wreint_18_in<=cell87_mux_in;

  if(wrcell116_mux==3)
    wreint_18_in<=cell116_mux_in;

Since the compiler doesn't know whether these conditions are exclusive, it must assume that both can be executed, but the state element can only be written with one value, and so this is an error.

Again, the fix depends on what the code is supposed to do.  You can either remove one of the write statements.  Or you can change the conditions so that only one write will happen.  For example:

  if(wrcell87_mux==3)
    wreint_18_in<=cell87_mux_in;

  if ((wrcell116_mux==3) && (wrcell87_mux!=3))
    wreint_18_in<=cell116_mux_in;

Here, I've changed the condition on the second write, so that it won't happen if the first write happens.

Unfortunately, there are 178 such duplicates, so I can't easily make this change manually.
Comment 5 Luke Kenneth Casson Leighton 2018-06-13 15:28:15 BST
ok so the assignment to GenericIOType is slightly problematic as
a change in the muxer also changes the IOpad (from input to output):

      /*====== This where the muxing starts for each io-cell======*/
      // output muxer for cell idx 0
      cell0_mux_out=wrcell0_mux==0?uart0_tx_io:
            wrcell0_mux==1?spi0_sclk_io:
            wrcell0_mux==2?uart2_tx_io:
            uart3_tx_io;

this is Bad because a pin may need to be declared as an input
so as not to damage what it is connected to.

also, it's unprecedented that changing the pin mux *also* changes
the type at the same time.  as in, there isn't a pinmux on any
commercial SoC that has this behaviour: they *all* explicitly have
registers that change the type, change the drivestrength, change
the pullup/pulldown *at the pad*, not the function... with some
exceptions: sometimes the pinmux is a bit more complicated in
that if the pad is set to a particular function the pullup/pulldown
and pushpull/opendrain and drivestrength are also set (overridden),
whereas if it's set to a general-purpose IO the settings are instead
taken from the registers.

this however is a bit complex as it is necessary to have a full spec
for absolutely every single function, hard-encoded right now in the
hardware.  get it wrong and it's unfixable.
Comment 6 Luke Kenneth Casson Leighton 2018-06-18 12:32:29 BST
TODO: set slew rate, set sink current, set impedance?
Comment 7 Luke Kenneth Casson Leighton 2020-06-25 15:21:25 BST
looks like the LowRISC team have been copying my ideas, based on public
discussions on common mailing lists from 2 years ago:

https://docs.opentitan.org/hw/ip/pinmux/doc/
https://github.com/lowRISC/opentitan/blob/master/hw/top_earlgrey/ip/pinmux/data/autogen/pinmux.hjson
Comment 8 Luke Kenneth Casson Leighton 2020-06-25 15:26:04 BST
yep: they've definitely been copying my work, most likely after the announcement
i made about the Shakti Bluespec work:

https://github.com/lowRISC/opentitan/blob/master/hw/top_earlgrey/data/top_earlgrey.hjson

these are the "specification" files that define the peripherals.  there is
nothing - literally nothing - that is hard-coded.  buses, addresses, IRQs:
*everything* is auto-generated from these specifications.
Comment 9 Luke Kenneth Casson Leighton 2020-06-25 15:32:53 BST
this is *one* of the template files into which the information from the
JSON files is substituted:

https://github.com/lowRISC/opentitan/blob/master/hw/top_earlgrey/data/top_earlgrey.sv.tpl

this one happens to generate a verilog "master peripheral suite" containing
all information sufficient to link together *all* peripherals, *all* interrupt
sources, *all* Buses - everything needed for a SoC.

additional templates (in that same directory) such as this:
https://github.com/lowRISC/opentitan/blob/master/hw/top_earlgrey/data/top_earlgrey.h.tpl

contain the exact same information needed for u-boot, linux kernel and RTOSes
to know, when compiled as c code, exactly where the peripheral memory-maps
start, exactly what the GPIO does and so on.