[Skiboot] [PATCH v2 2/5] hw/psi-p9: Make interrupt name array global

Oliver O'Halloran oohall at gmail.com
Thu Sep 5 18:36:42 AEST 2019


On Thu, Sep 5, 2019 at 6:06 PM Cédric Le Goater <clg at kaod.org> wrote:
>
> On 05/09/2019 09:46, Oliver O'Halloran wrote:
> > On Thu, Sep 5, 2019 at 5:02 PM Cédric Le Goater <clg at kaod.org> wrote:
> >>
> >>> +static char *psi_p9_irq_name(struct irq_source *is, uint32_t isn)
> >>> +{
> >>> +     struct psi *psi = is->data;
> >>> +     uint32_t idx = isn - psi->interrupt;
> >>> +
> >>> +     if (idx >= ARRAY_SIZE(p9_psi_int_names))
> >>> +             return NULL;
> >>> +     return strdup(p9_psi_int_names[idx]);
> >>
> >> I haven't seen any of the interrupt sources allocating the name returned.
> >>
> >> I think we could remove the strdup from all name() handlers and the free()
> >> from add_opal_interrupts() one day.
> >
> > Yeah looks like they don't currently. I think the idea was to return a
> > malloc()ed string so the source could label the interrupts them based
> > on the source name. It's not needed for the PSI since there's only one
>
> You see both PSI on a boston :

What I meant was there's one PSI per chip. The fact we've got two sets
of interrupts isn't a problem because the HWIRQ allows you to
differentiate.

>*snip*
>
> > and you can use the HWIRQ number to work out what chip it's from,
>
> yes. This is true for any HWIRQ number. The XIVE block id is embedded in
> the IRQ number (see above) and it gives you some information on the chip id.

Yes, but it only tells you the chip ID. When there's multiple units on
a chip (PHBs, or NPUs on swift) you need to encode something into the
name.

> > but
> > for the PHBs it's good to be able to differentiate since otherwise you
> > have a sea of interrupts labelled "PHB Error". Granted we don't have
> > the PHB error interrupts wired up at the moment, but it's something
> > I'll get around to eventually.
>
> OK. I understand that the goal was to include the chip id in the literal
> name. It is a good idea to keep the strdup but we should add the required
> snprintf() then.

ok

Oliver


More information about the Skiboot mailing list