Hi Thomas,
Sorry for the delay.
No worries, Thanks for the effort.
Supporting this truly on x86 needs some more
thought and surgery, but for ARM it should not matter.
ok, as long as you are content not to support x86 atm.
I made a few
tweaks to your original code. See below.
I think maybe a few more tweaks, below. Apart from that, it looks to
work ok.
Thanks,
tglx
---
From: John Garry <john.garry@xxxxxxxxxx>
Subject: genirq/affinity: Add irq_update_affinity_desc()
Date: Wed, 28 Oct 2020 20:33:05 +0800
From: John Garry <john.garry@xxxxxxxxxx>
Add a function to allow the affinity of an interrupt be switched to
managed, such that interrupts allocated for platform devices may be
managed.
Suggested-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
---
include/linux/interrupt.h | 8 ++++++
kernel/irq/manage.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
...
+/**
+ * irq_update_affinity_desc - Update affinity management for an interrupt
+ * @irq: The interrupt number to update
+ * @affinity: Pointer to the affinity descriptor
+ *
+ * This interface can be used to configure the affinity management of
+ * interrupts which have been allocated already.
+ */
+int irq_update_affinity_desc(unsigned int irq,
+ struct irq_affinity_desc *affinity)
Just a note on the return value, in the only current callsite -
platform_get_irqs_affinity() - we don't check the return value and
propagate the error. This is because we don't want to fail the interrupt
init just because of problems updating the affinity mask. So I could
print a message to inform the user of error (at the callsite).
+{
+ struct irq_desc *desc;
+ unsigned long flags;
+ bool activated;
+
+ /*
+ * Supporting this with the reservation scheme used by x86 needs
+ * some more thought. Fail it for now.
+ */
+ if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
+ return -EOPNOTSUPP;
+
+ desc = irq_get_desc_buslock(irq, &flags, 0);
+ if (!desc)
+ return -EINVAL;
+
+ /* Requires the interrupt to be shut down */
+ if (irqd_is_started(&desc->irq_data))
We're missing the unlock here, right?
+ return -EBUSY;
+
+ /* Interrupts which are already managed cannot be modified */
+ if (irqd_is_managed(&desc->irq_data))
And here, and I figure that this should be irqd_affinity_is_managed()
+ return -EBUSY;
+
+ /*
+ * Deactivate the interrupt. That's required to undo
+ * anything an earlier activation has established.
+ */
+ activated = irqd_is_activated(&desc->irq_data);
+ if (activated)
+ irq_domain_deactivate_irq(&desc->irq_data);
+
+ if (affinity->is_managed) {
+ irqd_set(&desc->irq_data, IRQD_AFFINITY_MANAGED);
+ irqd_set(&desc->irq_data, IRQD_MANAGED_SHUTDOWN);
+ }
+
+ cpumask_copy(desc->irq_common_data.affinity, &affinity->mask);
+
+ /* Restore the activation state */
+ if (activated)
+ irq_domain_deactivate_irq(&desc->irq_data);
+ irq_put_desc_busunlock(desc, flags);
+ return 0;
+}
+
int __irq_set_affinity(unsigned int irq, const struct cpumask *mask, bool force)
{
struct irq_desc *desc = irq_to_desc(irq);
.
Thanks,
John
