[doc] Error/warning on non-integer period on wave about opentitan HOT 5 OPEN

I researched this a bit. I didn't go deep enough into the wavesvg.py
code but it appears to be visually doing the right thing by averaging
down on the clock where it needs to. The offending line in the source
wavedrom is this within spi_device.md

{ signal: [
  { name: 'SCK', wave: '0...p.|.|...|l' , node:'.............a', period:0.5},
}

Other uses of period:0.5 are fine, but because of the 'p' on the line, it
is requesting a clock. The clock is rendered as if period 1, but the rest
of the labeling needs to be 0.5 to align with the remainder of the waveform.

I recommend we change this to a warning or drop. But perhaps someone
should dig deeper. Wavedrom is WYSIWYG so if it looks good, don't
change it. A warning could alert you to being outside of range though.

from opentitan.

Downgrade to P3. @sjgitty can you assign someone to update docgen.md to make this a WARNING, instead of ERROR, since the generated waveform looks ok? Thanks.

from opentitan.

Well, this might just get raised again. Hugo is pointing to the official
version of wavedrom so the 0.5 support might vanish all together.
Let's leave and see if @gkelly has any thoughts in post-hugo landscape.

from opentitan.

I had a look into this and I'm not completely sure I get it.
According to wavedrom schema, period can only be a natural number, and the clock indicator should be used only at the beginning of the string.
So my guess is we ignored those "rules" because it does work and the images look good?
And the reason for this was to have the extended cycle with the gap, which resulted in two cycles?
An alternative could be to use the barely documented sub-cycle feature wavedrom/wavedrom#243 but I think it would still require some upstream work in order to remove some artifacts which are the result of not having the clock indicator at the first position and some sub-cycle resetting.

Current:

Modified:

diff --git a/hw/ip/spi_device/doc/_index.md b/hw/ip/spi_device/doc/_index.md
index 25557ed6..74e9ae49 100644
--- a/hw/ip/spi_device/doc/_index.md
+++ b/hw/ip/spi_device/doc/_index.md
@@ -87,18 +87,18 @@ edges, polarities, and bit orders are described later.
 
 {{< wavejson >}}
 { signal: [
-  { name: 'CSB',  wave: '10.........|....1.'},
-  { name: 'SCK',  wave: '0.p........|....l.'},
-  { name: 'MOSI', wave: 'z.=..=.=.=.=.=.=.=.=.=|=.=.=.=.z....',
+  { name: 'CSB',  wave: '10........<0|>0...1.'},
+  { name: 'SCK',  wave: '0n........<n|>n...l.'},
+  { name: 'MOSI', wave: 'z=========<=|>====z.',
     data:['R07','R06','R05','R04','R03','R02','R01','R00','R17',
-          '','R73','R72','R71','R70'], period:0.5, },
-  { name: 'MISO', wave: 'z.=..=.=.=.=.=.=.=.=.=|=.=.=.=.z....',
+          '','R73','R72','R71','R70'], },
+  { name: 'MISO', wave: 'z=========<=|>====z.',
     data:['T07','T06','T05','T04','T03','T02','T01','T00','T17',
-          '','T73','T72','T71','T70'], period:0.5}],
+          '','T73','T72','T71','T70'], }],
   head:{
     text: 'Data Transfer',
-    tick: ['-2 -1 0 1 2 3 4 5 6 7 8 9 60 61 62 63     ']
-  }
+    tock: ['-1 0 1 2 3 4 5 6 7 8  60 61 62 63   ']
+    }
 }
 {{< /wavejson >}}

from opentitan.

@towoe yes you have the right way of looking at it. We used the half period because it happened to work, but gave the error in these few cases. I don't know if we care enough tbh to fix the source, but your proposed change looks "good enough". You could even remove the < and > for CSB to get rid of the unsightly "glitch", though it makes the S slightly misaligned.

from opentitan.