Re: [PATCH v2 2/3] irqchip: irq-mips-gic: Provide function to map GIC user section
From: Qais Yousef
Date: Thu Oct 15 2015 - 05:37:34 EST
On 10/12/2015 11:16 AM, Thomas Gleixner wrote:
On Mon, 12 Oct 2015, Marc Zyngier wrote:
On 12/10/15 10:40, Markos Chandras wrote:
From: Alex Smith <alex.smith@xxxxxxxxxx>
The GIC provides a "user-mode visible" section containing a mirror of
the counter registers which can be mapped into user memory. This will
be used by the VDSO time function implementations, so provide a
function to map it in.
<SNIP>
This looks much better than the previous version (though I cannot find
the two other patches on LKML just yet).
Yes, it looks better. But I really have to ask the question why we are
trying to pack the world and somemore into an irq chip driver. We
already have the completely misplaced gic_read_count() there.
This code has a bad history. It was scattered all over the place in arch
code. Andrew Bresticker did a good job cleaning it up and moved it to
this irqchip driver.
https://lkml.org/lkml/2014/9/18/487
https://lkml.org/lkml/2014/10/20/481
While I understand that all of this is in the GIC block at least
according to the documentation, technically it's different hardware
blocks. And logically its different as well.
Yes but they're exposed through the same register interface.
So why not describe the various blocks (interrupt controller, timer,
shadow timer) as separate entities in the device tree and let each
subsystem look them up on their own. This cross subsystem hackery is
just horrible and does not buy anything except merge dependencies and
other avoidable hassle.
There's a mips-gic-timer driver in drivers/clocksource. But in device
tree it's a subnode of the irqchip driver.
http://lxr.free-electrons.com/source/drivers/clocksource/mips-gic-timer.c
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
Thoughts?
It could be refactored but the DT binding already specifies the GIC
timer as a subnode of GIC. Exposing this usermode register is the only
thing left in the register set that GIC driver wasn't dealing with.
Little gain in changing all of this now I think?
Thanks,
Qais
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/