[PATCH 5/9] fsl-msi: Provide default retrigger callback
Marc Zyngier
maz at kernel.org
Wed Aug 26 12:37:30 EDT 2020
Hi Valentin,
On 2020-08-26 12:16, Valentin Schneider wrote:
> Hi Marc,
>
> Many thanks for picking this up!
> Below's the only comment I have, the rest LGTM.
>
> On 24/08/20 11:23, Marc Zyngier wrote:
>> Signed-off-by: Marc Zyngier <maz at kernel.org>
>> ---
>> drivers/bus/fsl-mc/fsl-mc-msi.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/bus/fsl-mc/fsl-mc-msi.c
>> b/drivers/bus/fsl-mc/fsl-mc-msi.c
>> index 8edadf05cbb7..5306ba7dea3e 100644
>> --- a/drivers/bus/fsl-mc/fsl-mc-msi.c
>> +++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
>> @@ -144,6 +144,8 @@ static void fsl_mc_msi_update_chip_ops(struct
>> msi_domain_info *info)
>> */
>> if (!chip->irq_write_msi_msg)
>> chip->irq_write_msi_msg = fsl_mc_msi_write_msg;
>> + if (!chip->irq_retrigger)
>> + chip->irq_retrigger = irq_chip_retrigger_hierarchy;
>
> AFAICT the closest generic hook we could use here is
>
> msi_create_irq_domain() -> msi_domain_update_chip_ops()
>
> which happens just below the fsl-specific ops update.
>
>
> However, placing a default .irq_retrigger callback in there would
> affect any
> and all MSI domain. IOW that would cover PCI and platform MSIs (covered
> by
> separate patches in this series), but also some x86 ("dmar" & "hpet")
> and
> TI thingies.
>
> I can't tell right now how bad of an idea it is, but I figured I'd
> throw
> this out there.
The problem with this approach is that it requires the resend path to be
cooperative and actually check for more than the top-level irq_data.
Otherwise you'd never actually trigger the HW resend if it is below
the top level.
But I like the idea though. Something like this should do the trick, and
is admittedly a bug fix:
diff --git a/kernel/irq/resend.c b/kernel/irq/resend.c
index c48ce19a257f..d11c729f9679 100644
--- a/kernel/irq/resend.c
+++ b/kernel/irq/resend.c
@@ -86,6 +86,18 @@ static int irq_sw_resend(struct irq_desc *desc)
}
#endif
+static int try_retrigger(struct irq_desc *desc)
+{
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+ return irq_chip_retrigger_hierarchy(&desc->irq_data);
+#else
+ if (desc->irq_data.chip->irq_retrigger)
+ return desc->irq_data.chip->irq_retrigger(&desc->irq_data);
+
+ return 0;
+#endif
+}
+
/*
* IRQ resend
*
@@ -113,8 +125,7 @@ int check_irq_resend(struct irq_desc *desc, bool
inject)
desc->istate &= ~IRQS_PENDING;
- if (!desc->irq_data.chip->irq_retrigger ||
- !desc->irq_data.chip->irq_retrigger(&desc->irq_data))
+ if (!try_retrigger(desc))
err = irq_sw_resend(desc);
/* If the retrigger was successfull, mark it with the REPLAY bit */
In general, introducing a irq_chip_retrigger_hierarchy() call
shouldn't be problematic as long as we don't overwrite an existing
callback.
I'll have a look at respining the series with that in mind.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
More information about the linux-arm-kernel
mailing list