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

[Cédric Le Goater]

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 :

IRQ     HWIRQ NAME 
496: 000ffff0 opal-psi:fsp
497: 000ffff1 opal-psi:occ
498: 000ffff2 opal-psi:fsi
499: 000ffff3 opal-psi:lpchc
500: 000ffff4 opal-psi:local_err
501: 000ffff5 opal-psi:global_err
502: 000ffff6 opal-psi:external
503: 000ffff7 opal-psi:lpc_serirq_mux0
504: 000ffff8 opal-psi:lpc_serirq_mux1
505: 000ffff9 opal-psi:lpc_serirq_mux2
506: 000ffffa opal-psi:lpc_serirq_mux3
507: 000ffffb opal-psi:i2c
508: 000ffffc opal-psi:dio
509: 000ffffd opal-psi:psu
510: 010ffff0 opal-psi:fsp
511: 010ffff1 opal-psi:occ
512: 010ffff2 opal-psi:fsi
 55: 010ffff3 opal-psi:lpchc
 56: 010ffff4 opal-psi:local_err
 57: 010ffff5 opal-psi:global_err
 58: 010ffff6 opal-psi:external
 63: 010ffffb opal-psi:i2c
 64: 010ffffc opal-psi:dio
 65: 010ffffd opal-psi:psu

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

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

C.

> I suppose we could just generate them before registering the source.
> 
> Oliver
>