Fix handling of CRC start and valid signals

[John-K]

As of version 0.5.2, the gateware implementation of CRC depends on the user asserting and then de-asserting the start and valid signals manually after exactly one clock cycle, or else the operation will be executed on each subsequent clock cycle.

This behavior is undesirable as it may be convenient for the user to wait for more than one clock cycle before interfacing with the CRC module again.

This PR automatically resets the start and valid signals after they have been processed once. Existing implementations which de-assert these signals manually should not be affected by this change.

[John-K]

It looks like un-successful checks are due to recent Python 3.8 removal

[jeanthom]

Hi, I'm not very familiar with the CRC implementation in Amaranth, but I think there are two issues with those changes:

• That would make valid and start registers, preventing them to be driven from combinatorial logic.
• These signals would be driven both from inside and outside the Elaboratable, which is not a usual design pattern, and could cause potential multi-driver issues.

[wanda-phi]

sorry, this PR will not be merged; the gateware is working as intended and following a well-established convention. the proposed change would break any usage where the user has valid data to process on two consecutive cycles, which is rather common.

[John-K]

Hi, I'm not very familiar with the CRC implementation in Amaranth, but I think there are two issues with those changes:

* That would make valid and start registers, preventing them to be driven from combinatorial logic.

* These signals would be driven both from inside and outside the Elaboratable, which is not a usual design pattern, and could cause potential multi-driver issues.

Thank you for the insights, you bring up some great points that I hadn't considered.

I'm coming from low-level firmware land where I'm used to the functionality I've added here from my experience in FW accessing MMIO space, but thinking more about it that functionality is better suited to a wrapper that may also have a fifo or other added functionality.

This been a good mini-lesson on how a slightly different approach is needed in gateware.

[John-K]

sorry, this PR will not be merged; the gateware is working as intended and following a well-established convention. the proposed change would break any usage where the user has valid data to process on two consecutive cycles, which is rather common.

Understood. Please excuse my inexperience in this space, I'm implementing my first project using Amaranth and am coming from a firmware background. Unfortunately, there aren't any good examples of projects using the CRC module (as gateware rather than software) that I could find.

I'm curious about the situation you describe, wouldn't a user be able to process data on two consecutive cycles as long as they set valid along with data? Or would this be indeterminate since we don't know the ordering of the user or CRC's setting of valid?

[whitequark]

Wanda is correct, an elaboratable shouldn't be assigning to its inputs.