[PATCH 10/17] irqchip/irq-mvebu-sei: add new driver for Marvell SEI

Thomas Petazzoni
2018-05-02T15:56:07Z

Hello,

On Wed, 2 May 2018 11:17:44 +0200, Thomas Petazzoni wrote:

> > +static const struct irq_domain_ops mvebu_sei_ap_domain_ops = {
> > + .xlate = irq_domain_xlate_onecell,
> > + .alloc = mvebu_sei_irq_domain_alloc,
> > + .free = mvebu_sei_irq_domain_free,
> > +};
> > +
> > +static const struct irq_domain_ops mvebu_sei_cp_domain_ops = {
> > + .xlate = irq_domain_xlate_twocell,
> > + .alloc = mvebu_sei_irq_domain_alloc,
> > + .free = mvebu_sei_irq_domain_free,
> > +};
>
> Why do you need two cells for the interrupts coming from the CP and
> only one cell for the interrupts coming from the AP ?
>
> For thermal in the AP, you do:
>
> + interrupt-parent = <&sei_wired_controller>;
> + interrupts = <18>;
>
> i.e, you don't specify an interrupt type. For thermal in the CP, you do:
>
> + interrupts-extended =
> + <&CP110_LABEL(icu_sei) 116 IRQ_TYPE_LEVEL_HIGH>;
>
> here you specify an interrupt type. I'm not sure why you have this
> difference. Even more so because I think a SEI level interrupt is not
> possible, since you only have a "SET" register and no "CLR" register.

OK, my comment is not very correct here, I'm comparing apple to
oranges. The former its an interrupt directly pointing to the GICP_SEI,
while the latter is an interrupt of the ICU, which itself will notify
the GICP_SEI through an MSI.

However, I'm still confused as to why you have .xlate =
irq_domain_xlate_twocell for the mvebu_sei_cp_domain_ops. I think there
is no need for ->xlate() call back here because it's going to be a MSI
domain.

Best regards,

Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering