[Libre-soc-bugs] [Bug 519] Get output from ulx3s serial port to show up in minicom

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=519

--- Comment #4 from Cole Poirier <colepoirier at gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #2)
> (In reply to Cole Poirier from comment #1)
> > Hi Luke, having some difficulty. There is no longer a cpu_variant
> > standardjtag, 
> 
> why do you feel it is necessary to remove a variant? do you think it would
> be a good idea to stop code from working by removing one of the options?

Nope, I’m being very cautious, that’s why I did’t commit it even though you
said “commit this line”, because I knew doing so had the potential to royally
mess up the code. I remember the rule. Don’t commit something that could break
code that is depended on. That’s why I was planning on getting your explicit
approval of the whole diff again after I had removed the changes you denied but
kept the ones you approved. That’s actually why I created this bug report.
Because it’s the appropriate place to review code and has the ability to upload
patch files. I was mistakenly using the mailing list for something other than
it’s intended purpose yesterday because I forgot/didn’t realize that this was
meant to be done as a bug :) 

After making the changes I then tested it to make sure it still worked and
thinking carefully about how changing one of the variants could have
ramifications for other code was planning on doing a series of tests to make
sure the code still worked as it did before my changes. Perhaps I should write
a unit test/some asserts... Anyhow, it failed with the assertion error
“standardjtag not in CPU_VARIANTS” on the first test which was running
“./versa_ecp5 —-build —-fpga ulx3s85f” because you approved the change where
standardjtag was changed to standardjtaggpiotest in
litex/florent/libresoc/core.py and litex/florent/sim.py but denied the change
where the same was done in versa_ecp5.py (see exact email where you said this
below). I was pretty sure you didn’t actually approve the changes I made that
you said commit because you misread my diff to be adding a variant not
modifying a variant. That’s why I’m coming to you for clarification because of
this miscommunication.

> > it has become standardjtaggpiotest.
> 
> why?

Because you said that that diff was ok to commit, however I didn’t commit it
because I tried testing it and it didn’t work, because as it stands there’s an
inconsistency.

> > You have instructed me to
> > not make the below change (versa_ecp5.py), but to make the bottom-most
> > change that makes standardjtag into standardjtaggpiotest (sim.py). 
> 
> no i did not.

If so I am confused see below in response to your ‘where did I say “remove
it”?’ You didn’t explicitly say remove it because it was a diff you were
commenting on. But approval of a diff where the string was removed and replaced
with the same string but new characters appended to it, is strong implication
(not implicature as most people use ‘implies’ to refer to) of this. In one of
the files you approved the change, in the other you denied it. That’s the part
I’m finding confusing and trying to resolve.

> > This
> > seems like a contradiction to me,
> 
> where did i say "remove it"?

This email
http://lists.libre-riscv.org/pipermail/libre-soc-dev/2020-October/000963.html

Are we on the same page now or is there still a
misunderstanding/miscommunication?

```

read carefully.

> ```
> diff --git a/src/soc/litex/florent/libresoc/core.py
> b/src/soc/litex/florent/libresoc/core.py
> index 81bd0dfc..10ad9396 100644
> --- a/src/soc/litex/florent/libresoc/core.py
> +++ b/src/soc/litex/florent/libresoc/core.py
> @@ -13,7 +13,7 @@ from libresoc.ls180 import io
>
> -CPU_VARIANTS = ["standard", "standard32", "standardjtag", "ls180",
> +CPU_VARIANTS = ["standard", "standard32", "standardjtaggpiotest", "ls180",
>                  "standardjtagnoirq"]

yes.  commit this one line change with an explanation (and the sim.py
below) in the same commit.

do NOT use "git commit -a".

>
> diff --git a/src/soc/litex/florent/sim.py b/src/soc/litex/florent/sim.py
> index c797a43d..bce505d2 100755
> --- a/src/soc/litex/florent/sim.py
> +++ b/src/soc/litex/florent/sim.py
> @@ -49,7 +49,7 @@ class LibreSoCSim(SoCSDRAM):
>          if cpu_data_width == 32:
>              variant = "standard32"
>          else:
> -            variant = "standardjtag"
> +            variant = "standardjtaggpiotest"

yes.

> diff --git a/src/soc/litex/florent/versa_ecp5.py
> b/src/soc/litex/florent/versa_ecp5.py
> index 8774b849..a001476e 100755
> --- a/src/soc/litex/florent/versa_ecp5.py
> +++ b/src/soc/litex/florent/versa_ecp5.py
> @@ -69,7 +69,7 @@ class ULX3S85FTestSoC(ulx3s.BaseSoC):
>              cpu_cls      = LibreSoC,
> -            cpu_variant  = "standardjtag",
> +            cpu_variant  = "standardjtaggpiotest",

no.
```

-- 
You are receiving this mail because:
You are on the CC list for the bug.