> -----Original Message----- > From: Aurelien Jarno [mailto:aurel...@aurel32.net] > Sent: Sunday, January 25, 2009 12:32 AM > To: Liu Yu-B13201 > Cc: qemu-de...@nongnu.org; holl...@us.ibm.com; kvm-ppc@vger.kernel.org > Subject: Re: [Qemu-devel] [PATCH 3/6] kvm/powerpc: Add irq > support for E500core > > On Thu, Jan 22, 2009 at 06:14:13PM +0800, Liu Yu wrote: > > Signed-off-by: Liu Yu <yu....@freescale.com> > > --- > > hw/ppc.c | 89 > +++++++++++++++++++++++++++++++++++++++++++ > > hw/ppc.h | 1 + > > target-ppc/cpu.h | 10 +++++ > > target-ppc/translate_init.c | 6 ++- > > 4 files changed, 104 insertions(+), 2 deletions(-) > > The same comment for indentation as in the previous patch also applies > here, though it concerns very few lines here. I also have a few minor > comments below, but otherwise the patch is ok.
Fixed. > > > diff --git a/hw/ppc.c b/hw/ppc.c > > index 05e787f..7a44951 100644 > > --- a/hw/ppc.c > > +++ b/hw/ppc.c > > @@ -314,6 +314,95 @@ void ppc40x_irq_init (CPUState *env) > > env, > PPC40x_INPUT_NB); > > } > > > > +/* PowerPC E500 internal IRQ controller */ > > +static void ppce500_set_irq (void *opaque, int pin, int level) > > +{ > > + CPUState *env = opaque; > > + int cur_level; > > + > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: env %p pin %d level %d\n", __func__, > > + env, pin, level); > > + } > > +#endif > > + cur_level = (env->irq_input_state >> pin) & 1; > > + /* Don't generate spurious events */ > > + if ((cur_level == 1 && level == 0) || (cur_level == 0 > && level != 0)) { > > + switch (pin) { > > + case PPCE500_INPUT_MCK: > > + if (level) { > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: reset the > PowerPC system\n", > > + __func__); > > + } > > +#endif > > You should probably use LOG_IRQ() instead here which does exactly the > same as the code between the #if and #endif. Fixed. > > > + fprintf(stderr,"PowerPC E500 reset core\n"); > > + qemu_system_reset_request(); > > + } > > + break; > > + case PPCE500_INPUT_RESET_CORE: > > + if (level) { > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: reset the > PowerPC core\n", __func__); > > + } > > +#endif > > + ppc_set_irq(env, PPC_INTERRUPT_MCK, level); > > + } > > + break; > > + case PPCE500_INPUT_CINT: > > + /* Level sensitive - active high */ > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: set the critical IRQ > state to %d\n", > > + __func__, level); > > + } > > +#endif > > Same here. Fixed. > > > + ppc_set_irq(env, PPC_INTERRUPT_CEXT, level); > > + break; > > + case PPCE500_INPUT_INT: > > + /* Level sensitive - active high */ > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: set the core IRQ > state to %d\n", > > + __func__, level); > > + } > > +#endif > > Same here. Fixed. > > > + ppc_set_irq(env, PPC_INTERRUPT_EXT, level); > > + break; > > + case PPCE500_INPUT_DEBUG: > > + /* Level sensitive - active high */ > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: set the debug pin > state to %d\n", > > + __func__, level); > > + } > > +#endif > > Same here. Fixed. > > > + ppc_set_irq(env, PPC_INTERRUPT_DEBUG, level); > > + break; > > + default: > > + /* Unknown pin - do nothing */ > > +#if defined(PPC_DEBUG_IRQ) > > + if (loglevel & CPU_LOG_INT) { > > + fprintf(logfile, "%s: unknown IRQ pin > %d\n", __func__, pin); > > + } > > +#endif > > Same here. Fixed. > > > + return; > > + } > > + if (level) > > + env->irq_input_state |= 1 << pin; > > + else > > + env->irq_input_state &= ~(1 << pin); > > + } > > +} > > + > > +void ppce500_irq_init (CPUState *env) > > +{ > > + env->irq_inputs = (void **)qemu_allocate_irqs(&ppce500_set_irq, > > + env, PPCE500_INPUT_NB); > > +} > > > /************************************************************* > ****************/ > > /* PowerPC time base and decrementer emulation */ > > struct ppc_tb_t { > > diff --git a/hw/ppc.h b/hw/ppc.h > > index 75eb11a..2ec4680 100644 > > --- a/hw/ppc.h > > +++ b/hw/ppc.h > > @@ -31,6 +31,7 @@ extern CPUReadMemoryFunc *PPC_io_read[]; > > void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val); > > > > void ppc40x_irq_init (CPUState *env); > > +void ppce500_irq_init (CPUState *env); > > void ppc6xx_irq_init (CPUState *env); > > void ppc970_irq_init (CPUState *env); > > > > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > > index f7a12da..0eb794f 100644 > > --- a/target-ppc/cpu.h > > +++ b/target-ppc/cpu.h > > @@ -1352,6 +1352,16 @@ enum { > > }; > > > > enum { > > + /* PowerPC E500 input pins */ > > + PPCE500_INPUT_RESET_CORE = 0, > > + PPCE500_INPUT_MCK = 1, > > + PPCE500_INPUT_CINT = 3, > > + PPCE500_INPUT_INT = 4, > > + PPCE500_INPUT_DEBUG = 6, > > + PPCE500_INPUT_NB, > > +}; > > + > > +enum { > > /* PowerPC 40x input pins */ > > PPC40x_INPUT_RESET_CORE = 0, > > PPC40x_INPUT_RESET_CHIP = 1, > > diff --git a/target-ppc/translate_init.c > b/target-ppc/translate_init.c > > index 5008a3a..7953c69 100644 > > --- a/target-ppc/translate_init.c > > +++ b/target-ppc/translate_init.c > > @@ -4154,7 +4154,8 @@ static void init_proc_e300 (CPUPPCState *env) > > POWERPC_FLAG_BUS_CLK) > > #define check_pow_e500 check_pow_hid0 > > > > -__attribute__ (( unused )) > > +extern void ppce500_irq_init (CPUState *env); > > + > > You should use PPC_IRQ_INIT_FN(e500) instead, see at the beginning of > translate_init.c. Fixed. > > > static void init_proc_e500 (CPUPPCState *env) > > { > > /* Time base */ > > @@ -4256,7 +4257,8 @@ static void init_proc_e500 (CPUPPCState *env) > > init_excp_e200(env); > > env->dcache_line_size = 32; > > env->icache_line_size = 32; > > - /* XXX: TODO: allocate internal IRQ controller */ > > + /* Allocate hardware IRQ controller */ > > + ppce500_irq_init(env); > > } > > > > /* Non-embedded PowerPC > */ > > -- > > 1.5.4 > > > > > > > > > > -- > Aurelien Jarno GPG: 1024D/F1BCDB73 > aurel...@aurel32.net http://www.aurel32.net > > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html