On Sun, 20 Feb 2022 at 15:37, Amir Gonnen <[email protected]> wrote: > I can split the VIC from the core+EIC changes, although the core+EIC changes > make little sense without a VIC to interact with them. > However, I'm not sure how to split the changes to the nios2 core into > multiple patches. > The shadow register set, together with the EIC interface and interrupt > handling are very much tied together. > > Regarding the "fixed eret" - perhaps I didn't phrase it right. What I meant > is that eret was changed to work correctly in the presence of a shadow > register set. > So, the changes to eret are part of the shadow register set support on the > core and cannot exist on their own.
> > https://www.qemu.org/docs/master/devel/submitting-a-patch.html > > is our guidelines on patch formatting. > > In fact, I tried to follow them. I've also run checkpatch.pl, etc. > Could you please point out where I failed to follow them or what I'm missing? Mostly it was the combination of two things: (1) a large patch that touches both device and cpu code (2) a commit message that lists half a dozen things with very low level of detail This triggers my "probably needs to be split" heuristic, and also "probably somebody's first patch". So I mentioned the URL in case you hadn't seen it yet. > I tested this on Neuroblade board with JUART. I did not wire it to an > existing demo board. I think we'd like an upstream board that uses this. Otherwise it's dead code, from our point of view. That said, my suggested split would be something like: * VIC device model * CPU changes, in digestible chunks -- these should all be conditional on the "behave the same as the old code" default value of intc_present, I assume, so it doesn't matter if they don't all arrive in the codebase in the same commit. If there are any changes which *do* change behaviour even for intc_present=true, they definitely need to be split out, anyway. * board changes to use the new device thanks -- PMM
