Add gmiiSgmiiBridge based on gig_ethernet_pcs_pma

[lmbollen]

This pull requests adds a wrapper to generate the LogiCORE IP Ethernet 1000BASE-X PCS/PMA or SGMII as a GMII to SGMII bridge.

It generates a .tcl file to be used by the clashConnector based infra structure.
For now the configuration is minimal and static.

[DigitalBrains1]

Nice job figuring out the primitive!

But I do think there's room for improvement. CI has some errors for you as well.

[DigitalBrains1]

First, it occurred to me you don't provide a VHDL primitive. If this isn't a simple oversight, it should be documented.

I was thinking about the module structure. Clash.Cores.Xilinx.Ethernet.Gmii is almost empty; it's just two definitions. If people want to do anything with the core other than just passing through inputs and outputs literally, they need to import Clash.Cores.Xilinx.Ethernet.Gmii.Types.

Maybe you are accomodating the module name for future additions of other cores that could be considered GMII?

I wonder if this even needs to be more than one module. We could have the Clash.Cores.Xilinx.Ethernet.Gmii module as follows:

• gmiiSgmiiBridge
• Gmii
• DuplexMode
• LinkSpeed
• Pause
• AutoNegConfig
• Config
• Status
• BridgeOutput

And then a section Internal in the Haddock with the following:

• gmiiSgmiiBridgePrim
• AutoNegConfigVector
• toAutoNegConfigVector (if pack is insufficient)
• ConfigVector
• toConfigVector (if pack is insufficient)
• StatusVector
• fromStatusVector (if unpack is insufficient)
• tclTemplateFunction
• tclTemplate

In this configuration, you'd just put the InlineYamlPrimitive implementation directly inside the annotation as the GHC staging restriction prevents you from using a function defined in the same module.

Also, this is just in the order that it is currently in the Haddock; I'm not proposing an ordering here. Do that as you think makes sense to the reader.

Alternatively, all the things that are marked Internal above could go into a separate module Clash.Cores.Xilinx.Ethernet.Gmii.Internal that is hidden from Haddock (but still exposed in the library! We like giving everybody all functions to do with as they please, as long as they understand it's all internal.)

I'm also wondering a bit about the very generic name of the module. What if some day we want to add another thing that is GMII, but has a different definition for Config, Status, LinkSpeed, ...? Maybe the module name should be GmiiSgmiiBridge. It's a bit long, but it does enable adding other cores later without getting them all mixed up. Ah, no, wait, this specific core can be configured as other things as well. Ugh. Naming things is hard.

[DigitalBrains1]

I think this concludes my review, but mostly I just want to go home now :-).

[DigitalBrains1]

Christiaans very first review comment was

The module documentation header should be at the very top, before the LANGUAGE and OPTIONS_GHC pragmas

but I suddenly realise that the fix for that evidently did not survive the reorganisation of the files...

[DigitalBrains1]

Also, you forgot to hide the Internal module from Haddock.

[DigitalBrains1]

You didn't apply all my suggestions. If you disagree with them, could you please discuss instead of simply presenting it again for review?

[DigitalBrains1]

LGTM!

Changes implemented

[DigitalBrains1]

@lmbollen Could you open a PR with this change over on clash-lang/clash-cores so we can merge it there? This PR hit in an unfortunate time of limbo where this code still exists in clash-compiler but we already split off to the new repo before this PR.

[lmbollen]

@DigitalBrains1 clash-lang/clash-cores#11 :)
I simply added clash-compiler as remote locally and cherry-picked 20c1749

Which is the only commit that touches the code.