[PATCH] Update motorola_power to irq-generic interrupt management

Chris Johns chrisj at rtems.org
Fri Feb 12 21:52:52 UTC 2021


On 13/2/21 7:35 am, Gedare Bloom wrote:
> On Fri, Feb 12, 2021 at 1:00 PM <chrisj at rtems.org> wrote:
>>
>> From: Chris Johns <chrisj at rtems.org>
>>
>> - Add support to the BSP to enable irq-generic management
>>
>> - Update the powerpc shared irq code to support irq-generic. This
>>   is an option in option for existing powerpc bsps. This change
>>   should be simpler now
>>
>> - Fix a number of issues in ISA IRQ controller handling by porting
>>   fixes from the i386 (PC) BSP
>>
>> Closes #4238
>> Closes #4239
>> ---
>>  bsps/powerpc/include/bsp/irq_supp.h           |   5 +
>>  .../motorola_powerpc/include/bsp/irq.h        |  15 +-
>>  .../powerpc/motorola_powerpc/start/bspstart.c |   9 +-
>>  bsps/powerpc/shared/irq/i8259.c               | 155 ++++++++++++++----
>>  bsps/powerpc/shared/irq/irq_init.c            |  15 +-
>>  bsps/powerpc/shared/irq/openpic_i8259_irq.c   |  27 ++-
>>  bsps/powerpc/shared/irq/ppc-irq-generic.c     | 118 +++++++++++++
>>  .../bsps/powerpc/motorola_powerpc/grp.yml     |   2 +-
>>  .../bsps/powerpc/motorola_powerpc/obj.yml     |   3 +-
>>  9 files changed, 291 insertions(+), 58 deletions(-)
>>  create mode 100644 bsps/powerpc/shared/irq/ppc-irq-generic.c
>>
>> diff --git a/bsps/powerpc/include/bsp/irq_supp.h b/bsps/powerpc/include/bsp/irq_supp.h
>> index 65af48c87f..fbb16d6211 100644
>> --- a/bsps/powerpc/include/bsp/irq_supp.h
>> +++ b/bsps/powerpc/include/bsp/irq_supp.h
>> @@ -50,6 +50,11 @@ extern int  BSP_disable_irq_at_pic(const rtems_irq_number irqLine);
>>   */
>>  extern int  BSP_setup_the_pic(rtems_irq_global_settings* config);
>>
>> +/*
>> + * Set up for the irq-generic.h interface.
>> + */
>> +int BSP_rtems_irq_generic_set(rtems_irq_global_settings* config);
>> +
>>  /* IRQ dispatcher to be defined by the PIC driver; note that it MUST
>>   * implement shared interrupts.
>>   * Note also that the exception frame passed to this handler is not very
>> diff --git a/bsps/powerpc/motorola_powerpc/include/bsp/irq.h b/bsps/powerpc/motorola_powerpc/include/bsp/irq.h
>> index 3690dbbff7..cbb6ff69cf 100644
>> --- a/bsps/powerpc/motorola_powerpc/include/bsp/irq.h
>> +++ b/bsps/powerpc/motorola_powerpc/include/bsp/irq.h
>> @@ -19,9 +19,17 @@
>>  #ifndef BSP_POWERPC_IRQ_H
>>  #define BSP_POWERPC_IRQ_H
>>
>> +#ifndef BSP_SHARED_HANDLER_SUPPORT
>>  #define BSP_SHARED_HANDLER_SUPPORT      1
>> +#endif
>> +
>>  #include <rtems/irq.h>
>> -#include <bsp/irq-default.h>
>> +
>> +/*
>> + * Switch to using the generic support. Remove this when all BSPs have
>> + * been converted.
>> + */
>> +#define BSP_POWERPC_IRQ_GENERIC_SUPPORT 1
>>
>>  /*
>>   * 8259 edge/level control definitions at VIA
>> @@ -107,6 +115,8 @@ extern "C" {
>>  #define BSP_IRQ_NUMBER                 (BSP_MISC_IRQ_MAX_OFFSET + 1)
>>  #define BSP_LOWEST_OFFSET              (BSP_ISA_IRQ_LOWEST_OFFSET)
>>  #define BSP_MAX_OFFSET                 (BSP_MISC_IRQ_MAX_OFFSET)
>> +#define BSP_INTERRUPT_VECTOR_MIN       (BSP_LOWEST_OFFSET)
>> +#define BSP_INTERRUPT_VECTOR_MAX       (BSP_MAX_OFFSET)
>>  /*
>>   * Some ISA IRQ symbolic name definition
>>   */
>> @@ -191,6 +201,9 @@ int BSP_irq_ack_at_i8259s                   (const rtems_irq_number irqLine);
>>   */
>>  int BSP_irq_enabled_at_i8259s          (const rtems_irq_number irqLine);
>>
>> +unsigned short BSP_irq_suspend_i8259s(unsigned short mask);
>> +void BSP_irq_resume_i8259s(unsigned short in_progress_save);
>> +
>>  extern void BSP_rtems_irq_mng_init(unsigned cpuId);
>>  extern void BSP_i8259s_init(void);
>>
>> diff --git a/bsps/powerpc/motorola_powerpc/start/bspstart.c b/bsps/powerpc/motorola_powerpc/start/bspstart.c
>> index e74b02c446..ab48858c46 100644
>> --- a/bsps/powerpc/motorola_powerpc/start/bspstart.c
>> +++ b/bsps/powerpc/motorola_powerpc/start/bspstart.c
>> @@ -27,6 +27,7 @@
>>  #include <bsp/pci.h>
>>  #include <bsp/openpic.h>
>>  #include <bsp/irq.h>
>> +#include <bsp/irq-generic.h>
>>  #include <libcpu/bat.h>
>>  #include <libcpu/pte121.h>
>>  #include <libcpu/cpuIdent.h>
>> @@ -334,10 +335,8 @@ static void bsp_early( void )
>>     */
>>    bsp_clicks_per_usec   = BSP_bus_frequency/(BSP_time_base_divisor * 1000);
>>
>> -  /*
>> -   * Initalize RTEMS IRQ system
>> -   */
>> -  BSP_rtems_irq_mng_init(0);
>> +  /* Initalize interrupt support */
> s/Initalize/Initialize
> 
>> +  bsp_interrupt_initialize();
>>
>>    /* Activate the page table mappings only after
>>     * initializing interrupts because the irq_mng_init()
>> @@ -360,7 +359,6 @@ static void bsp_early( void )
>>    printk("Exit from bspstart\n");
>>  #endif
>>  }
>> -
> This blank line should not be removed

OK

> 
>>  RTEMS_SYSINIT_ITEM(
>>    bsp_early,
>>    RTEMS_SYSINIT_BSP_EARLY,
>> @@ -370,6 +368,7 @@ RTEMS_SYSINIT_ITEM(
>>  void bsp_start( void )
>>  {
>>    /* Initialization was done by bsp_early() */
>> +
> Not needed blank line
>>  }
>>
>>  RTEMS_SYSINIT_ITEM(
>> diff --git a/bsps/powerpc/shared/irq/i8259.c b/bsps/powerpc/shared/irq/i8259.c
>> index 7363e87ba0..6a0b855981 100644
>> --- a/bsps/powerpc/shared/irq/i8259.c
>> +++ b/bsps/powerpc/shared/irq/i8259.c
>> @@ -12,6 +12,19 @@
>>  #include <bsp.h>
>>  #include <bsp/irq.h>
>>
>> +#define PIC_EOSI        0x60    ///< End of Specific Interrupt (EOSI)
>> +#define PIC_EOI         0x20    ///< Generic End of Interrupt (EOI)
> Is there a reason these two have a different comment format as the following?

The fragment is taken as is from the PC and I do not want to make changes so it
is easier to see what has come across. I have no idea why someone used that
style of comment but it has been in RTEMS like this for years.

It would be nicer to have this PIC code placed in the BSP shared dev location
and made common but that is something I cannot get to in this work.

>> +
>> +/* Operation control word type 3.  Bit 3 (0x08) must be set. Even address. */
>> +#define PIC_OCW3_RIS        0x01            /* 1 = read IS, 0 = read IR */
>> +#define PIC_OCW3_RR         0x02            /* register read */
>> +#define PIC_OCW3_P          0x04            /* poll mode command */
>> +/* 0x08 must be 1 to select OCW3 vs OCW2 */
>> +#define PIC_OCW3_SEL        0x08            /* must be 1 */
>> +/* 0x10 must be 0 to select OCW3 vs ICW1 */
>> +#define PIC_OCW3_SMM        0x20            /* special mode mask */
>> +#define PIC_OCW3_ESMM       0x40            /* enable SMM */
>> +
>>  /*-------------------------------------------------------------------------+
>>  | Cache for 1st and 2nd PIC IRQ line's status (enabled or disabled) register.
>>  +--------------------------------------------------------------------------*/
>> @@ -19,91 +32,140 @@
>>   * lower byte is interrupt mask on the master PIC.
>>   * while upper bits are interrupt on the slave PIC.
>>   */
>> -volatile rtems_i8259_masks i8259s_cache = 0xfffb;
>> +static rtems_i8259_masks i8259s_imr_cache = 0xFFFB;
>> +static rtems_i8259_masks i8259s_in_progress = 0;
>> +
>> +static inline
>> +void BSP_i8259s_irq_update_master_imr( void )
>> +{
>> +  rtems_i8259_masks mask = i8259s_in_progress | i8259s_imr_cache;
>> +  outport_byte( PIC_MASTER_IMR_IO_PORT, mask & 0xff );
>> +}
>> +
>> +static inline
>> +void BSP_i8259s_irq_update_slave_imr( void )
>> +{
>> +  rtems_i8259_masks mask = i8259s_in_progress | i8259s_imr_cache;
>> +  outport_byte( PIC_SLAVE_IMR_IO_PORT, ( mask >> 8 ) & 0xff );
>> +}
>> +
>> +/*
>> + * Is the IRQ valid?
>> + */
>> +static inline bool BSP_i8259s_irq_valid(const rtems_irq_number irqLine)
>> +{
>> +  return ((int)irqLine >= BSP_ISA_IRQ_LOWEST_OFFSET) &&
>> +    ((int)irqLine <= BSP_ISA_IRQ_MAX_OFFSET);
> nit: I think these casts are superfluous.

Again existing code and I want to keep it as close as possible to ease the task
of bringing the separate instance of the code together. Making changes would
only make that task harder.

>> +}
>> +
>> +/*
>> + * Read the IRR register. The default.
>> + */
>> +static inline uint8_t BSP_i8259s_irq_int_request_reg(uint32_t ioport)
>> +{
>> +  uint8_t isr;
>> +  inport_byte(ioport, isr);
>> +  return isr;
>> +}
>> +
>> +/*
>> + * Read the ISR register. Keep the default of the IRR.
>> + */
>> +static inline uint8_t BSP_i8259s_irq_in_service_reg(uint32_t ioport)
>> +{
>> +  uint8_t isr;
>> +  outport_byte(ioport, PIC_OCW3_SEL | PIC_OCW3_RR | PIC_OCW3_RIS);
>> +  inport_byte(ioport, isr);
>> +  outport_byte(ioport, PIC_OCW3_SEL | PIC_OCW3_RR);
>> +  return isr;
>> +}
>>
>>  /*-------------------------------------------------------------------------+
>>  |         Function:  BSP_irq_disable_at_i8259s
>>  |      Description: Mask IRQ line in appropriate PIC chip.
>> -| Global Variables: i8259s_cache
>> +| Global Variables: i8259s_imr_cache, i8259s_in_progress
>>  |        Arguments: vector_offset - number of IRQ line to mask.
>> -|          Returns: original state or -1 on error.
>> +|          Returns: 0 is OK.
>>  +--------------------------------------------------------------------------*/
>> -int BSP_irq_disable_at_i8259s    (const rtems_irq_number irqLine)
>> +int BSP_irq_disable_at_i8259s(const rtems_irq_number irqLine)
>>  {
>>    unsigned short        mask;
>>    rtems_interrupt_level level;
>> -  int                   rval;
>>
>> -  if ( ((int)irqLine < BSP_ISA_IRQ_LOWEST_OFFSET) ||
>> -       ((int)irqLine > BSP_ISA_IRQ_MAX_OFFSET)
>> -       )
>> +  if (!BSP_i8259s_irq_valid(irqLine))
>>      return -1;
>>
>>    rtems_interrupt_disable(level);
>>
>>    mask = 1 << irqLine;
>> -  rval = i8259s_cache & mask ? 0 : 1;
>> -  i8259s_cache |= mask;
>> +  i8259s_imr_cache |= mask;
>>
>>    if (irqLine < 8)
>>    {
>> -    outport_byte(PIC_MASTER_IMR_IO_PORT, i8259s_cache & 0xff);
>> +    BSP_i8259s_irq_update_master_imr();
>>    }
>>    else
>>    {
>> -    outport_byte(PIC_SLAVE_IMR_IO_PORT, ((i8259s_cache & 0xff00) >> 8));
>> +    BSP_i8259s_irq_update_slave_imr();
>>    }
>> +
>>    rtems_interrupt_enable(level);
>>
>> -  return rval;
>> +  return 0;
>>  }
>>
>>  /*-------------------------------------------------------------------------+
>>  |         Function:  BSP_irq_enable_at_i8259s
>>  |      Description: Unmask IRQ line in appropriate PIC chip.
>> -| Global Variables: i8259s_cache
>> +| Global Variables: i8259s_imr_cache, i8259s_in_progress
>>  |        Arguments: irqLine - number of IRQ line to mask.
>>  |          Returns: Nothing.
>>  +--------------------------------------------------------------------------*/
>> -int BSP_irq_enable_at_i8259s    (const rtems_irq_number irqLine)
>> +int BSP_irq_enable_at_i8259s(const rtems_irq_number irqLine)
>>  {
>> -  unsigned short        mask;
>>    rtems_interrupt_level level;
>> +  unsigned short        mask;
>> +  uint8_t               isr;
>> +  uint8_t               irr;
>>
>> -  if ( ((int)irqLine < BSP_ISA_IRQ_LOWEST_OFFSET) ||
>> -       ((int)irqLine > BSP_ISA_IRQ_MAX_OFFSET )
>> -       )
>> +  if (!BSP_i8259s_irq_valid(irqLine))
>>      return 1;
>>
>>    rtems_interrupt_disable(level);
>>
>> -  mask = ~(1 << irqLine);
>> -  i8259s_cache &= mask;
>> +  mask = 1 << irqLine;
>> +  i8259s_imr_cache &= ~mask;
>>
>>    if (irqLine < 8)
>>    {
>> -    outport_byte(PIC_MASTER_IMR_IO_PORT, i8259s_cache & 0xff);
>> +    isr = BSP_i8259s_irq_in_service_reg(PIC_MASTER_COMMAND_IO_PORT);
>> +    irr = BSP_i8259s_irq_int_request_reg(PIC_MASTER_COMMAND_IO_PORT);
>> +    BSP_i8259s_irq_update_master_imr();
>>    }
>>    else
>>    {
>> -    outport_byte(PIC_SLAVE_IMR_IO_PORT, ((i8259s_cache & 0xff00) >> 8));
>> +    isr = BSP_i8259s_irq_in_service_reg(PIC_SLAVE_COMMAND_IO_PORT);
>> +    irr = BSP_i8259s_irq_int_request_reg(PIC_SLAVE_COMMAND_IO_PORT);
>> +    BSP_i8259s_irq_update_slave_imr();
>>    }
>> +
>>    rtems_interrupt_enable(level);
>>
>> +  if (((isr ^ irr) & mask) != 0)
>> +    printk("powerpc: isr=%x irr=%x\n", isr, irr);
> DEBUG? if it's an error, should return non-zero?

Looks like left over debug code in the PC PIC code that has come across.
Superious interrupts around this controller are not easy.

>> +
>>    return 0;
>>  } /* mask_irq */
>>
>> -int BSP_irq_enabled_at_i8259s          (const rtems_irq_number irqLine)
>> +int BSP_irq_enabled_at_i8259s(const rtems_irq_number irqLine)
>>  {
>>    unsigned short mask;
>>
>> -  if ( ((int)irqLine < BSP_ISA_IRQ_LOWEST_OFFSET) ||
>> -       ((int)irqLine > BSP_ISA_IRQ_MAX_OFFSET)
>> -     )
>> +  if (!BSP_i8259s_irq_valid(irqLine))
>>      return 1;
>>
>>    mask = (1 << irqLine);
>> -  return  (~(i8259s_cache & mask));
>> +  return  (~(i8259s_imr_cache & mask));
>>  }
>>
>>  /*-------------------------------------------------------------------------+
>> @@ -113,24 +175,47 @@ int BSP_irq_enabled_at_i8259s             (const rtems_irq_number irqLine)
>>  |        Arguments: irqLine - number of IRQ line to acknowledge.
>>  |          Returns: Nothing.
>>  +--------------------------------------------------------------------------*/
>> -int BSP_irq_ack_at_i8259s      (const rtems_irq_number irqLine)
>> +int BSP_irq_ack_at_i8259s(const rtems_irq_number irqLine)
>>  {
>> +  uint8_t slave_isr = 0;
> where we have some flexibility, we should avoid using the master/slave
> dichotomy. I ignored it in some of the above API stuff, but for a
> local variable we don't need that. I understand this terminology is
> fully baked into the technical documentation, but we could consider
> using our own replacement (with appropriately documented mapping) such
> as primary/secondary. (This is a longer-term problem to resolve across
> the code base. But I have to start complaining somewhere I guess.)

I appreicate this and agree but again it is existing and making the change here
and not in the other location only makes for a new set of problems. An effort to
remove these terms is a good thing but I suggest it be a specific effort focused
on that task.

>>    if (irqLine >= 8) {
>> -    outport_byte(PIC_MASTER_COMMAND_IO_PORT, SLAVE_PIC_EOSI);
>> -    outport_byte(PIC_SLAVE_COMMAND_IO_PORT, (PIC_EOSI | (irqLine - 8)));
>> -  }
>> -  else {
>> -    outport_byte(PIC_MASTER_COMMAND_IO_PORT, (PIC_EOSI | irqLine));
>> +   outport_byte(PIC_SLAVE_COMMAND_IO_PORT, PIC_EOI);
>> +   slave_isr = BSP_i8259s_irq_in_service_reg(PIC_SLAVE_COMMAND_IO_PORT);
>>    }
>>
>> +  /*
>> +   * Only issue the EOI to the master if there are no more interrupts in
>> +   * service for the slave. i8259a data sheet page 18, The Special Fully Nested
>> +   * Mode, b.
>> +   */
>> +  if (slave_isr == 0)
> looks like the code base uses { } so prefer that for 1-liners also

ditto to being a copy

> 
>> +    outport_byte(PIC_MASTER_COMMAND_IO_PORT, PIC_EOI);
>> +
>>    return 0;
>>
>>  } /* ackIRQ */
>>
>> +unsigned short BSP_irq_suspend_i8259s(unsigned short mask)
>> +{
>> +  unsigned short in_progress_save = i8259s_in_progress;
>> +  i8259s_in_progress |= mask;
>> +  BSP_i8259s_irq_update_master_imr();
>> +  BSP_i8259s_irq_update_slave_imr();
>> +  return in_progress_save;
>> +}
>> +
>> +void BSP_irq_resume_i8259s(unsigned short in_progress_save)
>> +{
>> +  i8259s_in_progress = in_progress_save;
>> +  BSP_i8259s_irq_update_master_imr();
>> +  BSP_i8259s_irq_update_slave_imr();
>> +}
>> +
>>  void BSP_i8259s_init(void)
>>  {
>>    /*
>> -   * init master 8259 interrupt controller
>> +   * Always mask at least current interrupt to prevent re-entrance
>>     */
>>    outport_byte(PIC_MASTER_COMMAND_IO_PORT, 0x11); /* Start init sequence */
>>    outport_byte(PIC_MASTER_IMR_IO_PORT, 0x00);/* Vector base  = 0 */
>> diff --git a/bsps/powerpc/shared/irq/irq_init.c b/bsps/powerpc/shared/irq/irq_init.c
>> index 1042c9d1a8..233c659b85 100644
>> --- a/bsps/powerpc/shared/irq/irq_init.c
>> +++ b/bsps/powerpc/shared/irq/irq_init.c
>> @@ -280,6 +280,7 @@ void BSP_rtems_irq_mng_init(unsigned cpuId)
>>    int known_cpi_isa_bridge = 0;
>>  #endif
>>    int i;
>> +  int r;
>>
>>    /*
>>     * First initialize the Interrupt management hardware
>> @@ -351,7 +352,19 @@ void BSP_rtems_irq_mng_init(unsigned cpuId)
>>      initial_config.irqBase     = BSP_LOWEST_OFFSET;
>>      initial_config.irqPrioTbl  = irqPrioTable;
>>
>> -    if (!BSP_rtems_irq_mngt_set(&initial_config)) {
>> +#ifdef BSP_POWERPC_IRQ_GENERIC_SUPPORT
>> +#ifdef TRACE_IRQ_INIT
>> +    printk("RTEMS IRQ management: irq-generic\n");
>> +#endif
>> +    r = BSP_rtems_irq_generic_set(&initial_config);
>> +#else
>> +#ifdef TRACE_IRQ_INIT
>> +    printk("RTEMS IRQ management: legacy\n");
>> +#endif
>> +    r = BSP_rtems_irq_mngt_set(&initial_config);
>> +#endif
>> +
>> +    if (!r) {
>>        /*
>>         * put something here that will show the failure...
>>         */
>> diff --git a/bsps/powerpc/shared/irq/openpic_i8259_irq.c b/bsps/powerpc/shared/irq/openpic_i8259_irq.c
>> index 513b9ac3e0..2617555658 100644
>> --- a/bsps/powerpc/shared/irq/openpic_i8259_irq.c
>> +++ b/bsps/powerpc/shared/irq/openpic_i8259_irq.c
>> @@ -15,6 +15,7 @@
>>  #include <bsp.h>
>>  #include <bsp/irq.h>
>>  #include <bsp/irq_supp.h>
>> +#include <bsp/irq-generic.h>
>>  #ifndef BSP_HAS_NO_VME
>>  #include <bsp/VMEConfig.h>
>>  #endif
>> @@ -234,15 +235,15 @@ int C_dispatch_irq_handler (BSP_Exception_frame *frame, unsigned int excNum)
>>  #if BSP_ISA_IRQ_NUMBER > 0
>>    register unsigned isaIntr;                  /* boolean */
>>    register unsigned oldMask = 0;             /* old isa pic masks */
>> -  register unsigned newMask;                  /* new isa pic masks */
>>  #endif
>>
>>    if (excNum == ASM_DEC_VECTOR) {
>> -
>> -       bsp_irq_dispatch_list(rtems_hdl_tbl, BSP_DECREMENTER, default_rtems_entry.hdl);
>> -
>> +#ifdef BSP_POWERPC_IRQ_GENERIC_SUPPORT
>> +    bsp_interrupt_handler_dispatch(BSP_DECREMENTER);
>> +#else
>> +    bsp_irq_dispatch_list(rtems_hdl_tbl, BSP_DECREMENTER, default_rtems_entry.hdl);
>> +#endif
>>      return 0;
>> -
>>    }
>>
>>  #if BSP_PCI_IRQ_NUMBER > 0
>> @@ -274,7 +275,7 @@ int C_dispatch_irq_handler (BSP_Exception_frame *frame, unsigned int excNum)
>>
>>  #if BSP_ISA_IRQ_NUMBER > 0
>>  #ifdef BSP_PCI_ISA_BRIDGE_IRQ
>> -#if 0 == BSP_PCI_IRQ_NUMBER
>> +#if 0 == BSP_PCI_IRQ_NUMBER
>>  #error "Configuration Error -- BSP w/o PCI IRQs MUST NOT define BSP_PCI_ISA_BRIDGE_IRQ"
>>  #endif
>>    isaIntr = (irq == BSP_PCI_ISA_BRIDGE_IRQ);
>> @@ -289,11 +290,7 @@ int C_dispatch_irq_handler (BSP_Exception_frame *frame, unsigned int excNum)
>>      /*
>>       * store current PIC mask
>>       */
>> -    oldMask = i8259s_cache;
>> -    newMask = oldMask | irq_mask_or_tbl [irq];
>> -    i8259s_cache = newMask;
>> -    outport_byte(PIC_MASTER_IMR_IO_PORT, i8259s_cache & 0xff);
>> -    outport_byte(PIC_SLAVE_IMR_IO_PORT, ((i8259s_cache & 0xff00) >> 8));
>> +    oldMask = BSP_irq_suspend_i8259s(irq_mask_or_tbl [irq]);
>>      BSP_irq_ack_at_i8259s (irq);
>>  #if BSP_PCI_IRQ_NUMBER > 0
>>         if ( OpenPIC )
>> @@ -303,13 +300,15 @@ int C_dispatch_irq_handler (BSP_Exception_frame *frame, unsigned int excNum)
>>  #endif
>>
>>    /* dispatch handlers */
>> +#ifdef BSP_POWERPC_IRQ_GENERIC_SUPPORT
>> +  bsp_interrupt_handler_dispatch(irq);
>> +#else
>>    bsp_irq_dispatch_list(rtems_hdl_tbl, irq, default_rtems_entry.hdl);
>> +#endif
>>
>>  #if BSP_ISA_IRQ_NUMBER > 0
>>    if (isaIntr)  {
>> -    i8259s_cache = oldMask;
>> -    outport_byte(PIC_MASTER_IMR_IO_PORT, i8259s_cache & 0xff);
>> -    outport_byte(PIC_SLAVE_IMR_IO_PORT, ((i8259s_cache & 0xff00) >> 8));
>> +    BSP_irq_resume_i8259s(oldMask);
>>    }
>>    else
>>  #endif
>> diff --git a/bsps/powerpc/shared/irq/ppc-irq-generic.c b/bsps/powerpc/shared/irq/ppc-irq-generic.c
>> new file mode 100644
>> index 0000000000..fc364f06a8
>> --- /dev/null
>> +++ b/bsps/powerpc/shared/irq/ppc-irq-generic.c
>> @@ -0,0 +1,118 @@
>> +/* SPDX-License-Identifier: BSD-2-Clause */
>> +
>> +/**
>> + * @file
>> + *
>> + * @ingroup RTEMSBSPsPowerPC
>> + *
>> + * @brief Generic Interrupt suppoer
>> + */
>> +
>> +/*
>> + * Copyright (C) 2021 Chris Johns. All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer in the
>> + *    documentation and/or other materials provided with the distribution.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
>> + * POSSIBILITY OF SUCH DAMAGE.
>> + */
>> +
>> +#include <stdlib.h>
>> +
>> +#include <rtems.h>
>> +#include <stdlib.h>
>> +#include <rtems/bspIo.h> /* for printk */
>> +#include <libcpu/spr.h>
>> +#include <bsp/irq_supp.h>
>> +#include <bsp/irq-generic.h>
>> +#include <bsp/vectors.h>
>> +
>> +SPR_RW(BOOKE_TSR)
>> +SPR_RW(PPC405_TSR)
>> +
>> +/* legacy mode for bookE DEC exception;
>> + * to avoid the double layer of function calls
>> + * (dec_handler_bookE -> C_dispatch_irq_handler -> user handler)
>> + * it is preferrable for the user to hook the DEC
>> + * exception directly.
>> + * However, the legacy mode works with less modifications
>> + * of user code.
>> + */
>> +static int C_dispatch_dec_handler_bookE (BSP_Exception_frame *frame, unsigned int excNum)
>> +{
>> +  /* clear interrupt; we must do this
>> +   * before C_dispatch_irq_handler()
>> +   * re-enables MSR_EE.
>> +   * Note that PPC405 uses a different SPR# for TSR
>> +   */
>> +  if ( ppc_cpu_is_bookE()==PPC_BOOKE_405)
> nit: ws around == please

Copied in from legacy version. I will fix this.

The powerpc BSP would benefit from a code formatting tool. Attempting to follow
the style(s?) is like walking down a center line in the road after a night out.

> 
>> +    _write_PPC405_TSR( BOOKE_TSR_DIS );
>> +  else
>> +    _write_BOOKE_TSR( BOOKE_TSR_DIS );
>> +  return C_dispatch_irq_handler(frame, ASM_DEC_VECTOR);
>> +}
>> +
>> +/*
>> + * RTEMS Global Interrupt Handler Management Routines
>> + */
>> +int BSP_rtems_irq_generic_set(rtems_irq_global_settings* config)
>> +{
>> +  int r;
>> +
>> +  r = BSP_setup_the_pic(config);
>> +  if (!r)
>> +    return r;
>> +
>> +  ppc_exc_set_handler(ASM_EXT_VECTOR, C_dispatch_irq_handler);
>> +  if ( ppc_cpu_is_bookE() ) {
>> +    /* bookE decrementer interrupt needs to be cleared BEFORE
>> +     * dispatching the user ISR (because the user ISR is called
>> +     * with EE enabled)
>> +     * We do this so that existing DEC handlers can be used
>> +     * with minor modifications.
>> +     */
>> +    ppc_exc_set_handler(ASM_BOOKE_DEC_VECTOR, C_dispatch_dec_handler_bookE);
>> +  } else {
>> +    ppc_exc_set_handler(ASM_DEC_VECTOR, C_dispatch_irq_handler);
>> +  }
>> +
>> +  return 1;
>> +}
>> +
>> +
>> +void bsp_interrupt_vector_enable(rtems_vector_number vector)
>> +{
>> +  bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>> +  BSP_enable_irq_at_pic(vector);
>> +}
>> +
>> +void bsp_interrupt_vector_disable(rtems_vector_number vector)
>> +{
>> +  bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
>> +  BSP_disable_irq_at_pic(vector);
>> +}
>> +
>> +rtems_status_code bsp_interrupt_facility_initialize(void)
>> +{
>> +  /*
>> +   * Initalize RTEMS IRQ system
> same typo

OK

Chris

> 
>> +   */
>> +  BSP_rtems_irq_mng_init(0);
>> +  return RTEMS_SUCCESSFUL;
>> +}
>> diff --git a/spec/build/bsps/powerpc/motorola_powerpc/grp.yml b/spec/build/bsps/powerpc/motorola_powerpc/grp.yml
>> index f256fec10f..a3ad2b115d 100644
>> --- a/spec/build/bsps/powerpc/motorola_powerpc/grp.yml
>> +++ b/spec/build/bsps/powerpc/motorola_powerpc/grp.yml
>> @@ -10,7 +10,7 @@ links:
>>  - role: build-dependency
>>    uid: ../../obj
>>  - role: build-dependency
>> -  uid: ../../objirqdflt
>> +  uid: ../../objirq
>>  - role: build-dependency
>>    uid: ../crti
>>  - role: build-dependency
>> diff --git a/spec/build/bsps/powerpc/motorola_powerpc/obj.yml b/spec/build/bsps/powerpc/motorola_powerpc/obj.yml
>> index 07ee6fa721..ae7b9f96e2 100644
>> --- a/spec/build/bsps/powerpc/motorola_powerpc/obj.yml
>> +++ b/spec/build/bsps/powerpc/motorola_powerpc/obj.yml
>> @@ -39,7 +39,7 @@ source:
>>  - bsps/powerpc/shared/irq/irq_init.c
>>  - bsps/powerpc/shared/irq/openpic.c
>>  - bsps/powerpc/shared/irq/openpic_i8259_irq.c
>> -- bsps/powerpc/shared/irq/ppc-irq-legacy.c
>> +- bsps/powerpc/shared/irq/ppc-irq-generic.c
>>  - bsps/powerpc/shared/mmu/bat.c
>>  - bsps/powerpc/shared/mmu/mmuAsm.S
>>  - bsps/powerpc/shared/mmu/pte121.c
>> @@ -57,5 +57,6 @@ source:
>>  - bsps/powerpc/shared/start/zerobss.c
>>  - bsps/shared/dev/getentropy/getentropy-cpucounter.c
>>  - bsps/shared/dev/rtc/rtc-support.c
>> +- bsps/shared/irq/irq-default-handler.c
>>  - bsps/shared/start/bspfatal-default.c
>>  type: build
>> --
>> 2.24.1
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list