Hi Drew,
On 2020/2/11 21:37, Andrew Jones wrote:
Let's bail out of the wait loop if we see the expected state
to save over six seconds of run time. Make sure we wait a bit
before reading the registers and double check again after,
though, to somewhat mitigate the chance of seeing the expected
state by accident.
We also take this opportunity to push more IRQ state code to
the library.
Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
[...]
+
+enum gic_irq_state gic_irq_state(int irq)
This is a *generic* name while this function only deals with PPI.
Maybe we can use something like gic_ppi_state() instead? Or you
will have to take all interrupt types into account in a single
function, which is not a easy job I think.
+{
+ enum gic_irq_state state;
+ bool pending = false, active = false;
+ void *base;
+
+ assert(gic_common_ops);
+
+ switch (gic_version()) {
+ case 2:
+ base = gicv2_dist_base();
+ pending = readl(base + GICD_ISPENDR) & (1 << PPI(irq));
+ active = readl(base + GICD_ISACTIVER) & (1 << PPI(irq));
+ break;
+ case 3:
+ base = gicv3_sgi_base();
+ pending = readl(base + GICR_ISPENDR0) & (1 << PPI(irq));
+ active = readl(base + GICR_ISACTIVER0) & (1 << PPI(irq));
And you may also want to ensure that the 'irq' is valid for PPI().
Or personally, I'd just use a real PPI number (PPI(info->irq)) as
the input parameter of this function.
+ break;
+ }
+
+ if (!active && !pending)
+ state = GIC_IRQ_STATE_INACTIVE;
+ if (pending)
+ state = GIC_IRQ_STATE_PENDING;
+ if (active)
+ state = GIC_IRQ_STATE_ACTIVE;
+ if (active && pending)
+ state = GIC_IRQ_STATE_ACTIVE_PENDING;
+
+ return state;
+}
Otherwise,
Reviewed-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>
Tested-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>
Thanks
