search

evka85 / GEM_AMC

GEM uTCA AMC common firmware logic as well as board specific implementations for GLIB (Virtex 6) and CTP7 (Virtex 7)
0 stars 11 forks source link

Use generate for VFAT ChanReg #9

Closed jsturdy closed 7 years ago

jsturdy commented 7 years ago
jsturdy commented 7 years ago
evka85 commented 7 years ago

Hi Jared,

Removing the mask="0xffffffff" does impact the register generation in the firmware, but I already have a simple fix for that in the python script.

The bigger issue is that I see that you changed the permissions in some places e.g. TTC.CTRL.MODULE_RESET permission was write-only, but you changed it to read/write. Is there a reason behind it? I'm asking because this impacts how the registers are generated in the firmware and especially these registers that don't have fw_signal, but instead have fw_write_pulse, they cannot be read by design, because there's actually no value in the firmware, instead just a pulse is generated whenever you try to write anything to that address, so adding a read permission breaks it because it tries to connect to a value signal that doesn't exist.. Is it easy to revert all your permissions changes? (at least in the non-OH registers)

Cheers, Evaldas

evka85 commented 7 years ago

ah, I guess I might know why you chose to add read permissions there.. Is it because the mask is not 0xffffffff and so IPbus tries to read it first? Is it doing that even if there's no read permission?

jsturdy commented 7 years ago

Right, if you actually have implemented anywhere where a read on a specific bit is not valid, then the tools will call that an invalid transaction (not only in IPbus). If its a bit within a read/write register where reading it has no effect, then no harm, no foul. If it's truly write only (as you've explained I believe), then the only way that can reasonably be implemented is as a full register (as a masked transaction will always be executed as RMW (read-modify-write) I think the ones I changed all were a full register where a single bit mask was defined, and nothing else was happening in that register, so this is prime example for different implementation in the address table (code generation and address table generation are only able to be linked to a point) If writing a 0 has no effect (i.e., writing a 1 is what triggers the pulse), then maybe we can set up a single register as write-only (where all these signals get connected), and have the software know which bits do which functions...

Just spitballing, I'll look at reverting these change in the interim

evka85 commented 7 years ago

Yeah, that's a good point. In fact those pulse registers don't have to be 1 bit, I'm not sure why I defined them like that.. Because the pulse in the firmware is triggered on any write request to that register address, it doesn't even look at the value, let alone individual bits. So I'm pretty sure we can simply remove the mask from these registers and make them write-only as they were before. Though I have to review each such register to make sure that's really the case in all of them (I think there's a few in JTAG module that have both value and a read/write pulse)

evka85 commented 7 years ago

So I looked through all the registers that have either fw_write_pulse_signal and are write-only and all of those actually don't need a mask. These are the only registers in CTP7 that are write-only, so we can safely remove the mask there and make them write-only again.

But I noticed that OH also has some write-only registers that have a mask (not sure if the mask is really needed there, but we should check with Thomas)..

jsturdy commented 7 years ago