On Fri, Apr 26, 2019 at 11:25 AM Rishabh Jain <rishucoding@gmail.com> wrote:
>
> hi luke,
>
> i am having difficulty in understanding nmigen Record (hdl/rec.py)
> i am able to  follow some part of hdl/rec.py like Direction, fields,
> __init__ in class Layout checking
> for valid field, are the elements of field valid, avoiding duplication of
> names.
> but, i am not able to understand the full picture :(

 in some ways, you don't actually need to read all the code, just know
 the function names and read the docstrings.  except (*sigh*) Record
 and Layout don't *have* any docstrings... *sigh*...


> can you give me some pointers over what is layout, shape?

 Layout is just a list of signal-width plus signal-Direction.
 (or signal-width plus signed/unsigned plus Direction).

 shape is a tuple of signal-width plus if the signal is signed or unsigned.


> from our diagram of GPIO  (https://libre-riscv.org/shakti/m_class/pinmux/)
> the i/o pad is a PHYSICAL PIN of processor which connects it to
> peripherals, memory, etc, right?
> so, we  mux (select one of many) inputs of processor coming to i/o pad from
> various peripherals and mux outputs of
> processor coming to i/o pad going to peripherals.
> I am not able to relate FAN_OUT  and FAN_IN with this diagram :(

FAN_OUT and FAN_IN are actually Muxers, nothing to do with Record/Layout...
there is an example somewhere of a multi-selection Mux...

https://github.com/m-labs/nmigen/blob/master/examples/pmux.py

ah, it's not quite the same.  actually, FAN_IN and FAN_OUT would be:

 with m.Switch(mux):
    with m.Case(0):
       m.d.comb += self.o.eq(self.a)
    with m.Case(1):
       m.d.comb += self.o.eq(self.b)

that would be one of them and the other would be:

 with m.Switch(mux):
    with m.Case(0):
       m.d.comb += self.a.eq(self.o)
    with m.Case(1):
       m.d.comb += self.b.eq(self.o)

you'll have to work out which is which :)


actually it turns out that doing this is not necessary, you can use an Array:

signals = [self.a, self.b, self.c, self.d]
array = Array(signals)

m.d.comb += self.o.eq(array[self.muxselector])

or for the other one:

m.d.comb += array[self.muxselector].eq(self.o)


so instead of a massive list of Switch/Case statements, just use Array.

https://libera.irclog.whitequark.org/nmigen/2021-10-06#30958250;

nmigen-boards and nmigen-soc has the resources needed.

https://github.com/nmigen/nmigen-soc/blob/master/nmigen_soc/test/

the wishbone test shows how peripherals can be added at specific
address ranges.

https://github.com/ktemkin/lambdasoc/blob/master/examples/sram_soc.py

this is what needs autogenerating.

instead of explicit instances of sram, serial, etc, pinmux enumeration
creates pinsets, instantiates the peripheral and adds it, on-demand.

(In reply to Luke Kenneth Casson Leighton from comment #4)
> https://github.com/ktemkin/lambdasoc/blob/master/examples/sram_soc.py
> 
> this is what needs autogenerating.
> 
> instead of explicit instances of sram, serial, etc, pinmux enumeration
> creates pinsets, instantiates the peripheral and adds it, on-demand.

this:

https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/i_class.py;hb=HEAD#l63

  63     ps = PinSpec(pinbanks, fixedpins, function_names,
  64                  {'lcd': {'bus': 'fastbus',
  65                           'mmap': [['Cfg', 0x20000, 10]
  66                                    ]},
  67                   'jtag': {'bus': 'fastbus'},
  68                   'fb': {'bus': 'fastbus'},
  69                   'sdr': {'bus': 'fastbus',
  70                           'mmap': [['Mem', 0x70000000, 0x400000],
  71                                    ['Cfg', 0x17000, 12]
  72                                    ]},
  73                   })

turns into this:

        self._arbiter = wishbone.Arbiter(addr_width=30, data_width=32,
                                         granularity=8,
                                         features={"cti", "bte"})
        self._decoder = wishbone.Decoder(addr_width=30, data_width=32,
                                         granularity=8,
                                         features={"cti", "bte"})

        self.cpu = MinervaCPU(reset_address=reset_addr)
        self._arbiter.add(self.cpu.ibus)
        self._arbiter.add(self.cpu.dbus)

        self.rom = SRAMPeripheral(size=rom_size, writable=False)
        self._decoder.add(self.rom.bus, addr=rom_addr)

        self.ram = SRAMPeripheral(size=ram_size)
        self._decoder.add(self.ram.bus, addr=ram_addr)

        self.uart = AsyncSerialPeripheral(divisor=uart_divisor, pins=uart_pins)
        self._decoder.add(self.uart.bus, addr=uart_addr)


it's actually incredibly simple.  once, of course, the underlying
infrastructure is actually there.

useful diagram:
https://ftp.libre-soc.org/course_18oct2021/photo4.jpg

platform files such as this:

https://github.com/nmigen/nmigen-boards/blob/master/nmigen_boards/arty_a7.py

class _ArtyA7Platform(Xilinx7SeriesPlatform):
    package     = "csg324"
    speed       = "1L"
    default_clk = "clk100"
    default_rst = "rst"
    resources   = [
        Resource("clk100", 0, Pins("E3", dir="i"),
                 Clock(100e6), Attrs(IOSTANDARD="LVCMOS33")),
        Resource("rst", 0, PinsN("C2", dir="i"), Attrs(IOSTANDARD="LVCMOS33")),
        *LEDResources(pins="H5 J5 T9 T10", attrs=Attrs(IOSTANDARD="LVCMOS33")),
        RGBLEDResource(0, r="G6", g="F6", b="E1", 
                       attrs=Attrs(IOSTANDARD="LVCMOS33")),

would become:

class NGIPointerPlatform(DynamicASICPlatform):
     ....

where DynamicASICPlatform would, instead of explicitly adding
resources one by one, do this:

txt = open(JSON_FILE_WITH_PINSPECS).readlines()

for line in txt:
     if line.resourcename == 'JTAG':
           resources.append(JTAGResource("jtag", 0, Pins(line.tck/tms/tdi/tdo)

etc. etc.

again this is very straightforward.

where it will get complicated is how the hell to add JTAG Boundary Scan.
for litex, a *duplicate* ResourceManager was required.

https://git.libre-soc.org/?p=nmigen.git;a=blob;f=nmigen/build/res.py;h=fde981fcf3ad4b7427de8ac9ee2c62a1598721f0;hb=e88d283ed30448ed5fe3ba264e3e56b48f2a4982#l17

* one ResourceManager is for the actual inputs/outputs. it connects between:
  - ASIC corona (actual IOpads)
  - JTAG
* one ResourceManager is for the actual peripherals.  it connects between:
  - the peripheral(s) - UART, I2C, SPI, SDRAM
  - JTAG

PlatformManager *derives* from ResourceManager already, so that is one
covered:

https://git.libre-soc.org/?p=nmigen.git;a=blob;f=nmigen/build/plat.py;h=c1d8fc693c0c180d56f319433889b10125ee573e;hb=e88d283ed30448ed5fe3ba264e3e56b48f2a4982#l21

however to "redirect", it may.... sigh... be necessary for the
DynamicASICPlatform to override get_input, get_output etc. etc.

https://git.libre-soc.org/?p=nmigen.git;a=blob;f=nmigen/vendor/xilinx.py;h=f332a93230fd3c853e93322f6c11207e0181d94b;hb=e88d283ed30448ed5fe3ba264e3e56b48f2a4982

this can be done with a Mix-In class because, sigh, FPGA testing of
JTAG Boundary Scan is also necessary.

(and, this would also, in some future version, be the place where
the GPIO mux redirection also is added, re-mapping / redirecting
from one-to-many and many-to-one - many peripheral pins to one IO pin)

the DynamicASICPlatform would in no way be as complicated as
Vivado etc. etc.  in fact most of the code is deleted.  the actual
platform should end up as trivial as this:

https://git.libre-soc.org/?p=libresoc-litex.git;a=blob;f=libresoc/ls180.py;h=81858edd9ad026ae05187ed6c48d318a7fe60647;hb=b55917aafa6bbc9f16e1d97dc095e929c31aa81a#l171

def build - outputs verilog.

aside from the add_input, add_output etc. etc. overrides which will
redirect to the Corona Platform Resource:

    def get_input(self, pin, port, attrs, invert):
        self._check_feature("single-ended input", pin, attrs,
                            valid_xdrs=(0,), valid_attrs=None)

        m = Module()
        m.d.comb += pin.i.eq(self._invert_if(invert, port))
        return m

becomes:


    def get_input(self, pin, port, attrs, invert):
        self._check_feature("single-ended input", pin, attrs,
                            valid_xdrs=(0,), valid_attrs=None)

        m = Module()
        corona_pin = self.get_corona_pin(pin, port, attrs, invert)
        jtag_in = self.jtag.get_core_side(pi, port, attrs, invert)
        jtag_out = self.jtag.get_core_side(pi, port, attrs, invert)

        # instead of this:
        # m.d.comb += pin.i.eq(self._invert_if(invert, port))
        # put JTAG in the middle:

        m.d.comb += jtag_in.eq(self._invert_if(invert, port))
        m.d.comb += pin.i.eq(jtag_out)

        return m

likewise for output, and etc. etc. etc. etc.

i have cut it back somewhat, but this is how to create a platform.
Blinker is dome sort of top-level system (module) that, internally,
will have done this:

For example, by including this Resource into the platform's resources list:

    Resource("abc", 0, Pins("J3", dir="i"))

then pin J3 on the device will be configured as an input, and you can request the abc resource's input Signal like this:

    platform.request("abc").i

(and then assign to it)



from nmigen.build import Platform, Resource, Pins, Clock, Attrs, Connector
from nmigen.build.run import LocalBuildProducts
from nmigen.cli import main_parser, main_runner
from nmigen.vendor.lattice_ice40 import LatticeICE40Platform

class ICE40HX8KBEVNPlatform(LatticeICE40Platform):
    device = "iCE40HX8K"
    package = "CT256"

    resources = [
        Resource("clk1", 0, Pins("J3", dir="i"), Clock(12e6),
                 Attrs(GLOBAL=True, IO_STANDARD="SB_LVCMOS")),  # GBIN6
        Resource("rst", 0, Pins("R9", dir="i"),
                 Attrs(GLOBAL=True, IO_STANDARD="SB_LVCMOS")),  # GBIN5
        Resource("led", 0, Pins("C3", dir="o"),
                 Attrs(IO_STANDARD="SB_LVCMOS")),  # LED2
    ]

    default_clk = "clk1"  # you must have a default clock resource
    default_rst = "rst"   # you must have a default reset resource

    def toolchain_program(self, products: LocalBuildProducts, name: str):
        iceprog = os.environ.get("ICEPROG", "iceprog")
        with products.extract("{}.bin".format(name)) as bitstream_filename:
            subprocess.check_call([iceprog, "-S", bitstream_filename])

if __name__ == "__main__":
    ICE40HX8KBEVNPlatform().build(Blinker(), do_program=True)


so, basically, we do not just create a module and blat its output to a file,
we create an *ASICPlatform*, hand the module to that platform, and ask the
platform to "build" the pinconnections, which will include setting up JTAG
Boundary connectivity.

https://libre-soc.org/irclog/%23libre-soc.2021-11-12.log.html#t2021-11-12T20:24:16

found this:

https://github.com/nmigen/nmigen-boards/blob/master/nmigen_boards/resources/interface.py#L10

which can be used to create a Resource Suite from this:

  26 def dummy_pinset():
  27     # sigh this needs to come from pinmux.
  28     gpios = []
  29     for i in range(16):
  30         gpios.append("%d*" % i)
  31     return {'uart': ['tx+', 'rx-'],
  32              'gpio': gpios,
  33              'i2c': ['sda*', 'scl+']}

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/jtag.py;h=4b4f17a97d672c3f22b668008a1c058ac2500da6;hb=HEAD#l26

diagram explaining JTAG redirection:
https://libre-soc.org/shakti/m_class/JTAG/jtag-block.jpg

3 phases needed here:

* phase 1 - simple dummy_pinset Resource list

a function is needed which turns the results of dummy_pinset()
into:

[UARTResource("uart", 0, tx=..., rx=..),
 I2CResource("i2c", 0, scl=..., sda=...),
 Resource("gpio", 0, Subsignal("i"...), Subsignal("o"...)
 Resource("gpio", 1, Subsignal("i"...), Subsignal("o"...)
 ...
]

and to create a Platform instance with that list, and build
something random

   p=Platform()
   p.resources=listofstuff
   p.build(Blinker())

* phase 2 - read an arbitrary pinset (from a JSON file)

will edit tomorrow

* phase 3 - finally create an ASICPlatform with JTAG scan.



phase 1 should literally be no more than 20 lines of code.

phase 2 is a little more involved, some Resource peripherals
need to be created, wrapping class Pin and creating Resource
lists

phase 3 is quite straightforward.

more tomorrow

(In reply to Luke Kenneth Casson Leighton from comment #10)
> diagram explaining JTAG redirection:
> https://libre-soc.org/shakti/m_class/JTAG/jtag-block.jpg
> 
> 3 phases needed here:
> 
> * phase 1 - simple dummy_pinset Resource list
> 
> a function is needed which turns the results of dummy_pinset()
> into:
> 
> [UARTResource("uart", 0, tx=..., rx=..),
>  I2CResource("i2c", 0, scl=..., sda=...),
>  Resource("gpio", 0, Subsignal("i"...), Subsignal("o"...)
>  Resource("gpio", 1, Subsignal("i"...), Subsignal("o"...)
>  ...
> ]

I created a file "testing_stage1.py" here:
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=39690b52aa97cc116d73eafd0e2b5e7c8de0beb9;hb=8cb658e7ba3174d976a900065b1e62fcd49de61c

The file extends the dummy function you mentioned in comment #9

> and to create a Platform instance with that list, and build
> something random
> 
>    p=Platform()
>    p.resources=listofstuff
>    p.build(Blinker())

Haven't started yet.

> * phase 2 - read an arbitrary pinset (from a JSON file)
> 
> will edit tomorrow
> 
> * phase 3 - finally create an ASICPlatform with JTAG scan.
> 
> 
> 
> phase 1 should literally be no more than 20 lines of code.
> 
> phase 2 is a little more involved, some Resource peripherals
> need to be created, wrapping class Pin and creating Resource
> lists
> 
> phase 3 is quite straightforward.
> 
> more tomorrow

(In reply to andrey from comment #11)
> (In reply to Luke Kenneth Casson Leighton from comment #10)
> > diagram explaining JTAG redirection:
> > https://libre-soc.org/shakti/m_class/JTAG/jtag-block.jpg
> > 
> > 3 phases needed here:
> > 
> > * phase 1 - simple dummy_pinset Resource list
> > 
> > a function is needed which turns the results of dummy_pinset()
> > into:
> > 
> > [UARTResource("uart", 0, tx=..., rx=..),
> >  I2CResource("i2c", 0, scl=..., sda=...),
> >  Resource("gpio", 0, Subsignal("i"...), Subsignal("o"...)
> >  Resource("gpio", 1, Subsignal("i"...), Subsignal("o"...)
> >  ...
> > ]
> 
> I created a file "testing_stage1.py" here:
> https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;
> h=39690b52aa97cc116d73eafd0e2b5e7c8de0beb9;
> hb=8cb658e7ba3174d976a900065b1e62fcd49de61c
> 
> The file extends the dummy function you mentioned in comment #9

ok, you'll like this: by using that larger function, you increase
the workload required for the follow-on iterative tasks :)

can i suggest now splitting things so that an additional (separate)
task is added which considers using that (much larger) function as
a separate incremental task?

besides, it may turn out that this is the wrong approach, and it would
be better to discover that with much smaller examples than with larger
ones, yeh?

the reason dummy pinset exists was very strategically picked IO:
it covers all 3 types of IOpads.  InType, OutType and InOutType,
in bare minimum.

the only thing it doesn't cover is "banked" IO where one direction
pin covers the direction of multiple pins (as a "bank"). adding sdmmc
would do the trick there.

(In reply to Luke Kenneth Casson Leighton from comment #12)
> (In reply to andrey from comment #11)
> ok, you'll like this: by using that larger function, you increase
> the workload required for the follow-on iterative tasks :)
Hahahahaahah :'(

> 
> can i suggest now splitting things so that an additional (separate)
> task is added which considers using that (much larger) function as
> a separate incremental task?
As in, make "dummy_pinset" a separate task/file?

> 
> besides, it may turn out that this is the wrong approach, and it would
> be better to discover that with much smaller examples than with larger
> ones, yeh?
> 
> the reason dummy pinset exists was very strategically picked IO:
> it covers all 3 types of IOpads.  InType, OutType and InOutType,
> in bare minimum.
> 
> the only thing it doesn't cover is "banked" IO where one direction
> pin covers the direction of multiple pins (as a "bank"). adding sdmmc
> would do the trick there.

Sorry, you lost me again. 

The issue I'm finding is that I have a rough understanding of the abstract (and hardware) problem, but I don't understand the code/explanation. There are many classes/methods in many different files, and I don't know what to focus on. They also look quite esoteric to me, and their purpose isn't obvious.

So was what I wrote in "testing_stage1.py" not at all relevant to stage 1?

i'm tracing code through so you can see what the hell's going on.

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/simple/issuer.py;h=24830149a606a981f08e697e489a658fd7cea892;hb=df423ac698b5acd63260e4751569c7621d950d35#l184

here's where the pinspecs are actually picked up from
the pinmux ls180 output.

 175         if self.jtag_en:
 176             # XXX MUST keep this up-to-date with litex, and
 177             # soc-cocotb-sim, and err.. all needs sorting out, argh
 178             subset = ['uart',
 179                       'mtwi',
 180                       'eint', 'gpio', 'mspi0',
 181                       # 'mspi1', - disabled for now
 182                       # 'pwm', 'sd0', - disabled for now
 183                        'sdr']
 184             self.jtag = JTAG(get_pinspecs(subset=subset),
 185                              domain=self.dbg_domain)

get_pinspecs() returns *exactly* the same format as dummy_pinspecs

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/config/pinouts.py;h=03cfa974b957f654512d8ff204ce1262ce7d632f;hb=df423ac698b5acd63260e4751569c7621d950d35#l28

you can see there it grabs a subset of the JSON file which was
created by the pinmux program

(In reply to andrey from comment #13)
 
> The issue I'm finding is that I have a rough understanding of the abstract
> (and hardware) problem, but I don't understand the code/explanation. There
> are many classes/methods in many different files,

now you know why i set strict coding standards so that you can pull
up 12 xterms on-screen side by side.

> and I don't know what to focus on. 

this is why i said to focus on a tiny task (20 lines).  you then made it 5x larger, hence why you are struggling.  wark :)


> They also look quite esoteric to me, and their purpose isn't
> obvious.

yes, sigh, not enough docstrings due to not enough time.
does not help that external dependencies are the same way.
only reason i can cope is i am used to it.

i also went through the same "err what the hell" thing.
i just hide it.

> So was what I wrote in "testing_stage1.py" not at all relevant to stage 1?

it was... if you want to increase the work needed for stage 1 from 20 lines
to about 100.

suggest this:

1) use dummy_pinset the original
2) enumerate the dict and create Resources using
   format in comment #10 and comment #12
3) create a Platform() and call its build function.

this is literally 20 lines of code.

(In reply to Luke Kenneth Casson Leighton from comment #15)
> (In reply to andrey from comment #13)
> now you know why i set strict coding standards so that you can pull
> up 12 xterms on-screen side by side.
Yeah, currently using terminator terminal viewer to practice this.

> this is why i said to focus on a tiny task (20 lines).  you then made it 5x
> larger, hence why you are struggling.  wark :)
I see, just read the question and clarify if confused... :)

> suggest this:
> 
> 1) use dummy_pinset the original
> 2) enumerate the dict and create Resources using
>    format in comment #10 and comment #12
> 3) create a Platform() and call its build function.
> 
> this is literally 20 lines of code.
I've added a create_resources function which accepts dummy_pinset and instantiated a Platform instance. Actual code about 30 lines.

The code fails during Platform instantiation:

Traceback (most recent call last):
  File "testing_stage1.py", line 75, in <module>
    p=Platform(resources)
TypeError: Can't instantiate abstract class Platform with abstract methods connectors, required_tools, resources, toolchain_prepare

The main test code:
pinset = dummy_pinset()
resources = create_resources(pinset)
p=Platform(resources)
p.resources = create_resources(pinset)
p.build(Blinker())


The code is here:
g/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=00af2ac271a170b54b108acec22e026b9889da5b;hb=HEAD#71

(In reply to andrey from comment #16)

> I've added a create_resources function which accepts dummy_pinset and
> instantiated a Platform instance. Actual code about 30 lines.
> 
> The code fails during Platform instantiation:
> 
> Traceback (most recent call last):
>   File "testing_stage1.py", line 75, in <module>
>     p=Platform(resources)
> TypeError: Can't instantiate abstract class Platform with abstract methods
> connectors, required_tools, resources, toolchain_prepare

oooo how annoying.

ok i'll have a look.

urr that was fun (euphamism: a lot more challenging than i thought)

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=20ff27ba59ad9175082d72342b49871b913cb1d5

ah ha!

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=eab82d3c99e0eb8033f1bfac0951ef02820a4cc7

we have output!

cleaned up the create_resources function a bit, and started experimenting
to see what the heck is going on.  

@@ -78,6 +81,16 @@ class Blinker(Elaboratable):
         m = Module()
         count = Signal(5)
         m.d.comb += count.eq(5)
+        print ("resources", platform.resources.items())
+        gpio = platform.request("gpio", 0)
+        m.d.comb += gpio.gpio0.o.eq(1)

well, that looks pretty straightforward and easy, and the build/top.il
and build/debug.top.v look obvious as well.

okaay things are getting hairy but functional.  first observation/lesson:
under no circumstances declare a pin as "io" type.  this is reserved for
single wires that are bi-directional.

instwad, sigh, io pins have to be declared as a triple of wires: i, o and
oe as *actual* three-wire Resources().

second: the autogenerated names for the pads, ending up as top-level
Signals in the verilog output, are awful :)  but, this can be investigated
later.

i did phase 2 already, it works great, and can be tested/integrated
later.

which means moving to phase 3 already.

andrey you will need to install c4m-jtag for this part of the
experiment, and we will need a cut/pasting a copy of jtag.py
https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/jtag.py;hb=HEAD

use the libresoc c4m-jtag it has DMI in it.

(In reply to Luke Kenneth Casson Leighton from comment #20)
> okaay things are getting hairy but functional.  first observation/lesson:
> under no circumstances declare a pin as "io" type.  this is reserved for
> single wires that are bi-directional.
> 
> instwad, sigh, io pins have to be declared as a triple of wires: i, o and
> oe as *actual* three-wire Resources().
> 
> second: the autogenerated names for the pads, ending up as top-level
> Signals in the verilog output, are awful :)  but, this can be investigated
> later.
> 
> i did phase 2 already, it works great, and can be tested/integrated
> later.
> 
> which means moving to phase 3 already.
> 
> andrey you will need to install c4m-jtag for this part of the
> experiment, and we will need a cut/pasting a copy of jtag.py
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/jtag.py;hb=HEAD
> 
> use the libresoc c4m-jtag it has DMI in it.
Installed nmigen-soc and c4m-jtag from libre-soc.

The latest testing_stage1.py runs with no issues.

Not sure how to convert to IL for visualisation with Yosys.
Tried copying code from jtag.py:
vl = rtlil.convert(dut)
with open("test_jtag.il", "w") as f:
        f.write(vl)

but dut replaced with DummyPlatform in my case.
Got errors about DummyPlatform not being elaborateble (probably because DummyPlatform by itself is not an HDL module, right?)

i copied jtag.py over and removed DMI from it, so that it would work.

things got complicated: an iterator on the pins is only possible *after*
the resources have been requested.

this makes sense in that resources are "in potentia", and pins are
"actual".

what that meant was: i had to put the entire pinset into the DummyPlatform
and *pre-allocate* it.

the reason being: in order to create an absolutely identical pad-set,
and to be able to get that identical pad-pin on a per-core-pin basis,
you of course have to enumerate through them aaaallll and match them
up.... *before* get_input and get_output get called.

argh.

anyway, it works.

(In reply to andrey from comment #21)

> Got errors about DummyPlatform not being elaborateble (probably because
> DummyPlatform by itself is not an HDL module, right?)

(edit) yes

$ python3 src/spec/testing_stage1.py
$ ls -altr build/
total 408
-rw-r--r-- 1 lkcl lkcl 221049 Nov 13 23:05 top.il         <-----
-rw-r--r-- 1 lkcl lkcl 167096 Nov 13 23:05 top.debug.v
-rw-r--r-- 1 lkcl lkcl    122 Nov 13 23:05 build_top.sh
-rw-r--r-- 1 lkcl lkcl    139 Nov 13 23:05 build_top.bat
$

the elaboratable is passed to a build function of
a platform.

platforms are designed to bypass and duplicate the
concept of "make" and Makefiles. this is "For Your
Convenience (tm)", requiring you to be an experienced
python programmer if you want to augment the behaviour.

really valuable set of slides

* https://ftp.libre-soc.org/Pin_Control_Subsystem_Overview.pdf

Veera do you think you could do an SVG of the "My GPIO Block"
from page 13?  put in some colour as well, particularly
the wires to the MUXes (OUTABCD INABCD) and blocks?
(edit: i've added a PNG version here
https://libre-soc.org/docs/pinmux/gpio_block.png)

also these two:

* https://libre-soc.org/shakti/m_class/JTAG/jtag-block.jpg
* https://libre-soc.org/docs/pinmux/CH02-44.gif

make some colour, as well, anything marked TDO in one colour,
TDI a different one.

if you can create a new bugreport, let's put say EUR 350 on these
two diagrams, and link them into a new wiki page
http://libre-soc.org/docs/pinmux/

a great idea by Staf to override Platform.add_resource and Platform.request to
automatically manage JTAG Boundary.

https://libre-soc.org/irclog/%23libre-soc.2021-11-14.log.html#t2021-11-14T10:41:23

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=5800effaf28581e95f0a3cd9e8fd4ace07397a8a

done: hooked into both ResourceManager.request and ResourceManager.add_resources

add_resources now adds to *both* the core *and* pads.
request now also makes the same request of both core and pads,
but then also post-analyses the ports created, and creates
a cross-link between each pad_mgr._ports and each core._ports
entry *after they were requested.  this is slightly complicated.

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=b65a08d04f740bb04db2f4e06728efd1281ff4ec

Andrey i have to say, "very much good luck working this out" :)

there are 4 things: they have to be wired together.  i have
no idea which it is, or which direction: that's down to you
to work out.  i am partially-dsylexic when it comes to these
things.  it could be:

comb += pin.i.eq(io.core.i)
comb += io.pad.i.eq(self._invert_if(invert, port))

it could be like that, it could be completely the other way round.

the 4 things needing connecting are:

* self._invert_if(invert, port)
* pin.i
* io.pad.i
* io.core.i

the existing code is:

    comb += pin.i.eq(self._invert_if(invert, port))

so that already tells you that it is going to be:

       comb += pin.i.eq(SOMETHING1)
and:
       comb += SOMETHING2.eq(self._invert_if(invert, port)

this took 5 months for me to get completely wrong, but that was
because of litex.  hopefully with nmigen you will get "driver
conflict errors" immediately if you get it wrong the first time,
because, here, line 587, the wrong thing will conflict with this

https://git.libre-soc.org/?p=c4m-jtag.git;a=blob;f=c4m/nmigen/jtag/tap.py;h=1f3d424cbd7451c0434e0c71168f5aa0935af860;hb=c2bf4810f9f91ced7fcda777b92b86ab353da288#l587

 586             if conn._iotype == IOType.In:
 587                 m.d.comb += conn.core.i.eq(Mux(bd2core, io_bd[idx], conn.pad.i))

that, actually, should be enough to work out what to do.

once done set_input, set_output can also be done, and that really
should be all that's needed.  no need for the tristate ones.

(In reply to Luke Kenneth Casson Leighton from comment #27)
> https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;
> h=b65a08d04f740bb04db2f4e06728efd1281ff4ec
> 
> Andrey i have to say, "very much good luck working this out" :)
> 
> there are 4 things: they have to be wired together.  i have
> no idea which it is, or which direction: that's down to you
> to work out.  i am partially-dsylexic when it comes to these
> things.  it could be:
> 
> comb += pin.i.eq(io.core.i)
> comb += io.pad.i.eq(self._invert_if(invert, port))
> 
> it could be like that, it could be completely the other way round.
> 
> the 4 things needing connecting are:
> 
> * self._invert_if(invert, port)
> * pin.i
> * io.pad.i
> * io.core.i
> 
> the existing code is:
> 
>     comb += pin.i.eq(self._invert_if(invert, port))
> 
> so that already tells you that it is going to be:
> 
>        comb += pin.i.eq(SOMETHING1)
> and:
>        comb += SOMETHING2.eq(self._invert_if(invert, port)
> 
Before looking into muxes, I tried the following in get_input:
"""
m=Module()
print ("    get_input", pin, "port", port, port.layout)
if pin.name not in ['clk_0', 'rst_0']: # sigh
    (res, pin, port, attrs) = self.padlookup[pin.name]
    io = self.jtag.ios[pin.name]
    m.d.comb += io.pad.i.eq(self._invert_if(invert, port))
    m.d.comb += pin.i.eq(io.pad.i)
    m.d.comb += io.core.i.eq(pin.i)
else: # simple pass-through from port to pin
    print("No JTAG chain in-between")
    m.d.comb += pin.i.eq(self._invert_if(invert, port))
"""
The execution processed all the pins, but I get an AssertionError:
Traceback (most recent call last):
  File "testing_stage1.py", line 319, in <module>
    p.build(top)
  File "/home/rohdo/work/nmigen/nmigen/build/plat.py", line 95, in build
    plan = self.prepare(elaboratable, name, **kwargs)
  File "/home/rohdo/work/nmigen/nmigen/build/plat.py", line 166, in prepare
    fragment._propagate_ports(ports=self.iter_ports(), all_undef_as_ports=False)
  File "/home/rohdo/work/nmigen/nmigen/hdl/ir.py", line 460, in _propagate_ports
    self._prepare_use_def_graph(parent, level, uses, defs, ios, self)
  File "/home/rohdo/work/nmigen/nmigen/hdl/ir.py", line 434, in _prepare_use_def_graph
    subfrag._prepare_use_def_graph(parent, level, uses, defs, ios, top)
  File "/home/rohdo/work/nmigen/nmigen/hdl/ir.py", line 407, in _prepare_use_def_graph
    add_defs(stmt._lhs_signals())
  File "/home/rohdo/work/nmigen/nmigen/hdl/ir.py", line 394, in add_defs
    assert defs[sig] is self
AssertionError

I looked into that file, but don't really understand, I guess it complains about an identical name?

> this took 5 months for me to get completely wrong, but that was
> because of litex.  hopefully with nmigen you will get "driver
> conflict errors" immediately if you get it wrong the first time,
> because, here, line 587, the wrong thing will conflict with this
> 
> https://git.libre-soc.org/?p=c4m-jtag.git;a=blob;f=c4m/nmigen/jtag/tap.py;
> h=1f3d424cbd7451c0434e0c71168f5aa0935af860;
> hb=c2bf4810f9f91ced7fcda777b92b86ab353da288#l587
> 
>  586             if conn._iotype == IOType.In:
>  587                 m.d.comb += conn.core.i.eq(Mux(bd2core, io_bd[idx],
> conn.pad.i))
What signals do I need to use to control the Mux itself?
I need a way to select between pad and jtag boundary scan (or am I over-thinking?)

(In reply to andrey from comment #28)

> What signals do I need to use to control the Mux itself?

you don't.  that's JTAG's job.

> I need a way to select between pad and jtag boundary scan (or am I
> over-thinking?)

yes you are.

this works:

 261             m.d.comb += io.pad.i.eq(self._invert_if(invert, port))
 262             m.d.comb += pin.i.eq(io.core.i)

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=3906235b8a5f46004971e42e75f4fb9c44722b18

(In reply to Luke Kenneth Casson Leighton from comment #29)
> (In reply to andrey from comment #28)
> 
> > What signals do I need to use to control the Mux itself?
> 
> you don't.  that's JTAG's job.
> 
> > I need a way to select between pad and jtag boundary scan (or am I
> > over-thinking?)
> 
> yes you are.
Indeed I am XD

> this works:
> 
>  261             m.d.comb += io.pad.i.eq(self._invert_if(invert, port))
>  262             m.d.comb += pin.i.eq(io.core.i)
> 
> https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;
> h=3906235b8a5f46004971e42e75f4fb9c44722b18
Thanks, tested it and is working.

Extended to get_output, get_tristate, and get_input_output:
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=2adf184193bb11d8a05bc2c0935ece9b3498d8bc;hb=800e9b622f39a06547b6c63c0127135bc8a38bb1

See the new diagram I drew up based on the topology from nmigen's plat.py
https://libre-soc.org/docs/pinmux/
(The I/O and tristate statements were made from it, I recommend checking it for your sanity ;) )


From what I can tell it works, but I'll need further confirmation from Luke to make sure this is heading in the right direction.

(In reply to andrey from comment #30)

> See the new diagram I drew up based on the topology from nmigen's plat.py
> https://libre-soc.org/docs/pinmux/

briilliant. 

> (The I/O and tristate statements were made from it, I recommend checking it
> for your sanity ;) )

:)
 
> From what I can tell it works, but I'll need further confirmation from Luke
> to make sure this is heading in the right direction.

yeah looks great.

i just realised though, that returning $tristate has to go.  $tristate
is exactly the same as if this was a single FPGA endpoint. that is precisely
and exactly what we do not want.

now, in some distant future version we might actually want to output
$IOcell from C4M FlexLib IO package, but not right now, because it is
coriolis2 that is taking care of the IOPad cell allocation, and for
that to work, it has to have the triple signals: i, o and oe.

(In reply to Luke Kenneth Casson Leighton from comment #31)
> (In reply to andrey from comment #30)
> 
> i just realised though, that returning $tristate has to go.  $tristate
> is exactly the same as if this was a single FPGA endpoint. that is precisely
> and exactly what we do not want.
Sure.

> 
> now, in some distant future version we might actually want to output
> $IOcell from C4M FlexLib IO package, but not right now, because it is
> coriolis2 that is taking care of the IOPad cell allocation, and for
> that to work, it has to have the triple signals: i, o and oe.
Makes sense.

I made a naive modification, connecting the pin.i/o/oe straight to port.i/o/oe (or through the JTAG block):
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=7654d177756ae48db4ccf1735f27d735b8341d2d;hb=e4b662d5723dd919ef3c5d1f18c92395e63168dd#l280
"""
if pin.name in ['clk_0', 'rst_0']: # sigh
    print("No JTAG chain in-between")
    # Can port's i/o/oe be accessed like this?
    m.d.comb += port.o.eq(pin.o)
    m.d.comb += port.oe.eq(pin.oe)
    m.d.comb += pin.i.eq(port.i)
    return m
(res, pin, port, attrs) = self.padlookup[pin.name]
io = self.jtag.ios[pin.name]
m.d.comb += io.core.o.eq(pin.o)
m.d.comb += io.core.oe.eq(pin.oe)
m.d.comb += pin.i.eq(io.core.i)
m.d.comb += io.pad.i.eq(port.i)
m.d.comb += port.o.eq(io.pad.o)
m.d.comb += port.oe.eq(io.pad.oe)
"""

However, I'm getting an AttributeError about a missing "i" signal:
Traceback (most recent call last):
  File "testing_stage1.py", line 353, in <module>
    p.build(top)
  File "/home/rohdo/work/nmigen/nmigen/build/plat.py", line 95, in build
    plan = self.prepare(elaboratable, name, **kwargs)
  File "/home/rohdo/work/nmigen/nmigen/build/plat.py", line 152, in prepare
    add_pin_fragment(pin, self.get_tristate(pin, port, attrs, invert))
  File "testing_stage1.py", line 300, in get_tristate
    m.d.comb += pin.i.eq(io.core.i)
  File "/home/rohdo/work/nmigen/nmigen/hdl/rec.py", line 146, in __getattr__
    return self[name]
  File "/home/rohdo/work/nmigen/nmigen/hdl/rec.py", line 158, in __getitem__
    .format(reference, item, ", ".join(self.fields))) from None
AttributeError: Record 'gpio_0__gpio0__oe' does not have a field 'i'. Did you mean one of: o, oe?

Not sure of how to access the port's i,o,oe signals. Looks like there are only "i" OR "o/oe" gpios types only, I guess something hasn't been configured beforehand?

(In reply to andrey from comment #32)

> Not sure of how to access the port's i,o,oe signals. Looks like there are
> only "i" OR "o/oe" gpios types only, I guess something hasn't been
> configured beforehand?

sigh, this is my mistake.  look at how the resource is created: the format
of GPIO is not an i,o,oe block with a type (dir) "io".

i will take a look and see if i can sort it

nggggh i just realised: enumeration of the pad ports was masking the
actual inputs/outputs

    def get_input(self, pin, port, attrs, invert):
        (res, pin, port, attrs) = self.padlookup[pin.name]

should have been:

    def get_input(self, pin, port, attrs, invert):
        (padres, padpin, padport, padattrs) = self.padlookup[pin.name]

ngggggh.  there are *six* pieces of information to wire up.

(In reply to Luke Kenneth Casson Leighton from comment #34)
> nggggh i just realised: enumeration of the pad ports was masking the
> actual inputs/outputs
> 
>     def get_input(self, pin, port, attrs, invert):
>         (res, pin, port, attrs) = self.padlookup[pin.name]
> 
> should have been:
> 
>     def get_input(self, pin, port, attrs, invert):
>         (padres, padpin, padport, padattrs) = self.padlookup[pin.name]
> 
> ngggggh.  there are *six* pieces of information to wire up.

After your explanation about ports here:
https://libre-soc.org/irclog/%23libre-soc.2021-11-16.log.html
I added the changes to get_input_output(). The code runs, and the yosys diagram is quite a bit more complex...
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=f7dc99ca31f912cb0d82c43d769f6db229412873;hb=c49e5c233611122bd5838ba02f255801371c235f#330

Here's the combinatorial code (it looks woozy, but actually quite managable):
# Create aliases for the port sub-signals
port_i = port.io[0]
port_o = port.io[1]
port_oe = port.io[2]
padport_i = padport.io[0]
padport_o = padport.io[1]
padport_oe = padport.io[2]

# Connect SoC pins to SoC port
m.d.comb += pin.i.eq(port_i)
m.d.comb += port_o.eq(pin.o)
m.d.comb += port_oe.eq(pin.oe)
# Connect SoC port to JTAG io.core side 
m.d.comb += port_i.eq(io.core.i)
m.d.comb += io.core.o.eq(port_o)
m.d.comb += io.core.oe.eq(port_oe)
# Connect JTAG io.pad side to pad port
m.d.comb += io.pad.i.eq(padport_i)
m.d.comb += padport_o.eq(io.pad.o)
m.d.comb += padport_oe.eq(io.pad.oe)
# Connect pad port to pad pins
m.d.comb += padport_i.eq(padpin.i)
m.d.comb += padpin.o.eq(padport_o)
m.d.comb += padpin.oe.eq(padport_oe)

Can you confirm this is the behaviour we need Luke?

(In reply to andrey from comment #35)
> (In reply to Luke Kenneth Casson Leighton from comment #34)
> > nggggh i just realised: enumeration of the pad ports was masking the
> > actual inputs/outputs
> > 
> >     def get_input(self, pin, port, attrs, invert):
> >         (res, pin, port, attrs) = self.padlookup[pin.name]
> > 
> > should have been:
> > 
> >     def get_input(self, pin, port, attrs, invert):
> >         (padres, padpin, padport, padattrs) = self.padlookup[pin.name]
> > 
> > ngggggh.  there are *six* pieces of information to wire up.
> 
> After your explanation about ports here:
> https://libre-soc.org/irclog/%23libre-soc.2021-11-16.log.html
> I added the changes to get_input_output(). The code runs, and the yosys
> diagram is quite a bit more complex...

excellent.

> 
> Can you confirm this is the behaviour we need Luke?

not at all! that's a unit test's job.

I've been having issues with getting the tests from soc/debug/test/test_jtag_tap_srv.py working.

My current task is to get the tests to run without significant modifications.

Unfortunately it required adding a lot of misc. files (at least while I test)...
https://git.libre-soc.org/?p=pinmux.git;a=tree;f=src/spec;h=88f78079f80826573ddebbebe7fabaf5137b8425;hb=d6f4066d9aef1f84f1a1e0b885739e57b2e17710

"test_jtag_tap_srv.py" requires the following files: 
from soc/debug/test/:
-dmi_sim
-jtagremote

from soc/debug/:
-dmi.py
-jtag.py
-jtagutils.py

I copied those files into pinmux repo (for now), however I'm getting stumped on a dependency from "dmi.py":
-from soc.config.state import CoreState

Checking out state.py file, I see a nice wildcard import (hahaha):
from openpower.state import *

Further two files I copied from openpower-isa.git/src/openpower/:
-state.py
-sv/svstate.py

The python script test_jtag_tap_srv.py gets further, but I now get an AttributeError in regards to VCDWriter (from nmigen):
Traceback (most recent call last):
  File "test_jtag_tap_srv.py", line 264, in <module>
    with sim.write_vcd("dmi2jtag_test_srv.vcd"):
  File "/usr/lib/python3.7/contextlib.py", line 112, in __enter__
    return next(self.gen)
  File "/home/rohdo/work/nmigen/nmigen/sim/pysim.py", line 330, in write_vcd
    vcd_file=vcd_file, gtkw_file=gtkw_file, traces=traces)
  File "/home/rohdo/work/nmigen/nmigen/sim/pysim.py", line 110, in __init__
    self.vcd_writer.register_alias(
AttributeError: 'VCDWriter' object has no attribute 'register_alias'

Can you run this file on your end Luke?
I'm wondering if I borked my nmigen, or if it's something else.

(In reply to andrey from comment #37)

> Can you run this file on your end Luke?
> I'm wondering if I borked my nmigen, or if it's something else.

$ pip3 list | grep vcd
pyvcd                           0.2.3

(In reply to Luke Kenneth Casson Leighton from comment #38)
> (In reply to andrey from comment #37)
> $ pip3 list | grep vcd
> pyvcd                           0.2.3
Thanks Luke, fixed by re-installing pyvcd.


To stop my madness of copying soc files into pinmux repo, I set up a new schroot "libresoc-dev", and have ran "hdl-dev-repos" script to get soc and supporting libraries.
Then I modified test_jtag_tap_srv.py to use the original import paths.

Steps I take to try to run the test:
- Open two separate terminals and call "schroot -c libresoc-dev" to get into schroot

Terminal 1:
- cd ~/src/soc/pinmux/src/spec/
- python3 test_jtag_tap_srv.py

Terminal 2:
- cd ~/src/soc/src/soc/debug/test/
- python3 test_jtag_tap.py

Error in when running server in Terminal 1:
Traceback (most recent call last):
  File "test_jtag_tap_srv.py", line 265, in <module>
    sim.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 165, in run
    while self.advance():
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 156, in advance
    return self._engine.advance()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 319, in advance
    self._step()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 308, in _step
    process.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 123, in run
    self.coroutine.throw(exn)
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 64, in run
    command = self.coroutine.send(response)
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 90, in wrapper
    yield from process()
  File "/home/rohdo/src/nmutil/src/nmutil/util.py", line 53, in wrapper
    yield from process
  File "test_jtag_tap_srv.py", line 66, in jtag_sim
    yield srv_dut.ios[0].pad.i.eq(1)
KeyError: 0


Error in when running server in Terminal 2:
Traceback (most recent call last):
  File "test_jtag_tap.py", line 178, in <module>
    sim.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 165, in run
    while self.advance():
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 156, in advance
    return self._engine.advance()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 319, in advance
    self._step()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 308, in _step
    process.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 123, in run
    self.coroutine.throw(exn)
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 64, in run
    command = self.coroutine.send(response)
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 90, in wrapper
    yield from process()
  File "/home/rohdo/src/nmutil/src/nmutil/util.py", line 53, in wrapper
    yield from process
  File "test_jtag_tap.py", line 118, in jtag_sim
    assert status == 0
AssertionError

I don't know what causes Error1, but Error2 looks like it might be formal verification error.


Am I missing something else or not calling the tests correctly?
I'd really want to see something existing work.

(In reply to andrey from comment #39)
> Terminal 1:
> - cd ~/src/soc/pinmux/src/spec/
> - python3 test_jtag_tap_srv.py
I just noticed that the server script has an input argument "server".
Calling:
python3 test_jtag_tap_srv.py server

Causes the script to hang, I guess waiting for a connection.

(In reply to andrey from comment #40)

> I just noticed that the server script has an input argument "server".
> Calling:
> python3 test_jtag_tap_srv.py server
> 
> Causes the script to hang, I guess waiting for a connection.

+        print ("running server only as requested, use openocd remote to test")

$ ls -altr !$
ls -altr debug/test/openocd_test.sh
-rwxr-xr-x 1 lkcl lkcl 255 Oct 15  2020 debug/test/openocd_test.sh

run it from the soc directory.

(In reply to andrey from comment #39)

> Terminal 2:
> - cd ~/src/soc/src/soc/debug/test/
> - python3 test_jtag_tap.py

that's a (duplicate) stand-alone version.  notice the two
simulation-processes:

    sim.add_sync_process(wrap(jtag_sim(dut))) # actual jtag tester
    sim.add_sync_process(wrap(dmi_sim(dut)))  # handles (pretends to be) DMI


lkcl@fizzy:~/src/libresoc/soc/src/soc$ python3 debug/test/test_jtag_tap.py 
idcode 0x18ff
        dmi wen, addr 0 0
        read ctrl reg 4
dmi ctrl status 0x4
        dmi wen, addr 0 1
        dmi wen, addr 0 0
        read ctrl reg 4
dmi ctrl status 0x4
        dmi wen, addr 1 0
        write ctrl reg 5 6
        dmi wen, addr 0 1
        dmi wen, addr 0 0
        read ctrl reg 6
dmi ctrl status 0x6
        dmi wen, addr 0 1
        dmi wen, addr 0 3
        read msr reg
dmi msr 0xdeadbeef
        dmi wen, addr 0 4
wb write 0x0
wb read 0xfeef
dmi sim stopping


>   File "test_jtag_tap_srv.py", line 66, in jtag_sim
>     yield srv_dut.ios[0].pad.i.eq(1)
> KeyError: 0

this should be perfectly fine.  no such exception occurs here,
so i don't know what is different.  extensive debug printing
and vcd tracing will - eventually - find out what the hell is
going on there.  let's discuss on IRC.

> Error in when running server in Terminal 2:
>   File "test_jtag_tap.py", line 118, in jtag_sim
>     assert status == 0
> AssertionError

no idea, so i just altered the test to assert against the
value thats' returned (6).

https://git.libre-soc.org/?p=soc.git;a=commitdiff;h=b35ac7c2e49d36954eb2c07d6adefb9797d4f0f8

> I don't know what causes Error1, but Error2 looks like it might be formal
> verification error.

nope, that's a python assert.

(In reply to Luke Kenneth Casson Leighton from comment #41)
> (In reply to andrey from comment #40)
> run it from the soc directory.
This fixed it for me. test_jtag_tap_srv.py now runs

> 
> (In reply to andrey from comment #39)
> 
> > Terminal 2:
> > - cd ~/src/soc/src/soc/debug/test/
> > - python3 test_jtag_tap.py
> 
> that's a (duplicate) stand-alone version.  notice the two
> simulation-processes:
> 
>     sim.add_sync_process(wrap(jtag_sim(dut))) # actual jtag tester
>     sim.add_sync_process(wrap(dmi_sim(dut)))  # handles (pretends to be) DMI
Ok, the jtag_sim tests the io connections, whereas the dmi_sim exercises the DMI register interface?


Copied some of the code from test_jtag_tap_srv.py int testing_stage1.py.
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=a55987cf488b72b2bab4159958a8c15eee673d37;hb=35ecdecc2bffea7b285ca0e166e89b640741641a

Code crashing:
Traceback (most recent call last):
  File "testing_stage1.py", line 447, in <module>
    sim = Simulator(top)
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 66, in __init__
    self._fragment = Fragment.get(fragment, platform=None).prepare()
  File "/home/rohdo/src/nmigen/nmigen/hdl/ir.py", line 37, in get
    obj = obj.elaborate(platform)
  File "testing_stage1.py", line 122, in elaborate
    print ("resources", platform.resources.items())
AttributeError: 'NoneType' object has no attribute 'resources'

For the simulation part of the code, I replaced any mention of "dut" with top.jtag (as the sim invoked an instance of JTAG with the name "dut").

Honestly I've lost the plot, and going around in circles.
Done for today, I can't think properly.

Summary of what I understand:
-We need to write a program that takes a json pinset file and attaches the JTAG boundary scan chain where appropriate (for I/O/OE GPIO). This is already kind of working thanks to Luke's effort (although I barely understand the code).
-There needs to be a set of unit tests that check whether the generated logic indeed adds a boundary scan chain. The plan is to leverage at least some of the code from test_jtag_tap_srv.py.
So my guess is that IO needs to be driven manually, as well as by the JTAG (hence the use of the server) to confirm functionality.
-Once the above are done, code needs to be expanded to support the rest of the SoC's functions such as RGMII (which are basically GPIOs anyway)

What I don't understand:
-How to make a simulation for testing the Blinker block, given that it is inside ASICPlatform.
-How to invoke the JTAG server code, without adding all the extra blocks (such as SRAM) from the test_jtag_tap_srv.py
-What to do after the unit tests are working (not important atm, but useful for overall picture)

(In reply to andrey from comment #42)

> >     sim.add_sync_process(wrap(jtag_sim(dut))) # actual jtag tester
> >     sim.add_sync_process(wrap(dmi_sim(dut)))  # handles (pretends to be) DMI
> Ok, the jtag_sim tests the io connections, whereas the dmi_sim exercises the
> DMI register interface?

yes.  

> Code crashing:

terminology: "crashing" is extremely serious, it indicate a catastrophic
failure on the part of the binary executable, "/usr/bin/python3".

you should not use the term "crashing" for any interpreted language
unless, in fact, the actual interpreter binary executable does in fact
terminate with extreme prejudice, usually with a segfault and a coredump.

you meant to say, "an exception occurred" or "an exception was raised".

>     sim = Simulator(top)
>   File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 66, in __init__
>     self._fragment = Fragment.get(fragment, platform=None).prepare()
>   File "/home/rohdo/src/nmigen/nmigen/hdl/ir.py", line 37, in get
>     obj = obj.elaborate(platform)
>   File "testing_stage1.py", line 122, in elaborate
>     print ("resources", platform.resources.items())
> AttributeError: 'NoneType' object has no attribute 'resources'

this is likely correct.  you have attempted to use the module
directly, prior to having called "build".

only by calling "build" will platform resources be allocated.

if you recall, we went over this lsst week.

the situation is slightly unique in that further modules
are created (the pin pads) by the build process, meaning
that somehow the overall module needs to be extracted.

i have no idea how yet that is done, it will involve
analysing PlatformTemplate and comparing how it
calls elaborate() compared to how pysim calls
elaborate() to obtain a module.

(In reply to andrey from comment #42)

> So my guess is that IO needs to be driven manually, as well as by the JTAG
> (hence the use of the server) to confirm functionality.

yep.  except some IO signals in the Blink example are wired to
each other, uart rx to uart tx.  therefore, of course, trying
to drive uart tx in the simulation will clearly fail.

also bear in mind, the test jtag program has a totally different
io set.  so expecting one to work with the other is unreasonable
given that the example contains hardcoded JTAG IO test values.

> -How to invoke the JTAG server code, without adding all the extra blocks
> (such as SRAM) from the test_jtag_tap_srv.py

take out the add_wishbone function, take out the SRAM, Memory,
and take out the corresponding JTAG WB commands from the client
side.

> -What to do after the unit tests are working (not important atm, but useful
> for overall picture)

start thinking about cleanup, module names, module hierarchy, everything
needed to use this in larger pinmuxes.

also, consider actually adding a pinmux.

notes

step 1. override toolchain_prepare get at fragment
        store in local variable call super().tchcomp

step 2.  pass fragment to sim.

https://git.libre-soc.org/?p=nmigen.git;a=blob;f=nmigen/sim/core.py;h=886cea1820d0771a1e13c742485d78014c299e70;hb=e88d283ed30448ed5fe3ba264e3e56b48f2a4982#l66

that looks bleedin obvious.  but worth checking Fragment.get

https://git.libre-soc.org/?p=nmigen.git;a=blob;f=nmigen/hdl/ir.py;h=3c1ec6045d945fc6461d90eda08e5f48dfbb976e;hb=e88d283ed30448ed5fe3ba264e3e56b48f2a4982#l32

yep.  if already a Fragment return it.

so the key is an overide of toolchain_prepare that grabs the fragment
after it has had all the calls to get_input etc.

+++ b/src/spec/testing_stage1.py
@@ -400,6 +400,12 @@ class ASICPlatform(TemplatedPlatform):
         m.d.comb += padpin.oe.eq(padport_oe)
         return m
 
+    def toolchain_prepare(self, fragment, name, **kwargs):
+        """override toolchain_prepare in order to grab the fragment
+        """
+        self.fragment = fragment
+        return super().toolchain_prepare(fragment, name, **kwargs)
+

done

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=9f43ae5a883590d91b9a6f1211c1b29e5dd68fbc

also added some notes.  i leave you to work through them.

(In reply to Luke Kenneth Casson Leighton from comment #47)
 
> https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;
> h=9f43ae5a883590d91b9a6f1211c1b29e5dd68fbc
> 
> also added some notes.  i leave you to work through them.

sorry, was really tired yesterday, you probably noticed.

simulations require a complete pre-prepared AST fragment, and
will create one from an Elaboratable recursively if given one.

this preparatory phase "locks in" certain data structures and you
cannot simply blithely add random modules after a certain point.

in addition, the complication (not really) is that the Platform
build() process actually constructs additional modules
(as can be seen in get_input() etc) and adds them to a toplevel
AST fragment.

if you do not call build() and then get access to that toplevel
fragment, you cannot possibly hope or expect to have the IOPads
or the JTAG Boundary Scan connections.

this is what the override is about.

therefore look carefully where i have added notes, moving all
additional modules *above* the instantiation of the
ASICPlatform.

or as explained, simply delete them and remove the associated
JTAG code from the test client.

okaay some notes on this evening's investigation: Simulator() running on
Platform fragments has corrupted clock domains resulting in some sort
of recursive dependencies.

the solution: move the entire construction of JTAG Boundary IO creation
back out of ADICPlatform and into the JTAG class, where it used to be.

however to retain compatibility with platform resources, pad_mgr should
be moved into rhe JTAG class.

the complication is that the exact order in which pad_mgr.request_resource()
is called (and with the exact same arguments) must be precisely
and without deviation of any kind the exact order in which
Platform.request_resource is called, such that the _ports of
pad_mgr and _ports of ASICPlatform are identical.

additionally, the connectivity to jtag.ios has to come *out* of
get_input/output and move into a JTAG.elaborate special new function
that connects up the boundary scan registers.

the upshot here will be that:

* when Simulation() is called, which does NOT pass a platform
  instance into Blinker.elaborate() the pad_mgr connections become
  the external ports, which can then be manipulated by the unit test

* when ASICPlatform.build is called, the .il file is outputted with
  a stack of IOPad connections.

big redesign basically.

andrey i've started moving things back into the JTAG class
https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=47b033ea5d319279080f69da2ca16750678967ee

the goal is to establish the entire JTAG Boundary Scan in
the module named "top".... *BEFORE* calling Platform.build()

Platform.build can connect up pins by connecting to something
that has been set up by the JTAG() instance, in a very very
brain-dead straightforward way.

okaaay done, but holy cow what a mess.
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=ebb6c4c3918872e21dabca964bdc66016b90b220;hb=0009133fd29cbcc6fe6c189a7290bc4bfcee08ec

* the entire pair of pad and core ResourceManagers had to be duplicated
  inside the JTAG module

  (bear in mind: Platform *derives* from ResourceManager)

* these two resources still require duplicating resources.  therefore,
  add_resources is still overridden and still adds to *BOTH* jtag.pad_mgr
  *AND* jtag.core_mgr

* everything that was previously in Platform and used Platform-as-a-ResMgr
  was moved to the JTAG module

* the wiring up of ports-to-pads is now done *inside* JTAG Manager

* **ANOTHER** ResourceManager - this time the Platform one - has *yet another*
  duplicate set of resources

* this duplicate set of resources is *ONLY* wired up if a Platform is passed
  in.

* the duplicate set of resources in Platform is wired up "direct". strictly
  speaking it is totally unnecessary, but may in future replace coriolis2
  corona (hopefully not any time soon) creation and therefore would actually
  create $Instance("coriolis2_IOPAD") and wire them up.

* all bi-directional pads had to be converted to explicit triplet individual
  pins (named padname_i, padname_o and padname_oe) otherwise internal
  assumptions deep inside Platform about bi-directional pads got thoroughly
  in the way.

what a mess.  still, it works.  oh, and no, Simulation() of build fragment
still does not work, and won't.

however now that the JTAG module creates and manages the JTAG Boundary
Scan IO entirely itself, this is completely irrelevant.  a top module
*WITHOUT* calling Platform.build() can be used directly instead.

Removed code I took from the JTAG server test, replaced it with a simple sanity test case.

https://git.libre-soc.org/?p=pinmux.git;a=blobdiff;f=src/spec/testing_stage1.py;h=db7e7ce1af63df7c553eb61824005b30d46f5915;hp=a2f1c5c5959d321999ae1bbdf861d414dd5c01a1;hb=0d65da15997f7128cab430408e4629adfcd94210;hpb=a61fe0517a767c5c0985ef8b7d8482c2a1e12a44

I'm getting a TypeError because simulator does not consider this function:
def test_case0():
    print("Starting sanity test case!")
    yield top.gpio_0__gpio_0__i__io.eq(0)
    yield 

Even though I have 'yield' statements in the function itself.

Traceback (most recent call last):
  File "testing_stage1.py", line 361, in <module>
    sim.add_sync_process(test_case0())
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 85, in add_sync_process
    process = self._check_process(process)
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 73, in _check_process
    .format(process))
TypeError: Cannot add a process <generator object test_case0 at 0x7f459ca149a8> because it is not a generator function


What am I missing from the function to make it a generator?

(In reply to andrey from comment #52)

> What am I missing from the function to make it a generator?

a change in the simulator API 18 months ago requires the use
of the wrap() function.

(In reply to Luke Kenneth Casson Leighton from comment #53)
> (In reply to andrey from comment #52)
> 
> > What am I missing from the function to make it a generator?
> 
> a change in the simulator API 18 months ago requires the use
> of the wrap() function.

Thanks, adding the wrap function got the generator test function to be accepted.
Now I'm trying to figure out the signal names to use in the test.

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=0f5e9d3241043e2439f39b98bfa496e1169dcd75

For example, accessing gpio0 output.

I tried the following:
From the code in Blinker: gpio.gpio0.o
From yosys diagram: gpio_0__gpio0__o__io
From vcd GTKWave: gpio_0__gpio0__o__core__o, gpio_0__gpio0__o__o, gpio_0__gpio0__o__pad__o
From script's standard output resource: gpio0_o, gpio_0__gpio0__o

In all cases I got the:
Traceback (most recent call last):
  File "testing_stage1.py", line 425, in <module>
    sim.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 165, in run
    while self.advance():
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 156, in advance
    return self._engine.advance()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 319, in advance
    self._step()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 308, in _step
    process.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 123, in run
    self.coroutine.throw(exn)
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 64, in run
    command = self.coroutine.send(response)
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 90, in wrapper
    yield from process()
  File "/home/rohdo/src/nmutil/src/nmutil/util.py", line 53, in wrapper
    yield from process
  File "testing_stage1.py", line 376, in test_case0
    yield top.gpio0_o.eq(0)
AttributeError: 'Blinker' object has no attribute 'gpio0_o'

I thought the GPIO block is accessible as an attribute of top module.
What is the correct way to access the GPIO signals in the test?

(In reply to andrey from comment #54)

> AttributeError: 'Blinker' object has no attribute 'gpio0_o'

this is perfectly correct.  you have not added an object named
"gpio0_o" inside the Blinker class: why would you be surprised
that it does not exist?

> I thought the GPIO block is accessible as an attribute of top module.

to the nmigen *AST*, yes.  but you are not directly accessng the
AST, here, you are accessing the python object named "top" which
happens to be an instance of Blinker().

> What is the correct way to access the GPIO signals in the test?

first add the GPIO signals to the class instance so that they are
publicly accessible!

+++ b/src/spec/testing_stage1.py
@@ -172,6 +172,10 @@ class Blinker(Elaboratable):
         m.d.comb += uart.tx.eq(intermediary)
         m.d.comb += intermediary.eq(uart.rx)
 
+        # to even be able to get at objects, you first have to make them
+        # available - i.e. not as local variables
+        self.gpio = gpio
+

Created attachment 148 [details]
Sim showing GPIO 2 and 3 o/oe signals switching

(In reply to Luke Kenneth Casson Leighton from comment #55)
> (In reply to andrey from comment #54)
> 
> > AttributeError: 'Blinker' object has no attribute 'gpio0_o'
> 
> this is perfectly correct.  you have not added an object named
> "gpio0_o" inside the Blinker class: why would you be surprised
> that it does not exist?
Due to not really understanding the topology, my brain basically switched off.

Now it makes sense though, given that the gpio object was not a public attribute.

Also in regards to topology, the "gpio" object seems to me to be a core side block (thus you can control the o/oe signals). Is there a corresponding "pad" object (which would allow me to control the i signal)?

> 
> > I thought the GPIO block is accessible as an attribute of top module.
> 
> to the nmigen *AST*, yes.  but you are not directly accessng the
> AST, here, you are accessing the python object named "top" which
> happens to be an instance of Blinker().
I seem to remember you talking about AST and then some other object after the AST step, but this escaped me.

> 
> > What is the correct way to access the GPIO signals in the test?
> 
> first add the GPIO signals to the class instance so that they are
> publicly accessible!
> 
> +++ b/src/spec/testing_stage1.py
> @@ -172,6 +172,10 @@ class Blinker(Elaboratable):
>          m.d.comb += uart.tx.eq(intermediary)
>          m.d.comb += intermediary.eq(uart.rx)
>  
> +        # to even be able to get at objects, you first have to make them
> +        # available - i.e. not as local variables
> +        self.gpio = gpio
> +
I'm able to control o and oe signals by using top.gpio.gpio2.oe (for example), which is a great step forward.
Not sure how to control the i signal, but I'm using your print method for seeing the objects contained in JTAG, and I'll try to figure it out tomorrow.

Made uart signals public by writing
self.uart = uart
Tried to control the rx, (as in this example, rx sets "intermediary" signal, which then sets tx), but not working. I'm guessing this is a similar issue I have with gpio i where I'm trying to drive the wrong signal.

You can see the GPIO 2 and 3 outputs being toggled in the attached screenshot :)

(In reply to andrey from comment #56)

> Due to not really understanding the topology, my brain basically switched
> off.

understanding comes later and is not strictly necessary: results generate
their own understanding-map based on repeated observations.  this is the
basics of auto-didact / self-learning.  babies do this with spectacular
results.  why larger humans lose this ability is bewildering to me :)
 
> Now it makes sense though, given that the gpio object was not a public
> attribute.

you would not be very happy with python as a language if it magically
added hidden objects without your consent or knowledge.

> Also in regards to topology, the "gpio" object seems to me to be a core side
> block (thus you can control the o/oe signals). Is there a corresponding
> "pad" object (which would allow me to control the i signal)?

reminder: forget - for now - about JTAG.  at this point you should
*only* be:

1) yield-eq'ing the o/oe record objects to integer values (python values)
2) yield-ing i.e. GETTING the / record objects.

gpio2_input_value = yield top.gpio.gpio2.i

do NOT attempt to set (as you have been) any record objects equal
to other record objects or Signals.  this is DIRECTLY interfering
with the AST tree and it is guaranteed to be precisely and exactly
what you DO NOT want.

> > 
> > > I thought the GPIO block is accessible as an attribute of top module.
> > 
> > to the nmigen *AST*, yes.  but you are not directly accessng the
> > AST, here, you are accessing the python object named "top" which
> > happens to be an instance of Blinker().
> I seem to remember you talking about AST and then some other object after
> the AST step, but this escaped me.

you created a Blinker() instance. you put it into a variable named "top".
that instance has absolutely nothing to do with Simulations() in any
way, shape or form.

it CAN - if REQUESTED - create an Abstract Syntax Tree - IF REQUESTED
TO DO SO.

you are conflating things in your head again.  the object that is
**CAPABLE** of creating (on-demand) an Abstract Syntax Tree is not
**ITSELF** the **ACTUAL** Abstract Syntax Tree.

the AST happens to be the return result of a function provided **BY**
the object that you happened to have assigned to a python variable,
which you chose to call by the name "top"

this is about conceptual separation: you need to be absolutely clear
about it, and not "mentally lump stuff together just because it happens
to be in the same source code file".


> I'm able to control o and oe signals by using top.gpio.gpio2.oe (for
> example), which is a great step forward.

excellent.

> Not sure how to control the i signal, but I'm using your print method for
> seeing the objects contained in JTAG, and I'll try to figure it out tomorrow.

see below
 
> Made uart signals public by writing
> self.uart = uart
> Tried to control the rx, (as in this example, rx sets "intermediary" signal,
> which then sets tx), but not working. I'm guessing this is a similar issue I
> have with gpio i where I'm trying to drive the wrong signal.
> 
> You can see the GPIO 2 and 3 outputs being toggled in the attached
> screenshot :)

exccellent!

ok, well, i totally lied about not having to get involved with JTAG Boundary
Scan stuff (or, not quite).  try setting the new no_jtag_connect argument
to True and, magically, by *disconnecting* the gpio and uart resources from
the JTAG Boundary scan, suddenly you can set them successfully.

you cannot "tap into" a wire and set it in the middle, you have to get the
end of the chain, which in this case (sigh) is the stuff created by
Boundary Scan, i.e. right back at the "pads" as you probably suspected.

these are accumulated in a data structure Blinker.jtag.resource_table_pads
i've committed some more stuff which should in theory be self-evident
from the diffs at least

(In reply to Luke Kenneth Casson Leighton from comment #57)
> (In reply to andrey from comment #56)
> understanding comes later and is not strictly necessary: results generate
> their own understanding-map based on repeated observations.  this is the
> basics of auto-didact / self-learning.  babies do this with spectacular
> results.  why larger humans lose this ability is bewildering to me :)
Hahaha, I'll try to keep this in mind. Poke things until something does something else.

>  
> > Now it makes sense though, given that the gpio object was not a public
> > attribute.
> 
> you would not be very happy with python as a language if it magically
> added hidden objects without your consent or knowledge.
> 
> > Also in regards to topology, the "gpio" object seems to me to be a core side
> > block (thus you can control the o/oe signals). Is there a corresponding
> > "pad" object (which would allow me to control the i signal)?
> 
> reminder: forget - for now - about JTAG.  at this point you should
> *only* be:
> 
> 1) yield-eq'ing the o/oe record objects to integer values (python values)
> 2) yield-ing i.e. GETTING the / record objects.
> 
> gpio2_input_value = yield top.gpio.gpio2.i
> 
> do NOT attempt to set (as you have been) any record objects equal
> to other record objects or Signals.  this is DIRECTLY interfering
> with the AST tree and it is guaranteed to be precisely and exactly
> what you DO NOT want.
> 
Ok, only use integer values, or return records themselves.

> you created a Blinker() instance. you put it into a variable named "top".
> that instance has absolutely nothing to do with Simulations() in any
> way, shape or form.
> 
> it CAN - if REQUESTED - create an Abstract Syntax Tree - IF REQUESTED
> TO DO SO.
> 
> you are conflating things in your head again.  the object that is
> **CAPABLE** of creating (on-demand) an Abstract Syntax Tree is not
> **ITSELF** the **ACTUAL** Abstract Syntax Tree.
> 
> the AST happens to be the return result of a function provided **BY**
> the object that you happened to have assigned to a python variable,
> which you chose to call by the name "top"
> 
> this is about conceptual separation: you need to be absolutely clear
> about it, and not "mentally lump stuff together just because it happens
> to be in the same source code file".
Everything is separated, ok.
 
>  
> > You can see the GPIO 2 and 3 outputs being toggled in the attached
> > screenshot :)
> 
> exccellent!
It is indeed a little scary that the signals even toggled at all, when I set them to be equal to a signal object as opposed to an integer :')

> 
> ok, well, i totally lied about not having to get involved with JTAG Boundary
> Scan stuff (or, not quite).  try setting the new no_jtag_connect argument
> to True and, magically, by *disconnecting* the gpio and uart resources from
> the JTAG Boundary scan, suddenly you can set them successfully.
> 
> you cannot "tap into" a wire and set it in the middle, you have to get the
> end of the chain, which in this case (sigh) is the stuff created by
> Boundary Scan, i.e. right back at the "pads" as you probably suspected.
> 
Makes sense, I'll play with it some more.

> these are accumulated in a data structure Blinker.jtag.resource_table_pads
> i've committed some more stuff which should in theory be self-evident
> from the diffs at least
Thanks Luke

(In reply to andrey from comment #58)

> It is indeed a little scary that the signals even toggled at all, when I set
> them to be equal to a signal object as opposed to an integer :')

basically from what i can gather you may have modified the actual AST,
with potentially disastrous unpredictable results.

the only reason it "worked" is because, back in the Blinker elaborate(),
those GPIO pads had not - have not - been wired up to anything.

if you now grab the relevant GPIO resource from resource_table_pads
then the ends of the wires can be safely toggled.

(btw please do trim excessive context especially in bugzilla replies)

(In reply to Luke Kenneth Casson Leighton from comment #59)
> (In reply to andrey from comment #58)
> basically from what i can gather you may have modified the actual AST,
> with potentially disastrous unpredictable results.
Sounds like fun...I'm sure this hasn't affected anyone before :)

> if you now grab the relevant GPIO resource from resource_table_pads
> then the ends of the wires can be safely toggled.
Now makes senses, thanks luke.
 
> (btw please do trim excessive context especially in bugzilla replies)
Apologies, will keep in mind.

From our chat on IRC:
https://libre-soc.org/irclog/%23libre-soc.2021-11-30.log.html

Next steps are:
-Create two multi-bit signals (based on number of gpios) called:
gpio_o_test
gpio_oe_test

-Wire up the output signal of each gpio by XOR'ing each bit of gpio_o_test with gpio's input:
comb += gpio0.o.eq(gpio_test[0] ^ gpio0.i)
...

-Wire up each bit of gpio_oe_test signal to oe signal of each gpio. 

-Have the sim run through a for-loop where the gpio_o_test is incremented like a counter (0000, 0001...)
-At each iteration of the for-loop, assert:
+ output set at core matches output seen at pad
+ input set at pad matches input seen at core
+ if gpio_o_test bit is cleared, output seen at pad matches input seen at pad

-Another for loop to run through gpio_oe_test. Assert:
+ oe set at core matches oe seen at pad.

As there is no I/O pad logic, the state of the output signal is NOT affected by the oe signal. I've confirmed this by setting gpio3.o and toggling gpio3.oe and checking the waveforms.

For the moment I'll start working on this. This is simple, and it shouldn't take me a lot of additional code. If I start to write too much, I'll ask questions.

(In reply to andrey from comment #60)

> -Another for loop to run through gpio_oe_test. Assert:
> + oe set at core matches oe seen at pad.

looks like a plan.

> As there is no I/O pad logic, the state of the output signal is NOT affected
> by the oe signal. 

err is that true? yes it must be.  if using an actual IOpad it would
not.  interesting.

when it comes to testing boundary scan this will not hold true. not because
of IOpads but because the i o and oe (each) get re-routed.  but, one thing
at a time.

> I've confirmed this by setting gpio3.o and toggling
> gpio3.oe and checking the waveforms.

nicely done.

Created attachment 149 [details]
GPIO output counter test

Added the for loop needed to go through all permutations of the GPIO output signal states (very satisfying screenshot).

Before adding oe and i testing, I wanted to add the python asserts so that the test could do the work for me.
Here's the code:
for gpio_o_val in range(0, num_gpio_o_states):
    yield top.gpio_o_test.eq(gpio_o_val)
    yield Settle()
    yield # Move to the next clk cycle

    print(type(top.gpio.gpio0.o), type(gpios_pad.gpio0.o))
    print(top.gpio.gpio0.o, gpios_pad.gpio0.o)
    core_out = yield top.gpio.gpio0.o
    pad_out = yield gpios_pad.gpio0.o
    assert core_out == pad_out

I get a runtime TypeError at the "core_out" assignment:
Traceback (most recent call last):
  File "testing_stage1.py", line 554, in <module>
    sim.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 165, in run
    while self.advance():
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 156, in advance
    return self._engine.advance()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 319, in advance
    self._step()
  File "/home/rohdo/src/nmigen/nmigen/sim/pysim.py", line 308, in _step
    process.run()
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 123, in run
    self.coroutine.throw(exn)
  File "/home/rohdo/src/nmigen/nmigen/sim/core.py", line 90, in wrapper
    yield from process()
  File "/home/rohdo/src/nmutil/src/nmutil/util.py", line 53, in wrapper
    yield from process
  File "testing_stage1.py", line 532, in test_gpios
    core_out = yield top.gpio.gpio0.o
  File "/home/rohdo/src/nmigen/nmigen/sim/_pycoro.py", line 115, in run
    .format(command, self.src_loc()))
TypeError: Received unsupported command (rec gpio_0__gpio0__o o) from process 'testing_stage1.py:532'

Looks like I'm not using the gpio object correctly, something like that?
I also guessed their might be a ".value" attribute I could access, but there isn't.
Can you clarify tomorrow as to what I'm missing here?

(In reply to andrey from comment #62)

> Here's the code:
> for gpio_o_val in range(0, num_gpio_o_states):
>     yield top.gpio_o_test.eq(gpio_o_val)
>     yield Settle()
>     yield # Move to the next clk cycle
> 
>     print(type(top.gpio.gpio0.o), type(gpios_pad.gpio0.o))
>     print(top.gpio.gpio0.o, gpios_pad.gpio0.o)

drop this:

>     core_out = yield top.gpio.gpio0.o

keep this:

>     pad_out = yield gpios_pad.gpio0.o

replace this:

>     assert core_out == pad_out

print (gpio_o_val, pad_out)
assert (gpio_o_val & 0b0001) == pad_out
 
> TypeError: Received unsupported command (rec gpio_0__gpio0__o o) from
> process 'testing_stage1.py:532'

middle of chain. confirm with yosys "show top".
neither possible nor desirable.

        gpio_oe_test = Signal(num_gpios)
        # Wire up the output signal of each gpio by XOR'ing each bit of g
pio_o_test with gpio's input
        # Wire up each bit of gpio_oe_test signal to oe signal of each gpio. 
        # Turn into a loop at some point, probably a way without
        # using get_attr()

PEP-8.  do keep comments (actually, all lines) to under 80 chars

if you must insist on wasting screen real-estate by running editors in
windows of greater than 80 chars in size then add a "guide line" at
80 chars.  it is better to fix the terminal width *at* 80 chars which
has the result of making the code look immediately and obviously s***,
like this:

https://ftp.libre-soc.org/2021-12-02_11-55.png

you can install and run pep8checker and even autopep8 to in-place sort
these things, but if you do *please run it as a separate commit*
with the comment "whitespace" or "autopep8 run"

total fail on my part.  the resource_table_pads is a Pin/Resource
not a Signal or a Record: of course it can't be set/got.

fixed by instead registering the actual Signal/Record (named the "pin"
in Platform resource terminology, *not* the port which is the Pin/Resource)

i made it a dict-of-dicts so there is a dict by keynames
'gpio_0__gpio0__o' and then the pin named 'o', 'oe', or 'i'
inside that.

oe is its *own pin* and it should be accessed as:

    gpio0_oe = top.jtag.boundary_scan_pads['gpio_0__gpio0__oe']['o']

because it is actually an *output*.  long story.

(In reply to Luke Kenneth Casson Leighton from comment #65)
> fixed by instead registering the actual Signal/Record (named the "pin"
> in Platform resource terminology, *not* the port which is the Pin/Resource)
Ah ok, makes sense. Is it effectively trying to read/write a node between signals, not the wire/signal itself?

> 
> i made it a dict-of-dicts so there is a dict by keynames
> 'gpio_0__gpio0__o' and then the pin named 'o', 'oe', or 'i'
> inside that.
Thanks. I added the pad input signals which I'm able to control as well.

> 
> oe is its *own pin* and it should be accessed as:
> ...
> because it is actually an *output*.  long story.
I'll add this one after resolving an issue with reading core input.

After writing a 1 or 0 to the pad input, I want to read the input value seen at the core. In GTKWave I can see that the core input is being set correctly.

https://git.libre-soc.org/?p=pinmux.git;a=commitdiff;h=6d46867430b5ec24f5d9fce2ff1ae098a1a4114d


I tried the following to access core input (only showing GPIO0 i for simplicity):
gpio0_core_in = yield top.gpio['gpio0']['i']
and
temp_in = yield top.gpio.gpio0.i

Both of these threw TypeError seen before, so I'm guessing that I'm trying to use pins, as opposed to signals.

The yosys show top gives me the following chain:
1. gpio_0__gpio0__i__i feeds into jtag_gpio_0__gpio0__i__pad__i
2. jtag_gpio_0__gpio0__i__pad__i feeds into gpio_0__gpio0__i__pad__i (into jtag chain)
3. (out of jtag chain) gpio_0__gpio0__i__core__i feeds into jtag_gpio_0__gpio0__i__core__i 
4. jtag_gpio_0__gpio0__i__core__i feeds into gpio_0__gpio0__i__i$2 (looks like extra signal made for the comb logic)
5. gpio_0__gpio0__i__i$2 is then used in the Blinker's gpio output (XOR).

Based on the yosys diagram, I thought that jtag_gpio_0__gpio0__i__core__i is what I can read to check if the pad input is propagating.

(In reply to andrey from comment #66)

> Ah ok, makes sense. Is it effectively trying to read/write a node between
> signals, not the wire/signal itself?

no, it's worse than that: anything of type Pin/Resource is simply 100%
categorically absolutely nothing to do with Simulations().

> I'll add this one after resolving an issue with reading core input.

which you can only do via these:

    # Grab GPIO input pad resource from JTAG BS - start of chain
    gpio0_pad_in = top.jtag.boundary_scan_pads['gpio_0__gpio0__i']['i']


> After writing a 1 or 0 to the pad input, I want to read the input value seen
> at the core.

then add a temporary Signal to Blinker into which a copy of that value
is placed.


> I tried the following to access core input (only showing GPIO0 i for
> simplicity):
> gpio0_core_in = yield top.gpio['gpio0']['i']
> and
> temp_in = yield top.gpio.gpio0.i
> 
> Both of these threw TypeError seen before, so I'm guessing that I'm trying
> to use pins, as opposed to signals.

don't guess: CHECK. use a print statement to print out the type of the
object.

*you* are in control, here.  any "guessing", your *immediate* reaction
as an Engineer should be, "how can i confirm 100% my guess?"

> Based on the yosys diagram, I thought that jtag_gpio_0__gpio0__i__core__i is
> what I can read to check if the pad input is propagating.

probably... but not via the Pins/Resources, it is completely the wrong
data structure.

git pull and check the code-comments: you missed re-reading pad0_out-pad3_out
after changing gpio0_pad_in.

Created attachment 150 [details]
Sim showing I/O/OE test for all GPIOs

(In reply to Luke Kenneth Casson Leighton from comment #67)
> (In reply to andrey from comment #66)
> then add a temporary Signal to Blinker into which a copy of that value
> is placed.
Added, I'll keep that in mind for future problems.

> don't guess: CHECK. use a print statement to print out the type of the
> object.
> 
> *you* are in control, here.  any "guessing", your *immediate* reaction
> as an Engineer should be, "how can i confirm 100% my guess?"
I'm starting to do this more, and will aim to be self-sufficient, but also offer my findings and ask to clarify if I still don't understand. 

> probably... but not via the Pins/Resources, it is completely the wrong
> data structure.
Yeah. The yosys diagram is starting to make more sense, now I can even tell where the pins and signals are XD

> git pull and check the code-comments: you missed re-reading pad0_out-pad3_out
> after changing gpio0_pad_in.
Fixed.

There's now a test to verify i/o/oe (however it's not completely parametrised, I had some thoughts to use get_attr to iterate over gpios, there might be a cleaner way though). See the attached screenshot for more beautiful waveforms. Oh, and am I uploading too many? The previous screenshots could probably be removed to save space.

I started the UART test, and only noticed later you already gave the example for writing to the UART pad.

Why do I have to use the UART pads from resource_table_pads, and not boundary_scan_pads? Is the JTAG BS connection different for GPIOs and peripherals like UART?

To get a GPIO pad:
gpio0_o = top.jtag.boundary_scan_pads['gpio_0__gpio0__o']['o']

To get a UART pad: 
uart_pad = top.jtag.resource_table_pads[('uart', 0)]

I guess it's not too important, but it might come to bite me when I do I2C (I'll try both later, too tired now).

(In reply to andrey from comment #68)
> Created attachment 150 [details]
> Sim showing I/O/OE test for all GPIOs

exciting as it is, try not to upload unnecessary binary-formatted
images that people can generate for themselves on their local
machine by running a command.

annotated on the other hand (showing something important) is a
different matter.

> Yeah. The yosys diagram is starting to make more sense, now I can even tell
> where the pins and signals are XD

takes a while, doesn't it.

> > git pull and check the code-comments: you missed re-reading pad0_out-pad3_out
> > after changing gpio0_pad_in.
> Fixed.

excellent. comments look great.

> There's now a test to verify i/o/oe (however it's not completely
> parametrised, I had some thoughts to use get_attr to iterate over gpios,
> there might be a cleaner way though). See the attached screenshot for more
> beautiful waveforms. Oh, and am I uploading too many? The previous
> screenshots could probably be removed to save space.

that's not possible.  it's now permanently and irrevocably taking up
space on the server - in a postgresql database - that i have to pay money for.

> To get a UART pad: 
> uart_pad = top.jtag.resource_table_pads[('uart', 0)]

shouldn't be.  amazed it works.

(In reply to Luke Kenneth Casson Leighton from comment #69)
> (In reply to andrey from comment #68)
> annotated on the other hand (showing something important) is a
> different matter.
>...
> that's not possible.  it's now permanently and irrevocably taking up
> space on the server - in a postgresql database - that i have to pay money
> for.
Sorry about that. Will keep in mind from now on.

> 
> > To get a UART pad: 
> > uart_pad = top.jtag.resource_table_pads[('uart', 0)]
> 
> shouldn't be.  amazed it works.
Why? Is it not meant to be accessed this way?

The UART rx/tx jtag.boundary_scan_pads dict seem to be missing the records for the signals, which is why I couldn't access them in the same way as the GPIO or I2C.

The I2C signals are present:
Printing top.jtag.boundary_scan_pads['i2c_0__sda__i']
{'i': (sig i2c_0__sda__i__i)}
And type of printing top.jtag.boundary_scan_pads['i2c_0__sda__i']['i']
<class 'nmigen.hdl.ast.Signal'>

While the UART equivalent
Printing top.jtag.boundary_scan_pads['uart_0__rx__i']
{}

The access via jtag.resource_table_pads works:
Printing type of top.jtag.resource_table_pads[('uart',0)].rx.i
<class 'nmigen.hdl.ast.Signal'>

How can the UART signals be added to the dict? Is this an issue with Blinker definition, or something to add to jtag.py?



Also, UART and I2C basic tests have been implemented.
For the I2C test I connected the SDA i/o and SCL i/o as an internal loopback (similar to what you did with the UART). For o/e I have a separate test port. No complex logic is present, just toggle low and high and check signal propagated from pad to pad.

So once we resolve the little UART issue, what comes next?

(In reply to andrey from comment #70)

> While the UART equivalent
> Printing top.jtag.boundary_scan_pads['uart_0__rx__i']
> {}

that doesn't exist (if it does, the only reason it will be "accessible"
is because top.jtag.boundary_scan_pads is a defaultdict(dict).
which auto-creates a dict *on-demand* when a lookup does not exist

fixed:

 def test_uart():
     # grab the JTAG resource pad
-    uart_pad = top.jtag.resource_table_pads[('uart', 0)]
-    #uart_rx_pad = top.jtag.boundary_scan_pads['uart_0__rx__i']['i']
+    print ()
+    print ("bs pad keys", top.jtag.boundary_scan_pads.keys())
+    print ()
+    uart_rx_pad = top.jtag.boundary_scan_pads['uart_0__rx']['i']
+    uart_tx_pad = top.jtag.boundary_scan_pads['uart_0__tx']['o']


you could have found the correct name with

    print ("bs pad keys", top.jtag.boundary_scan_pads.keys())


> Also, UART and I2C basic tests have been implemented.

fantastic.

> So once we resolve the little UART issue, what comes next?

flipping the boundary scan bits, as is done in test_jtag_tap_srv.py

(In reply to Luke Kenneth Casson Leighton from comment #71)
> (In reply to andrey from comment #70)
> +    print ("bs pad keys", top.jtag.boundary_scan_pads.keys())
Thanks for the fix. I'll be sure to use .keys() from now on, I was trying to figure out how to view the dictionary keys too...

> flipping the boundary scan bits, as is done in test_jtag_tap_srv.py
Started working on this. The sequence so far is:
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=03c8c5def0a034e4f2657d791f5d9e8239625f17;hb=dd0c84a11f93661d1682a95419dcf04aa91cad96#642
(Edit: apologies, forgot to update link)
1. Reset the JTAG BS by doing:
yield from jtag_set_reset(top.jtag)
2. Manipulate the core and pad signals (for example GPIO 0):
top.jtag.ios['gpio_0__gpio0__i'].core.i.eq(1)
top.jtag.ios['gpio_0__gpio0__i'].pad.i.eq(0)
3. Call an empty yield to progress to next clock cycle.

You mentioned that Tck is toggled by jtag_set_reset(), and I checked this is the case in the code:
https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/jtagutils.py;hb=0eb75ea62a5f20e1824f9a25daefda9ff0340e53#l23

I looked at the test_jtag_tap_srv.py as a basis, and I don't see anything done other than the steps I mentioned. I'm ignoring the jtag_read_write_reg however, as I don't expect it to be needed for this task yet.

...
UPDATE
...

As I was writing this, I spotted that I missed "yield from" when using "jtag_set_reset()", so that's one issue solved.

However I get an AttributeError because inside "tms_state_set()", the code is trying to access "cbus" object to toggle Tck:
https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/jtagutils.py;hb=0eb75ea62a5f20e1824f9a25daefda9ff0340e53#l25
yield dut.cbus.tms.eq(bit)

Our Blinker top.jtag does NOT have cbus. Should it have one?
And if so, what is it for?

(In reply to Andrey Miroshnikov from comment #72)

> I looked at the test_jtag_tap_srv.py as a basis, and I don't see anything
> done other than the steps I mentioned. I'm ignoring the jtag_read_write_reg
> however, as I don't expect it to be needed for this task yet.

it will be: it's the only way that the shift registers can be altered.
 
> ...
> UPDATE
> ...
> 
> As I was writing this, I spotted that I missed "yield from" when using
> "jtag_set_reset()", so that's one issue solved.
> 
> However I get an AttributeError because inside "tms_state_set()", the code
> is trying to access "cbus" object to toggle Tck:
> https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/jtagutils.py;
> hb=0eb75ea62a5f20e1824f9a25daefda9ff0340e53#l25
> yield dut.cbus.tms.eq(bit)

use test_jtag_tap.py instead.

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/debug/test/test_jtag_tap.py;h=757c313c54a67db054097cc8dba979cac64424b1;hb=0eb75ea62a5f20e1824f9a25daefda9ff0340e53

> Our Blinker top.jtag does NOT have cbus. Should it have one?

no.  look at c4m-jtag tap.py

https://git.libre-soc.org/?p=c4m-jtag.git;a=blob;f=c4m/nmigen/jtag/tap.py;h=1f3d424cbd7451c0434e0c71168f5aa0935af860;hb=c2bf4810f9f91ced7fcda777b92b86ab353da288#l328

always hunt down the inheritance tree, either in a browser window
or keep the source code open in a terminal.

$ ps auxww | grep vi | wc
    586    7162   56392

for the past 10 years i now have to do "jobs | grep {filename}"
in many of the xterms i have open.

> And if so, what is it for?

https://git.libre-soc.org/?p=c4m-jtag.git;a=blob;f=c4m/nmigen/jtag/bus.py;h=d1532bc08d0d8170c5b7c5e62dba67f8c6fe709a;hb=c2bf4810f9f91ced7fcda777b92b86ab353da288#l18

  18 class Interface(Record):
  28         layout = [
  29             ("tck", 1, Direction.NONE),
  30             ("tms", 1, Direction.NONE),
  31             ("tdo", 1, Direction.FANOUT),
  32             ("tdi", 1, Direction.FANIN),
  33         ]

(In reply to Luke Kenneth Casson Leighton from comment #73)
> (In reply to Andrey Miroshnikov from comment #72)
> use test_jtag_tap.py instead.
Test_jtag_tap.py doesn't interact with the core/pad signals using the "ios" dict directly, which I guess it can't anyway (because everything has to be done via TDI/TDO).

The DMI and WB code I took from test_jtag_tap.py was more just to play with, see if it did anything. I saw that the FSM was updating and JTAG clocks were ticking, so good sign there.

Looking at "show jtag", I saw a few signals of note:
-irblock_ir (which is produced from a 4-bit ir in irblock) determines the mux connections.
-io_bd is connected to the jtag shift register, and is the second option for the pad muxes

For switching the muxes to break pad/core connection, I need to somehow clear the irbloc_ir signal, I guess it can be done via one of the "jtag_" commands (I haven't figured out which one).

Do you know the sequence shifting in data? Or where I can find an example sequence? (Did not find such an example in jtag.py, jtagutils.py, test_jtag_tap.py, test_jtag_tap_srv.py)


Your example of writing to the "ios" shift register doesn't change the bits in io_sr/io_bd:
top.jtag.ios['uart_0__rx'].core.i.eq(1)
yield
top.jtag.ios['uart_0__rx'].core.i.eq(0)
yield

I tried:
-Reseting the JTAG
-Setting the TDI bit (which didn't work)
-Running jtag_set_shift_ir() a few times

Interestingly enough, I'm seeing TDO toggle when jtag_set_shift_ir() is being called.


> 
> $ ps auxww | grep vi | wc
>     586    7162   56392
> 
> for the past 10 years i now have to do "jobs | grep {filename}"
> in many of the xterms i have open.
> 
Sorry, I don't understand why your are checking for "vi" processes. Is this for when you have so many terminals open, that you just trying to find the right one? But then why count the number of entries with wc?

(In reply to Andrey Miroshnikov from comment #74)
> (In reply to Luke Kenneth Casson Leighton from comment #73)
> > (In reply to Andrey Miroshnikov from comment #72)
> > use test_jtag_tap.py instead.
> Test_jtag_tap.py doesn't interact with the core/pad signals using the "ios"
> dict directly, which I guess it can't anyway (because everything has to be
> done via TDI/TDO).

... check c4m-jtag tap source code. add_io()

https://git.libre-soc.org/?p=c4m-jtag.git;a=blob;f=c4m/nmigen/jtag/tap.py;h=1f3d424cbd7451c0434e0c71168f5aa0935af860;hb=c2bf4810f9f91ced7fcda777b92b86ab353da288#l532

that's adding the Record (IOConn) to an internal list, self._ios.
that then, if you search for self._ios, lines 587, 590, 593, 595,
600, 601 and 602, you can see the Muxes.

now returning to jtag.py

https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/jtag.py;h=efda2806c07e6f01f3e8501e9bcdd8245fc63991;hb=9f43ae5a883590d91b9a6f1211c1b29e5dd68fbc#l83

we see the *exact same data structure being added to a dictionary-version
self.ios instead of a list-version self._ios.

therefore, logically, we may deduce that those *are* the IOConn Records,
and that if they're not changing then you're doing something wrong.

> Looking at "show jtag", I saw a few signals of note:
> -irblock_ir (which is produced from a 4-bit ir in irblock) determines the
> mux connections.

yyep.

> For switching the muxes to break pad/core connection, I need to somehow
> clear the irbloc_ir signal, I guess it can be done via one of the "jtag_"
> commands (I haven't figured out which one).

well, it's going to be a shift register, obviously, because JTAG *is*
shift registers.  this looks like it's setting something, doesn't it?

 74     bs = yield from jtag_read_write_reg(dut, BS_SAMPLE, bslen, bs_actual)

 
> Do you know the sequence shifting in data? Or where I can find an example
> sequence? (Did not find such an example in jtag.py, jtagutils.py,
> test_jtag_tap.py, test_jtag_tap_srv.py)

 74     bs = yield from jtag_read_write_reg(dut, BS_SAMPLE, bslen, bs_actual)

> I tried:
> -Reseting the JTAG

if you do that everything will be lost (the shift registers returned
to default values)

> Sorry, I don't understand why your are checking for "vi" processes. Is this
> for when you have so many terminals open, that you just trying to find the
> right one? But then why count the number of entries with wc?

because when it gets to 1,350 as it did a few months ago i find it
hilariously funny.

(In reply to Luke Kenneth Casson Leighton from comment #75)
> (In reply to Andrey Miroshnikov from comment #74)
> that's adding the Record (IOConn) to an internal list, self._ios.
> that then, if you search for self._ios, lines 587, 590, 593, 595,
> 600, 601 and 602, you can see the Muxes.
Thanks, very useful.

> we see the *exact same data structure being added to a dictionary-version
> self.ios instead of a list-version self._ios.
> 
> therefore, logically, we may deduce that those *are* the IOConn Records,
> and that if they're not changing then you're doing something wrong.
JTAG has the ios signals as public, however I did not see those signals being brought out to Blinker's public ports.
Since the JTAG mechanism is working by using TDI however, I'll leave the deeper understanding till later.

> well, it's going to be a shift register, obviously, because JTAG *is*
> shift registers.  this looks like it's setting something, doesn't it?
> 
>  74     bs = yield from jtag_read_write_reg(dut, BS_SAMPLE, bslen, bs_actual)
JTAG also has additional registers (like Instruction Register), so I thought that some special address has to be specified in order to propagate the test bitstream into the actual shift register used for the Blinker's core/pad signals.

I'm honestly amazed how deceptively simple it is. Again I was overcomplicating.

Just one command!

Also, for the BS_SAMPLE parameter (address), I set it to be 0. Is this meant to be anything else?


After trying jtag_read_write_reg() with a few test bitstream, I was able to alter the states of the signals normally controlled by the core.

At the same time, after setting the pad inputs (uart rx, gpio i, etc.), jtag_read_write_reg() also shows that those signals are set.

Here's the jtag bs test so far:
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=a73da8179078e8e67879fd04dd3c1da29ba67fda;hb=823c97d150034ced5e10ceaf36d54eb626c642f7#l645

Now I'll add asserts to the test by verifying:
-Signals set by JTAG shift register have the right values
-JTAG output shows correct values for the input pad signals

For this I want to use the ios dict, as it will allow to easily decode the JTAG output bistream (e.g. uart rx pin corresponds to bit0 of the shift register and bit19 of the output word).

(In reply to Andrey Miroshnikov from comment #76)

> JTAG has the ios signals as public, however I did not see those signals
> being brought out to Blinker's public ports.

no, and you shouldn't be trying to access them publicly, only via the
public API which is JTAG TDI/TDO.

> Since the JTAG mechanism is working by using TDI however, I'll leave the
> deeper understanding till later.

sensible.  it'll emerge, over time.

> I'm honestly amazed how deceptively simple it is. Again I was
> overcomplicating.

it's easy to do.  this *has* to be simple, because the entire ASIC is
critically dependent on this.

> Just one command!
> 
> Also, for the BS_SAMPLE parameter (address), I set it to be 0. Is this meant
> to be anything else?

see how these match up?

https://git.libre-soc.org/?p=c4m-jtag.git;a=blob;f=c4m/nmigen/jtag/tap.py;h=1f3d424cbd7451c0434e0c71168f5aa0935af860;hb=c2bf4810f9f91ced7fcda777b92b86ab353da288#l357

 356         # TODO: Make commands numbers configurable
 357         cmd_extest = 0
 358         cmd_intest = 0
 359         cmd_idcode = 1
 360         cmd_sample = 2
 361         cmd_preload = 2
 362         cmd_bypass = 2**ir_width - 1 # All ones

https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/test_jtag_tap_srv.py;h=1ecbd3bad496e29cd29dc7cd19aab71dea2471e0;hb=823c97d150034ced5e10ceaf36d54eb626c642f7#l42

  42 # JTAG boundary scan reg addresses
  43 BS_EXTEST = 0
  44 BS_INTEST = 0
  45 BS_SAMPLE = 2
  46 BS_PRELOAD = 2

generally, having magic constants for addresses is frowned on.

ok, a *Software Engineer* looks on magic constants with either horror
or irritation. sometimes amusement.
 
> After trying jtag_read_write_reg() with a few test bitstream, I was able to
> alter the states of the signals normally controlled by the core.

awwwesome.  quite an amazing feeling, isn't it?
  
> For this I want to use the ios dict, as it will allow to easily decode the
> JTAG output bistream (e.g. uart rx pin corresponds to bit0 of the shift
> register and bit19 of the output word).

do make sure to use the names because i've realised over the past couple
of days that create_resources() has to merge with create_pinouts().
long story, will explain on IRC.

(In reply to Luke Kenneth Casson Leighton from comment #77)
> (In reply to Andrey Miroshnikov from comment #76)
>   42 # JTAG boundary scan reg addresses
>   43 BS_EXTEST = 0
>   44 BS_INTEST = 0
>   45 BS_SAMPLE = 2
>   46 BS_PRELOAD = 2
After some experimentation, and discussion here: https://libre-soc.org/irclog/%23libre-soc.2021-12-11.log.html

figured out the main test modes:
-BS_SAMPLE: core/pad are connected together, the TDI data has no effect on the peripherals. In this case JTAG is ONLY used for *sampling* the state of the I/O connected the JTAG's shift register.
EX_TEST/IN_TEST: core/pad connections are broken, JTAG has full control over the state of core inputs and pad outputs. In this case the TDI data has to be tailored to the desired test. At the same time TDO returns the JTAG shift register data.

> 
> generally, having magic constants for addresses is frowned on.
Indeed, but we'll probably have to bare with them in c4m-jtag for now.

> 
> ok, a *Software Engineer* looks on magic constants with either horror
> or irritation. sometimes amusement.
:')

> awwwesome.  quite an amazing feeling, isn't it?
Very much so!

> do make sure to use the names because i've realised over the past couple
> of days that create_resources() has to merge with create_pinouts().
> long story, will explain on IRC.
Yes, this is still in progress, I'll be implementing it once I get back.

So far there are four test cases within the jtag test:
-Before the peripherals are exercised:
1. Send 0xFFFFF via TDI in EX_TEST mode
2. Send 0xFFFFF via TDO in BS_SAMPLE mode
-After peripherals tested (all pad inputs and core outputs remain asserted)
3. Send 0x00000 via TDI in EX_TEST mode
4. Send 0x00000 via TDI in BS_SAMPLE mode

The expected results are:
1. All core outputs and pad inputs should be low. All core inputs and core outputs should be high (as these come from JTAG).
2. All signals should be low (as JTAG TDI is ignored, and nothing has been asserted yet).
3. All pad inputs and core output enables should remain asserted. Core output's and pad input's low.
4. All signals should be high.

Asserts have not been added, but I started writing some code (commented out), which will test outputs/inputs by detecting '__o' or '__i' in the string of the signal name.

Latest commit:
https://git.libre-soc.org/?p=pinmux.git;a=blob;f=src/spec/testing_stage1.py;h=c05ed5e6e82784d6a9d4cfe1ace24f39c0fec984;hb=8026e22d89673d81d463d9faea54a76f76522c47#l653

Things I haven't considered:
-JTAG register retains data given by TDI, so should be cleared before each test case. Leaving it in may affect the system when switching to EX_TEST mode.
-Some of the combinatorial logic added in GPIOs/I2C/UART will need to be moved to dedicated tests, so that JTAG can fully control all signals.

For now I will take a backseat (pardon the pun), as I will be getting ready for my driving test. 21st onwards I'll return to work full time again.
During the next week and a half I'll probably jot things down or do simple clean up (work that doesn't require too much thinking).

(In reply to Andrey Miroshnikov from comment #78)
> (In reply to Luke Kenneth Casson Leighton from comment #77)
> > do make sure to use the names because i've realised over the past couple
> > of days that create_resources() has to merge with create_pinouts().
> > long story, will explain on IRC.
> Yes, this is still in progress, I'll be implementing it once I get back.
> ...
> Asserts have not been added, but I started writing some code (commented
> out), which will test outputs/inputs by detecting '__o' or '__i' in the
> string of the signal name.
Currently I wrote a function for checking that inputs and outputs match the expected values for the four test cases I mention in comment #78 . Currently cleaning up.

> -JTAG register retains data given by TDI, so should be cleared before each
> test case. Leaving it in may affect the system when switching to EX_TEST
> mode.
Added BS chain reset after every test case.

> -Some of the combinatorial logic added in GPIOs/I2C/UART will need to be
> moved to dedicated tests, so that JTAG can fully control all signals.
This is not required, as values of core_input can be deduced from core_output.


After conversation with Luke (starts at 17:34:47, https://libre-soc.org/irclog/%23libre-soc.2022-01-06.log.html) had the discussion on the I/O mux block (iomb).

I drew a diagram of the iomb as well as how to build larger bank select muxes from it. The diagram can be found in the IRC log, however it is a rough and temporary sketch, so I'll make a better version and put it in the wiki later (and link it with a new comment).

Now Luke has clarified that I need to extend the simple_gpio (https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/bus/simple_gpio.py;hb=HEAD) to have:
-Input and oe signals in addition to output
-Direction selection (input/output)
-Add bank select signal
-Add CSR write command to affect the direction and bank of the GPIO

So far I only copied the simple_gpio to the pinmux repo. Tomorrow I will continue working at it.

(In reply to Andrey Miroshnikov from comment #79)
> (In reply to Andrey Miroshnikov from comment #78)
> > (In reply to Luke Kenneth Casson Leighton from comment #77)
> > > do make sure to use the names because i've realised over the past couple
> > > of days that create_resources() has to merge with create_pinouts().
> > > long story, will explain on IRC.
> > Yes, this is still in progress, I'll be implementing it once I get back.
> > ...
> > Asserts have not been added, but I started writing some code (commented
> > out), which will test outputs/inputs by detecting '__o' or '__i' in the
> > string of the signal name.
> Currently I wrote a function for checking that inputs and outputs match the
> expected values for the four test cases I mention in comment #78 . Currently
> cleaning up.
> 
> > -JTAG register retains data given by TDI, so should be cleared before each
> > test case. Leaving it in may affect the system when switching to EX_TEST
> > mode.
> Added BS chain reset after every test case.
> 
> > -Some of the combinatorial logic added in GPIOs/I2C/UART will need to be
> > moved to dedicated tests, so that JTAG can fully control all signals.
> This is not required, as values of core_input can be deduced from
> core_output.
> 
> 
> After conversation with Luke (starts at 17:34:47,
> https://libre-soc.org/irclog/%23libre-soc.2022-01-06.log.html) had the
> discussion on the I/O mux block (iomb).
> 
> I drew a diagram of the iomb as well as how to build larger bank select
> muxes from it. The diagram can be found in the IRC log, however it is a
> rough and temporary sketch,

please put it in the wiki straight away

> o I'll make a better version and put it in the
> wiki later (and link it with a new comment).

don't worry about "better". this prevents understanding because
of a forced and false dependency, waiting for "better".  judgement of
quality ahould never be a deciding factor in whether to publish.

you might find soneone else writes the diagram and saves time.

if however you fail to publish the "s***" version that cannot happen

this is why you see hand drawn diagrams in the wiki.  6 months later
someone upgrades them.

> Now Luke has clarified that I need to extend the simple_gpio
> (https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/bus/simple_gpio.py;
> hb=HEAD) to have:
> -Input and oe signals in addition to output
> -Direction selection (input/output)
> -Add bank select signal
> -Add CSR write command to affect the direction and bank of the GPIO
> 
> So far I only copied the simple_gpio to the pinmux repo. Tomorrow I will
> continue working at it.

great.

if you can start a wiki page describing the bits and how they will be
addressed this will greatly help review and understanding.

also can refer to the page in the source code.

there is an ericsson linux gpio presentation on elinux.org about what
i talked about yesterday, it should go in the wiki as well

(In reply to Luke Kenneth Casson Leighton from comment #80) 
> please put it in the wiki straight away
Done.


> you might find soneone else writes the diagram and saves time.
> 
> if however you fail to publish the "s***" version that cannot happen
> 
> this is why you see hand drawn diagrams in the wiki.  6 months later
> someone upgrades them.
Makes sense.

> if you can start a wiki page describing the bits and how they will be
> addressed this will greatly help review and understanding.
> 
> also can refer to the page in the source code.
> 
> there is an ericsson linux gpio presentation on elinux.org about what
> i talked about yesterday, it should go in the wiki as well
Sure, will add a page on this. Should help clarify my understanding as well XD

After fixing an assert issue with my JTAG unit tests, continued working on a simple GPIO block (took a while to think about how to control it).
Still sorting out the kink with setting the gpio inputs, will get that fixed tomorrow.

Will document the gpio block tomorrow on the wiki page as well, latest code has been pushed however.

On another note, it's probably a little early, but I wanted to start looking at the budget allocation. Can some of the pinmux budget be allocated to my existing and current work? Not in a hurry, but at least by the end of the month wanted to do an RFP.

By the end of the week I want to make further progress, as well as start looking at the muxing aspect.

(In reply to Andrey Miroshnikov from comment #82)

> Will document the gpio block tomorrow on the wiki page as well, latest code
> has been pushed however.

excellent
 
> On another note, it's probably a little early, but I wanted to start looking
> at the budget allocation. Can some of the pinmux budget be allocated to my
> existing and current work? Not in a hurry, but at least by the end of the
> month wanted to do an RFP.

yes of course, we should have created a bugreport a lot sooner and discussed
this under that, otherwise this bugreport gets overloaded (as does the
server)

create a bugreport describing what you've done so far
(and a separate one for the documentation) then work out how to bring
that first one to a "natural stop" so that it can be closed.  continuing
with muxing should be under *another* (3rd) bugreport, unless you want
to include the documentation of muxing *with* the actual work on muxing,
that's up to you.

The totals were goofed today, I double checked the history and fixed it, assuming the amounts assigned to the child tasks are correct.