glasgow icon indicating copy to clipboard operation
glasgow copied to clipboard

Hardware description documentation for Glasgow revC3

Open gsuberland opened this issue 10 months ago • 13 comments

Adds technical documentation for the Glasgow revC3 hardware.

Outstanding tasks:

  • [x] Format review (was written in markdown and then converted to rst, so might have some conversion errors)
  • [ ] Technical correctness review
  • [x] TODO item for VIO on hierarchical sheet symbol (search "todo" in document)
  • [x] TODO item for SYNC connector usage examples (search "todo" in document)

gsuberland avatar Mar 26 '24 07:03 gsuberland

@gsuberland By the way, if you turn on GH Actions and Pages in your fork, it'll build the docs automatically and you will have a public link you could show for others to see.

whitequark avatar Mar 26 '24 22:03 whitequark

Style question: should we go for "100mA" or "100 mA"? I believe that from a typesetting perspective, the latter is the correct option, and I've tried to consistently stick to it in applets. For example in memory-onfi:

Screenshot_20240420_115939

We should choose one option and use it consistently. I do agree in principle that separating the unit, as in "100 mA", feels appropriate to the nature of the format: the value and the unit are separate things. I also acknowledge that in engineering it's very common to glue them together, and there is a certain practicality to it.

whitequark avatar Apr 20 '24 12:04 whitequark

Separated (e.g: "100 mA") is technically correct, and I'd agree to stick with that.

And it would also be good to fix on capital 'V' for Volts - "5 V" and "500 mV"... (even if "5v" feels more natural to me)

attie avatar Apr 20 '24 12:04 attie

And it would also be good to fix on capital 'V' for Volts - "5 V" and "500 mV"... (even if "5v" feels more natural to me)

We shouldn't have "5v" anywhere unless I missed some--I will insist on proper capitalization for units since "5v" could as well be "multiply variable v by 5".

whitequark avatar Apr 20 '24 12:04 whitequark

Regarding the units, saying 5 V, 2 A, 100 Ω, 4.7 uF, etc. feels very weird to me, at least in terms of prioritising readability in documentation.

Since units are technically applied as a product (e.g. 20mA equals "20 multiplied by 10^-3 multiplied by one amp), both in the case of numeric values and compound units, it always felt correct and natural to me to combine them without spacing (or with a • between component SI units for readability, in the case of compound units).

The other downside to spacing them is that word wrap can end up separating the value from its unit.

I'll aquiesce if others have a strong opinion in the other direction, but I personally have a significant preference for no spaces.

Regarding differences between the way units are written in the documentation and applet output, I think it's ok to have an inconsistency if there's a justification for it, and the difference in notation is itself consistent, e.g. if we felt that spaced units read better in the case of terminal output in a likely-monospaced display context, but non-spaced units read better in continuous prose within documentation. I don't have any strong opinion one way or the other on this front though.

gsuberland avatar Apr 20 '24 23:04 gsuberland

The other downside to spacing them is that word wrap can end up separating the value from its unit.

We can fix this with a post-processing stage that adds  ; it looks like it's fairly easy to bulk post-process the HTML.

whitequark avatar Apr 21 '24 00:04 whitequark

Additional thought: the docs refer to the power rails by net names (e.g. "the +1.2V rail" meaning the power net with the name +1.2V) so we'd end up using non-spaced measurements there anyway.

gsuberland avatar Apr 21 '24 00:04 gsuberland

Additional thought: the docs refer to the power rails by net names (e.g. "the +1.2V rail" meaning the power net with the name +1.2V) so we'd end up using non-spaced measurements there anyway.

I think this is an argument in favor of adding spaces in measurements. Here's why:

  • +1.2V is then clearly an identifier (of a net, in this case). It is not a measurement on that net, and indeed, you could in principle have 3.3 V on +1.2V. (The iCE40 even survives that, inexplicably. It stops being low power though)
  • +1.2 V is a measurement.

Of course there is value in distinguishing these further (we don't really have negative voltages so most measurements would be just 1.2 V, and we also have HTML markup), but I like having this as a baseline.

whitequark avatar Apr 21 '24 00:04 whitequark

I was about to edit to mention that upside, but you beat me to it :)

Seems like spacing is the way we're going so I'll make the changes.

gsuberland avatar Apr 21 '24 01:04 gsuberland

Thanks! That definitely makes me feel a lot better about just hitting Merge on this after technical review.

whitequark avatar Apr 21 '24 01:04 whitequark

Cool, should all be good to go at this point. I've given it a good read through and can't spot any obvious faults at this point.

Do we want to ping any specific folks for a final technical review?

gsuberland avatar Apr 21 '24 01:04 gsuberland

Also, once you are done, can you rebase/squash please, with our standard commit message format? I'll then add one more commit linking up the page from revisions.rst and it's good to go!

whitequark avatar May 13 '24 05:05 whitequark

Also I think you'd have to rebase on tip of main either way due to the way our CI works

whitequark avatar May 13 '24 05:05 whitequark

Got fed up of fighting with git so I moved all changes to a new branch and opened #669.

gsuberland avatar Aug 18 '24 23:08 gsuberland

Superseded by #669. Thanks!

whitequark avatar Aug 19 '24 08:08 whitequark