glasgow icon indicating copy to clipboard operation
glasgow copied to clipboard

Bring revC2 firmware to the same level as revC1

Open electroniceel opened this issue 4 years ago • 6 comments

These commits bring the firmware for revC2 to the same level as revC1. This means you can read voltages, set alert levels, read alert levels, read alert status. Also alerts are properly handeled and the Vio switched off. After an alert was cleared, you can re-arm the alert.

This does not yet make use of the advanced features of the INA233, e.g. current measurement, current alerts or the enhanced voltage range of up to 36V. This will be developed at a later stage.

These changes do not alter the control request interface, so no API level change required.

This is the first part of #219

electroniceel avatar Dec 05 '20 23:12 electroniceel

I've cherry-picked the first three commits that are unobjectionable and nicely improve our level of support for revC2.

whitequark avatar Dec 27 '20 07:12 whitequark

The mentioned issues should be resolved now

electroniceel avatar Dec 28 '20 19:12 electroniceel

Cherry-picked the ~ALERT commit. Regarding the cache, do you think we can poll and clear the alert in one operation while recording what happened, and then communicating that to the USB host separately? Essentially, making the "cache" logic common for all revisions and moving it to main.c. This is just an idea and if it doesn't make much sense, please say so.

whitequark avatar Dec 29 '20 02:12 whitequark

Regarding the cache, do you think we can poll and clear the alert in one operation while recording what happened,

Poll and clear in one operation won't work on revC2: when you clear the ~ALERT, you re-enable the Vio that was previously disabled by the hardwired logic. You have to disable the voltage regulator first. And for this you have to know which one, that means polling the status.

So poll and clear have to stay separate.

Essentially, making the "cache" logic common for all revisions and moving it to main.c.

I don't think that the cache for the alert status is that bad, so it could be made the default. IIRC, your main argument against the cache is that it could go out of sync with the hardware. But if we reset the adcs thoroughly during init in main (like I do now with the INA233), I don't see how that could happen.

If we move it to main, we'd have to translate it from the adc specific bits to a generic format though. Also main doesn't know about the ports yet, it just deals with port-masks. So we'd have to introduce a small array for the ports to main.

electroniceel avatar Dec 29 '20 10:12 electroniceel

OK, I think this is probably fine. Can you rebase? Then I'll give it another look and probably merge.

whitequark avatar Dec 29 '20 10:12 whitequark

I now think the cache and reset logic used in this PR should stay and be used for revC3 too, see https://github.com/GlasgowEmbedded/glasgow/issues/221#issuecomment-757494226 for details

electroniceel avatar Jan 10 '21 15:01 electroniceel