Messages in this thread | | | Subject | Re: [PATCH v2 01/12] irqchip/gic: Rework gic_configure_irq to take the full ICFGR base | From | Marc Zyngier <> | Date | Mon, 19 Aug 2019 15:53:11 +0100 |
| |
On 19/08/2019 15:26, Zenghui Yu wrote: > Hi Marc, > > On 2019/8/6 18:01, Marc Zyngier wrote: >> gic_configure_irq is currently passed the (re)distributor address, >> to which it applies an a fixed offset to get to the configuration >> registers. This offset is constant across all GICs, or rather it was >> until to v3.1... >> >> An easy way out is for the individual drivers to pass the base >> address of the configuration register for the considered interrupt. >> At the same time, move part of the error handling back to the >> individual drivers, as things are about to change on that front. >> >> Signed-off-by: Marc Zyngier <maz@kernel.org> >> --- >> drivers/irqchip/irq-gic-common.c | 14 +++++--------- >> drivers/irqchip/irq-gic-v3.c | 11 ++++++++++- >> drivers/irqchip/irq-gic.c | 10 +++++++++- >> drivers/irqchip/irq-hip04.c | 7 ++++++- >> 4 files changed, 30 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c >> index b0a8215a13fc..6900b6f0921c 100644 >> --- a/drivers/irqchip/irq-gic-common.c >> +++ b/drivers/irqchip/irq-gic-common.c >> @@ -63,7 +63,7 @@ int gic_configure_irq(unsigned int irq, unsigned int type, >> * for "irq", depending on "type". >> */ >> raw_spin_lock_irqsave(&irq_controller_lock, flags); >> - val = oldval = readl_relaxed(base + GIC_DIST_CONFIG + confoff); >> + val = oldval = readl_relaxed(base + confoff); >> if (type & IRQ_TYPE_LEVEL_MASK) >> val &= ~confmask; >> else if (type & IRQ_TYPE_EDGE_BOTH) >> @@ -83,14 +83,10 @@ int gic_configure_irq(unsigned int irq, unsigned int type, >> * does not allow us to set the configuration or we are in a >> * non-secure mode, and hence it may not be catastrophic. >> */ >> - writel_relaxed(val, base + GIC_DIST_CONFIG + confoff); >> - if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val) { >> - if (WARN_ON(irq >= 32)) >> - ret = -EINVAL; > > Since this WARN_ON is dropped, the comment above should also be updated. > But what is the reason for deleting it? (It may give us some points > when we fail to set type for SPIs.)
The core code already warns in the case where irq_set_type() fails, and the duplication of warnings is pretty superfluous.
Thanks,
M. -- Jazz is not dead, it just smells funny...
| |