Re: [PATCH v2 3/6] lib: utils/irqchip/imsic: embed sbi_irqchip_device in imsic data

On Wed, Jul 8, 2026 at 9:30 PM Anup Patel <[email protected]> wrote:
>
> On Thu, Jun 18, 2026 at 12:16 AM Pawandeep Oza
> <[email protected]> wrote:
> >
> > Embed sbi_irqchip_device in struct imsic_data to enable instance-based
> > irqchip registration, replacing the previous global imsic_device
> > singleton. Include sbi_irqchip.h from imsic.h to make the embedded
> > struct visible to consumers.
> >
> > Signed-off-by: Oza Pawandeep <[email protected]>
> > ---
> >  include/sbi_utils/irqchip/imsic.h |  2 ++
> >  lib/utils/irqchip/imsic.c         | 25 ++++++++++++++++++-------
> >  2 files changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/sbi_utils/irqchip/imsic.h b/include/sbi_utils/irqchip/imsic.h
> > index 88dd3cfb..1910b62a 100644
> > --- a/include/sbi_utils/irqchip/imsic.h
> > +++ b/include/sbi_utils/irqchip/imsic.h
> > @@ -12,6 +12,7 @@
> >  #define __IRQCHIP_IMSIC_H__
> >
> >  #include <sbi/sbi_types.h>
> > +#include <sbi/sbi_irqchip.h>
> >
> >  #define IMSIC_MMIO_PAGE_SHIFT          12
> >  #define IMSIC_MMIO_PAGE_SZ             (1UL << IMSIC_MMIO_PAGE_SHIFT)
> > @@ -25,6 +26,7 @@ struct imsic_regs {
> >
> >  struct imsic_data {
> >         u32 unique_id;
> > +       struct sbi_irqchip_device irqchip;
>
> Minor nit: make this as first member of struct imsic_data
Oza: sure
>
> >         bool targets_mmode;
> >         u32 guest_index_bits;
> >         u32 hart_index_bits;
> > diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
> > index 0e7f3e6b..2abdc8ef 100644
> > --- a/lib/utils/irqchip/imsic.c
> > +++ b/lib/utils/irqchip/imsic.c
> > @@ -149,8 +149,13 @@ int imsic_get_target_file(u32 hartindex)
> >
> >  static int imsic_process_hwirqs(struct sbi_irqchip_device *chip)
> >  {
> > +       struct imsic_data *imsic;
> >         ulong mirq;
> >
> > +       imsic = container_of(chip, struct imsic_data, irqchip);
> > +       if (!imsic || !imsic->targets_mmode)
> > +               return SBI_EINVAL;
> > +
> >         while ((mirq = csr_swap(CSR_MTOPEI, 0))) {
> >                 mirq = (mirq >> IMSIC_TOPEI_ID_SHIFT);
> >
> > @@ -258,10 +263,15 @@ void imsic_local_irqchip_init(void)
> >
> >  static int imsic_warm_irqchip_init(struct sbi_irqchip_device *dev)
> >  {
> > -       struct imsic_data *imsic = imsic_get_data(current_hartindex());
> > +       struct imsic_data *imsic;
> > +       struct imsic_data *hart_imsic;
> > +
> > +       imsic = container_of(dev, struct imsic_data, irqchip);
> > +       hart_imsic = imsic_get_data(current_hartindex());
> >
> >         /* Sanity checks */
> > -       if (!imsic || !imsic->targets_mmode)
> > +       if (!hart_imsic || hart_imsic != imsic ||
> > +               !hart_imsic->targets_mmode)
> >                 return SBI_EINVAL;
> >
> >         /* Disable all interrupts */
> > @@ -399,16 +409,17 @@ int imsic_cold_irqchip_init(struct imsic_data *imsic)
> >         }
> >
> >         /* Register irqchip device */
> > -       imsic_device.id = imsic->unique_id;
> > +       imsic->irqchip = imsic_device;
> > +       imsic->irqchip.id = imsic->unique_id;
> >         imsic_device.caps = SBI_IRQCHIP_CAPS_MSI;
> > -       imsic_device.num_hwirq = imsic->num_ids + 1;
> > -       sbi_hartmask_set_all(&imsic_device.target_harts);
> > -       rc = sbi_irqchip_add_device(&imsic_device);
> > +       imsic->irqchip.num_hwirq = imsic->num_ids + 1;
> > +       sbi_hartmask_set_all(&imsic->irqchip.target_harts);
> > +       rc = sbi_irqchip_add_device(&imsic->irqchip);
> >         if (rc)
> >                 return rc;
> >
> >         /* Mark hwirq 0 and IPI hwirq as reserved */
> > -       rc = sbi_irqchip_register_reserved(&imsic_device, 0, IMSIC_IPI_ID + 1);
> > +       rc = sbi_irqchip_register_reserved(&imsic->irqchip, 0, IMSIC_IPI_ID + 1);
> >         if (rc)
> >                 return rc;
> >
> > --
> > 2.43.0
> >
> >
> > --
> > opensbi mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/opensbi
>
> Otherwise, this looks good to me.
>
> Reviewed-by: Anup Patel <[email protected]>
>
> Thanks,
> Anup

-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi