RE: [PATCH] target/nios2: Shadow register set, EIC and VIC

[Amir Gonnen]

Hi Peter,

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.

I tested this on Neuroblade board with JUART. I did not wire it to an existing 
demo board.

> 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?

Thanks,
Amir

-----Original Message-----
From: Peter Maydell <peter.maydell@linaro.org>
Sent: Sunday, February 20, 2022 5:13 PM
To: Amir Gonnen <amir.gonnen@neuroblade.ai>
Cc: qemu-devel@nongnu.org; Chris Wulff <crwulff@gmail.com>; Marek Vasut 
<marex@denx.de>
Subject: Re: [PATCH] target/nios2: Shadow register set, EIC and VIC

[EXTERNAL]

On Sun, 20 Feb 2022 at 13:07, <amir.gonnen@neuroblade.ai> wrote:
>
> From 99efcd655e83f034bce25271fe592d8789529e54 Mon Sep 17 00:00:00 2001
> From: Amir Gonnen <amir.gonnen@neuroblade.ai>
> Date: Thu, 17 Feb 2022 15:43:14 +0200
> Subject: [PATCH] target/nios2: Shadow register set, EIC and VIC
>
> Implement nios2 Vectored Interrupt Controller (VIC).
> This includes Exteral Interrupt Controller interface (EIC) and Shadow
> Register set implementation on the nios2 cpu.
> Implemented missing wrprs and rdprs instructions, and fixed eret.
> Added intc_present property, true by default. When set to false, nios2
> uses the EIC interface when handling IRQ.

Hi; this patch seems to be trying to fix multiple things at once. Could you 
split it up into a multi-patch patch series, where each patch does one logical 
thing, please? In particular bug fixes to existing code ("fixed eret") should 
be their own patches, separate from patches adding new features.

> To use VIC, the nios2 board should set VIC cpu property, disable
> intc_present, connect VIC irq to cpu and connect VIC gpios.

Is there a patch which wires up one of the nios2 boards to do this ?

https://www.qemu.org/docs/master/devel/submitting-a-patch.html
is our guidelines on patch formatting.

thanks
-- PMM

The contents of this email message and any attachments are intended solely for 
the addressee(s) and may contain confidential and/or privileged information and 
may be legally protected from disclosure. If you are not the intended recipient 
of this message or their agent, or if this message has been addressed to you in 
error, please immediately alert the sender by reply email and then delete this 
message and any attachments. If you are not the intended recipient, you are 
hereby notified that any use, dissemination, copying, or storage of this 
message or its attachments is strictly prohibited.