Closed enjoy-digital closed 4 years ago
@enjoy-digital I see one problem here - we won't be able to configure the clock generator in run time to generate a clock faster than reference. This functionality is needed in e.g. video input/output (when we change the resolution).
@kgugala: sorry, i was only suggesting these simplifications for LiteSDCard (unless you are using it for video application :)). For LiteSDCard, we don't have strong frequency requirements as it's the case for video, so using a clock divider would be fine. The MMCM and linux driver for it would not be changed and could still be used for video, it's just that it removes this dependency for LiteSDCard and it allows simplifications while also simplifying support of other devices.
oh, sorry my bad :) I got a link and didn't really notice that this is SD card only. For SD card I think this is OK.
@enjoy-digital -- for my part, I'd vote for keeping things as simple as they can be, so +1 from me.
Once the gateware settles, I'll work on making the Linux driver a bit more flexible w.r.t. 32-bit csr_data_width
(right now it's hard-coded for 8 bits throughout).
Thanks!
The Core/PHY has been simplified a lot and most of it now works in sys_clk
domain, avoiding the complex synchronization that were required before (while also reducing footprint). The Clocker is also now working fromsys_clk
and can generate divided version of it (only supports powers or 2) from div=2 to div=256. This avoid complex control of specific clocking components (DCM, MMCM, PLL) and make the code generic/portable (the exact same code is already running on ECP5 and 7-series).
The current version of the code uses dynamic re-configuration of a DCM, PLL, MMCM to generate the dynamic clocking. This was introduced to test the core with fine frequency steps and verify the evaluate the max frequencies but this complicates a lot the gateware/firmware/linux drivers and we could probably replace it with a simple clock divider in the core that would use the
sys_clk
of the SoC or a provided reference (50MHz?).This would mean that:
@mateusz-holenko, @gsomlo what are your thoughts on this? I could do the change in the gateware/bios if you are fine with this. We would then need to also update the Linux driver, but it should not be complicated.