Re: [PATCH 9/9] clocksource: dw_apb_timer: special variant for rockchip rk3188 timers
Ulrich, On Sat, 6 Jul 2013, Ulrich Prinz wrote: I got the message. With modifying the existing driver to support more function pointers in its system struct and assigning them at the beginning, and using them on runtime, these quirks are obsolete. Correct. Again, this is the first time I provide code to the kernel officially No problem. That's what code review is about. First post or not. and I learned from others that I should try it by modifying not too much code if not needed. Adding more function pointers to a system relevant structure, doubling the number of functions and such didn't look non-invasive to me. Well, it always depends. If there is a single place to deal with some oddball hardware, a flag is often the simplest way to go. If you have to add 10 conditionals in several functions then in a first step converting the existing implementation into function pointer calls and then in the next step providing new implementations for your hardware is most of the time simpler and cleaner. But, I totally agree with your argumentation and I even wanted to do it in the way you explained in your replies. Just the courage was missing I guess :) Gut feelings are often a better guidance than our self-doubting intellect. :) But don't worry. I had to learn it the hard way as well and I still trip over from time to time. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/9] clocksource: dw_apb_timer: quirk for variants with 64bit counter
On Sat, 6 Jul 2013, Heiko Stübner wrote:
This adds a quirk for IP variants containing two load_count and value
registers that are used to provide 64bit accuracy on 32bit systems.
The added accuracy is currently not used, the driver is only adapted to
handle the different register layout and make it work on affected devices.
Signed-off-by: Heiko Stuebner he...@sntech.de
---
drivers/clocksource/dw_apb_timer.c | 27 +++
include/linux/dw_apb_timer.h |6 ++
2 files changed, 33 insertions(+)
diff --git a/drivers/clocksource/dw_apb_timer.c
b/drivers/clocksource/dw_apb_timer.c
index f5e7be8..bd45351 100644
--- a/drivers/clocksource/dw_apb_timer.c
+++ b/drivers/clocksource/dw_apb_timer.c
@@ -56,6 +56,17 @@ static void apbt_init_regs(struct dw_apb_timer *timer, int
quirks)
timer-reg_control = APBTMR_N_CONTROL;
timer-reg_eoi = APBTMR_N_EOI;
timer-reg_int_status = APBTMR_N_INT_STATUS;
+
+ /*
+ * On variants with 64bit counters some registers are
+ * moved further down.
+ */
+ if (quirks APBTMR_QUIRK_64BIT_COUNTER) {
+ timer-reg_current_value += 0x4;
+ timer-reg_control += 0x8;
+ timer-reg_eoi += 0x8;
+ timer-reg_int_status += 0x8;
+ }
}
Oh, no. this is not how we handle these things.
1) We want proper constants for this 64bit IP block
2) This is not a quirk, it's a property of that particular IP block
You already made the register offsets a part of the timer structure,
so why don't you supply a proper structure filled with that values to
the init function?
That's what we do all over the place. Either we instantiate those
structs at compile time or runtime fed by device tree or any other
configuration mechanism.
static unsigned long apbt_readl(struct dw_apb_timer *timer, unsigned long
offs)
@@ -145,6 +156,10 @@ static void apbt_set_mode(enum clock_event_mode mode,
udelay(1);
pr_debug(Setting clock period %lu for HZ %d\n, period, HZ);
apbt_writel(timer, period, timer-reg_load_count);
+
+ if (timer-quirks APBTMR_QUIRK_64BIT_COUNTER)
+ apbt_writel(timer, 0, timer-reg_load_count + 0x4);
+
No. We are not adding such conditional constructs when we can deal
with them just by providing a proper set of function pointers. And
definitely not with hardcoded magic 0x4 constants involved.
timer-load_count(timer, value);
Provide a 32 bit and a 64 bit version of that function and be done
with it.
ctrl |= APBTMR_CONTROL_ENABLE;
apbt_writel(timer, ctrl, timer-reg_control);
break;
@@ -168,6 +183,10 @@ static void apbt_set_mode(enum clock_event_mode mode,
* running mode.
*/
apbt_writel(timer, ~0, timer-reg_load_count);
+
+ if (timer-quirks APBTMR_QUIRK_64BIT_COUNTER)
+ apbt_writel(timer, 0, timer-reg_load_count + 0x4);
+
Makes this copy go away.
ctrl = ~APBTMR_CONTROL_INT;
ctrl |= APBTMR_CONTROL_ENABLE;
apbt_writel(timer, ctrl, timer-reg_control);
@@ -199,6 +218,10 @@ static int apbt_next_event(unsigned long delta,
apbt_writel(timer, ctrl, timer-reg_control);
/* write new count */
apbt_writel(timer, delta, timer-reg_load_count);
+
+ if (timer-quirks APBTMR_QUIRK_64BIT_COUNTER)
+ apbt_writel(timer, 0, timer-reg_load_count + 0x4);
+
And this one as well.
ctrl |= APBTMR_CONTROL_ENABLE;
apbt_writel(timer, ctrl, timer-reg_control);
@@ -325,6 +348,10 @@ void dw_apb_clocksource_start(struct dw_apb_clocksource
*dw_cs)
ctrl = ~APBTMR_CONTROL_ENABLE;
apbt_writel(timer, ctrl, timer-reg_control);
apbt_writel(timer, ~0, timer-reg_load_count);
+
+ if (timer-quirks APBTMR_QUIRK_64BIT_COUNTER)
+ apbt_writel(timer, 0, timer-reg_load_count + 0x4);
+
Copy and paste is a conveniant thing, right? It just should have a pop
up window assigned which asks at the second instance of copying the
same thing whether you really thought about it.
Thanks,
tglx___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/9] clocksource: dw_apb_timer: quirk for variants without EOI register
On Sat, 6 Jul 2013, Heiko Stübner wrote: - dw_ced-eoi = apbt_eoi; + if (quirks APBTMR_QUIRK_NO_EOI) + dw_ced-eoi = apbt_eoi_int_status; + else + dw_ced-eoi = apbt_eoi; No again. This has nothing to do with quirks. We use quirks for workarounds and not for refactoring of code. Thanks, tglx___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 6/9] clocksource: dw_apb_timer: quirk for inverted int mask
On Sat, 6 Jul 2013, Heiko Stübner wrote: Some timer variants use an inverted setting to mask the timer interrupt. Therefore add a quirk to handle these variants. And by that add even more pointless conditionals into critical code pathes. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] clocksource:arm_global_timer: Add ARM global timer support.
On Tue, 25 Jun 2013, Srinivas KANDAGATLA wrote:
If its not too late can this patch be considered for 3.11 via clocksource
tree?
Sure. No worry, though I noticed a little detail when reading through it
again. See below.
+/**
+ * To ensure that updates to comparator value register do not set the
+ * Interrupt Status Register proceed as follows:
+ * 1. Clear the Comp Enable bit in the Timer Control Register.
+ * 2. Write the lower 32-bit Comparator Value Register.
+ * 3. Write the upper 32-bit Comparator Value Register.
+ * 4. Set the Comp Enable bit and, if necessary, the IRQ enable bit.
+ */
+static void gt_compare_set(unsigned long delta, int periodic)
+{
+ u64 counter = gt_counter_read();
+ unsigned long ctrl = readl(gt_base + GT_CONTROL);
Why are you reading the control register back over and over? All you
need to preserve is the GT_CONTROL_TIMER_ENABLE bit. So you can spare
that read out and just do
ctrl = GT_CONTROL_TIMER_ENABLE;
writel(ctrl, gt_base + GT_CONTROL);
+static irqreturn_t gt_clockevent_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = dev_id;
+
+ if (readl_relaxed(gt_base + GT_INT_STATUS)
+ GT_INT_STATUS_EVENT_FLAG) {
If you negate the check and return IRQ_NONE, you save one indent level
for the real code.
+ /**
+ * ERRATA 740657( Global Timer can send 2 interrupts for
+ * the same event in single-shot mode)
+ * Workaround:
+ * Either disable single-shot mode.
+ * Or
+ * Modify the Interrupt Handler to avoid the
+ * offending sequence. This is achieved by clearing
+ * the Global Timer flag _after_ having incremented
+ * the Comparator register value to a higher value.
+ */
+ if (!(readl_relaxed(gt_base + GT_CONTROL)
+ GT_CONTROL_AUTO_INC))
Again, no need to read from the hardware.
if (evt-mode == CLOCK_EVT_MODE_ONESHOT)
tells you what you need to know.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3] irqchip: Add TB10x interrupt controller driver
On Fri, 31 May 2013, Christian Ruppert wrote: The SOC interrupt controller driver for the Abilis Systems TB10x series of SOCs based on ARC700 CPUs. This patch depends on commits eb76bdd407d8a90e59a06cb0158886df390e5d1c and 712bc93df9e7f14b8a163148d2aa7c778e151627 from branch irq/for-arm of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git. This should go below --- as it is not part of the changelog. Signed-off-by: Christian Ruppert christian.rupp...@abilis.com Signed-off-by: Pierrick Hascoet pierrick.hasc...@abilis.com Why is Pierrick in the SOB chain? --- .../interrupt-controller/abilis,tb10x-ictl.txt | 38 arch/arc/boot/dts/abilis_tb100.dtsi| 32 ++-- arch/arc/boot/dts/abilis_tb101.dtsi| 32 ++-- arch/arc/boot/dts/abilis_tb10x.dtsi| 32 ++-- arch/arc/plat-tb10x/Kconfig|1 + drivers/irqchip/Kconfig|5 + drivers/irqchip/Makefile |1 + drivers/irqchip/irq-tb10x.c| 195 I can pick it up, but I'm not so fond of carrying arch/arc patches which might conflict with other stuff in the arc tree. Veenet, if I should pick the whole thing up, I'd like to have an acked-by at least. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] clocksource:arm_global_timer: Add ARM global timer support.
On Tue, 25 Jun 2013, Srinivas KANDAGATLA wrote:
On 25/06/13 14:17, Thomas Gleixner wrote:
On Tue, 25 Jun 2013, Srinivas KANDAGATLA wrote:
+static void gt_compare_set(unsigned long delta, int periodic)
+{
+ u64 counter = gt_counter_read();
+ unsigned long ctrl = readl(gt_base + GT_CONTROL);
Why are you reading the control register back over and over? All you
need to preserve is the GT_CONTROL_TIMER_ENABLE bit. So you can spare
that read out and just do
ctrl = GT_CONTROL_TIMER_ENABLE;
writel(ctrl, gt_base + GT_CONTROL);
Logically you are right we could do as you suggested, However its not
implicit that which bits are going to be cleared. Its more to do with
readability of the code.
If you still insist I can change it.
I'm not insisting. I just pointed out, that you can save cycles in a
hotpath. :)
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3] irqchip: Add TB10x interrupt controller driver
On Tue, 25 Jun 2013, Christian Ruppert wrote: On Tue, Jun 25, 2013 at 03:58:43PM +0200, Thomas Gleixner wrote: On Fri, 31 May 2013, Christian Ruppert wrote: The SOC interrupt controller driver for the Abilis Systems TB10x series of SOCs based on ARC700 CPUs. This patch depends on commits eb76bdd407d8a90e59a06cb0158886df390e5d1c and 712bc93df9e7f14b8a163148d2aa7c778e151627 from branch irq/for-arm of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git. This should go below --- as it is not part of the changelog. OK, I will change that. Signed-off-by: Christian Ruppert christian.rupp...@abilis.com Signed-off-by: Pierrick Hascoet pierrick.hasc...@abilis.com Why is Pierrick in the SOB chain? We work on the platform port together and review and test each others' patches. It is planned submitting several of his patches as soon as basic platform is in. --- .../interrupt-controller/abilis,tb10x-ictl.txt | 38 arch/arc/boot/dts/abilis_tb100.dtsi| 32 ++-- arch/arc/boot/dts/abilis_tb101.dtsi| 32 ++-- arch/arc/boot/dts/abilis_tb10x.dtsi| 32 ++-- arch/arc/plat-tb10x/Kconfig|1 + drivers/irqchip/Kconfig|5 + drivers/irqchip/Makefile |1 + drivers/irqchip/irq-tb10x.c| 195 I can pick it up, but I'm not so fond of carrying arch/arc patches which might conflict with other stuff in the arc tree. Veenet, if I should pick the whole thing up, I'd like to have an acked-by at least. Would you prefer if we separated everything in arch/arc from this patch and push it through Vineet's channel? If it can be split w/o breaking the world, this would be the most elegant solution. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5] clocksource:arm_global_timer: Add ARM global timer support.
On Mon, 24 Jun 2013, Srinivas KANDAGATLA wrote: From: Stuart Menefy stuart.men...@st.com This is a simple driver for the global timer module found in the Cortex A9-MP cores from revision r1p0 onwards. This should be able to perform the functions of the system timer and the local timer in an SMP system. The global timer has the following features: The global timer is a 64-bit incrementing counter with an auto-incrementing feature. It continues incrementing after sending interrupts. The global timer is memory mapped in the private memory region. The global timer is accessible to all Cortex-A9 processors in the cluster. Each Cortex-A9 processor has a private 64-bit comparator that is used to assert a private interrupt when the global timer has reached the comparator value. All the Cortex-A9 processors in a design use the banked ID, ID27, for this interrupt. ID27 is sent to the Interrupt Controller as a Private Peripheral Interrupt. The global timer is clocked by PERIPHCLK. Signed-off-by: Stuart Menefy stuart.men...@st.com Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@st.com CC: Arnd Bergmann a...@arndb.de CC: Rob Herring robherri...@gmail.com CC: Linus Walleij linus.wall...@linaro.org CC: Will Deacon will.dea...@arm.com CC: Thomas Gleixner t...@linutronix.de Reviewed-by: Thomas Gleixner t...@linutronix.de ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4] clocksource:arm_global_timer: Add ARM global timer support.
On Fri, 21 Jun 2013, Srinivas KANDAGATLA wrote:
+static void gt_clockevent_set_mode(enum clock_event_mode mode,
+struct clock_event_device *clk)
+{
+ unsigned long ctrl;
+
+ ctrl = readl(gt_base + GT_CONTROL);
+ switch (mode) {
+ case CLOCK_EVT_MODE_PERIODIC:
+ gt_compare_set(DIV_ROUND_CLOSEST(gt_clk_rate, HZ), 1);
+ break;
+ case CLOCK_EVT_MODE_ONESHOT:
+ ctrl = ~(GT_CONTROL_AUTO_INC);
You should probably clear the irq enable bit as well. The core will
program the next event right away.
+static irqreturn_t gt_clockevent_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
What kind of construct is this?
You are using request_percpu_irq() and the device id is pointing to
per cpu memory. Why do you need this horrible pointer indirection?
Because a lot of other ARM code uses the same broken construct?
+static struct clock_event_device __percpu **gt_evt;
+static DEFINE_PER_CPU(struct clock_event_device, gt_clockevent);
So you have compile time allocated clock event device structures and
then you allocate a percpu pointer area which holds pointers to the
static area. Whatfor?
Why not doing the obvious?
static struct clock_event_device __percpu *gt_evt;
gt_evt = alloc_percpu(struct clock_event_device):
request_percpu_irq(.., gt_evt);
And in the interrupt handler
struct clock_event_device *evt = dev_id;
+static int __cpuinit gt_clockevents_init(struct clock_event_device *clk)
+{
+ struct clock_event_device **this_cpu_clk;
+ int cpu = smp_processor_id();
+
+ clk-name = ARM global timer clock event;
No spaces in the names please
+ clk-features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+ clk-set_mode = gt_clockevent_set_mode;
+ clk-set_next_event = gt_clockevent_set_next_event;
+ this_cpu_clk = __this_cpu_ptr(gt_evt);
+ *this_cpu_clk = clk;
+ clk-cpumask = cpumask_of(cpu);
+ clk-irq = gt_ppi;
+ clockevents_config_and_register(clk, gt_clk_rate,
+ 1, 0x);
+ per_cpu(percpu_init_called, cpu) = true;
+ enable_percpu_irq(clk-irq, IRQ_TYPE_NONE);
+ return 0;
+}
+
+static void gt_clockevents_stop(struct clock_event_device *clk)
+{
+ gt_clockevent_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+ disable_percpu_irq(clk-irq);
+}
+
+static int __cpuinit gt_clockevents_setup(struct clock_event_device *clk)
+{
+ /* Use existing clock_event for boot cpu */
That comment is telling me what?
+ if (per_cpu(percpu_init_called, smp_processor_id()))
+ return 0;
And why do you need another percpu variable, if you can deduce the
same information from clk as well.
return clk-name ? 0 : gt_clockevents_init(clk);
Hmm?
+ /* already enabled in gt_clocksource_init. */
Huch?
+ return gt_clockevents_init(clk);
+}
+static void __init global_timer_of_register(struct device_node *np)
+{
+ gt_clk = of_clk_get(np, 0);
+ if (!IS_ERR(gt_clk)) {
+ err = clk_prepare_enable(gt_clk);
If that fails, you happily proceed, right?
+ } else {
+ pr_warn(global-timer: clk not found\n);
+ err = -EINVAL;
+ goto out_unmap;
+ }
+
+ gt_evt = alloc_percpu(struct clock_event_device *);
+ if (!gt_evt) {
+ pr_warn(global-timer: can't allocate memory\n);
+ err = -ENOMEM;
+ goto out_unmap;
Leaves the clock enabled and prepared.
+ }
+
+ err = request_percpu_irq(gt_ppi, gt_clockevent_interrupt,
+ gt, gt_evt);
+ if (err) {
+ pr_warn(global-timer: can't register interrupt %d (%d)\n,
+ gt_ppi, err);
+ goto out_free;
Ditto
+ }
+
+ gt_clk_rate = clk_get_rate(gt_clk);
+ gt_clocksource_init();
+ gt_clockevents_init(evt);
+#ifdef CONFIG_LOCAL_TIMERS
+ err = local_timer_register(gt_lt_ops);
+ if (err) {
+ pr_warn(global-timer: unable to register local timer.\n);
+ free_percpu_irq(gt_ppi, gt_evt);
+ goto out_free;
So that frees your magic pointers, but you still have the clockevent
registered for the boot cpu. The interrupt handler of that device is
happily dereferencing the freed percpu memory.
How is that supposed to work?
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4] clocksource:arm_global_timer: Add ARM global timer support.
On Fri, 21 Jun 2013, Stephen Boyd wrote:
On 06/21/13 08:56, Thomas Gleixner wrote:
+static irqreturn_t gt_clockevent_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
What kind of construct is this?
You are using request_percpu_irq() and the device id is pointing to
per cpu memory. Why do you need this horrible pointer indirection?
Because a lot of other ARM code uses the same broken construct?
This is an artifact of the ARM local timer API. I have been trying for a
No, it's not an artifact. It's a copy and paste issue. Looking at
drivers/clocksource/arm_arch_timer.c
arch_timer_evt = alloc_percpu(struct clock_event_device);
...
err = request_percpu_irq(ppi, arch_timer_handler_virt,
arch_timer, arch_timer_evt);
This code is correct and it does not need any of the changes.
Doing it with the pointer madness is just wrong, nothing else. Even if
there is a historic reason why the pointer juggling was necessary at
some point, it's obviously not needed anymore.
And historic crap is no justification for brainlessly copied code.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: lockdep dump on devtree_lock (involving esdhc)
On Tue, 11 Jun 2013, Scott Wood wrote: I get the following lockdump output on p2020rdb using v3.10-rc5-43-g34376a5. While it's not particularly polite for the esdhc driver to be calling OF functions while holding another lock which can be acquired from interrupt context, why is devtree_lock usually acquired in an irqsafe manner but sometimes not? Both types of usage were added by the same commit: commit d6d3c4e656513dcea61ce900f0ecb9ca820ee7cd Author: Thomas Gleixner t...@linutronix.de Date: Wed Feb 6 15:30:56 2013 -0500 OF: convert devtree lock from rw_lock to raw spinlock Stephen, you asked about this here: http://lkml.indiana.edu/hypermail/linux/kernel/1302.1/01383.html Did you ever get an answer? https://patchwork.kernel.org/patch/2470731/ I'm also curious why devtree_lock was made raw to begin with... Iterating over a device tree doesn't seem like something you'd want to trust to be low-latency. The reason is that it's taken in low level cpu bringup code and I did not find a different solution. :( Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] of: Fix locking vs. interrupts
On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote: The OF code uses irqsafe locks everywhere except in a handful of functions for no obvious reasons. Since the conversion from the old rwlocks, this now triggers lockdep warnings when used at interrupt time. At least one driver (ibmvscsi) seems to be doing that from softirq context. This converts the few non-irqsafe locks into irqsafe ones, making them consistent with the rest of the code. Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org CC: sta...@vger.kernel.org [v3.9+] Acked-by: Thomas Gleixner t...@linutronix.de --- Note: It's silly to access the device-tree at interrupt time in most cases, and we should probably fix ibmvscsi, but for the time being, let's fix the Right. obvious bug. Thomas, this can probably still go into 3.10... If not, I've CCed stable. Should go through Grant I think. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/6] irqchip: add support for Marvell Orion SoCs
On Thu, 6 Jun 2013, Sebastian Hesselbarth wrote: This patch adds an irqchip driver for the main interrupt controller found on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation). Corresponding device tree documentation is also added. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Reviewed-by: Thomas Gleixner t...@linutronix.de ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/6] Marvell Orion SoC irqchip and clocksource
On Tue, 11 Jun 2013, Sebastian Hesselbarth wrote: On 06/11/13 14:35, Ezequiel Garcia wrote: With Thomas Gleixner's Review now only somebody has to take the irqchip patch then all three drivers are queued for next release. I can take it through tip if nobody else wants it :) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/6] irqchip: add support for Marvell Orion SoCs
On Tue, 11 Jun 2013, Thomas Gleixner wrote:
On Thu, 6 Jun 2013, Sebastian Hesselbarth wrote:
This patch adds an irqchip driver for the main interrupt controller found
on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
Corresponding device tree documentation is also added.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Reviewed-by: Thomas Gleixner t...@linutronix.de
Second thoughts:
+static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+ struct irq_domain *d = irq_get_handler_data(irq);
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, irq);
+ u32 stat = readl_relaxed(gc-reg_base + ORION_BRIDGE_IRQ_CAUSE)
+ gc-mask_cache;
In init you map the first irq of that chip and install the chain
handler for it. Now if that first irq fires, isn't that set in the
cause register as well? And what acks that first irq?
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/6] irqchip: add support for Marvell Orion SoCs
On Tue, 11 Jun 2013, Sebastian Hesselbarth wrote:
On 06/11/13 15:30, Thomas Gleixner wrote:
On Tue, 11 Jun 2013, Thomas Gleixner wrote:
On Thu, 6 Jun 2013, Sebastian Hesselbarth wrote:
This patch adds an irqchip driver for the main interrupt controller
found
on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
Corresponding device tree documentation is also added.
Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com
Reviewed-by: Thomas Gleixner t...@linutronix.de
Second thoughts:
+static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc
*desc)
+{
+ struct irq_domain *d = irq_get_handler_data(irq);
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, irq);
+ u32 stat = readl_relaxed(gc-reg_base + ORION_BRIDGE_IRQ_CAUSE)
+ gc-mask_cache;
In init you map the first irq of that chip and install the chain
handler for it. Now if that first irq fires, isn't that set in the
cause register as well? And what acks that first irq?
It is acked by acking all unmasked bridge irqs.
Ok. A comment would be nice.
But what about the bit in of that first irq in the cause register? If
it's set on entry you call generic_handle_irq() for that as well. So
if it's set you need to mask it in stat. If not, then it wants a
comment.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/6] irqchip: add support for Marvell Orion SoCs
On Tue, 11 Jun 2013, Sebastian Hesselbarth wrote: On 06/11/13 15:45, Thomas Gleixner wrote: But what about the bit in of that first irq in the cause register? If it's set on entry you call generic_handle_irq() for that as well. So if it's set you need to mask it in stat. If not, then it wants a comment. I am not sure I can follow. orion_bridge_irq_init() maps the first parent irq, i.e. hwirq 0 of orion_irq. The parent irq controller clears that irq cause when all corresponding chained irqs are cleared. The chained (bridge) irqs are cleared by orion_bridge_irq_handler above. That makes sense. I got confused by: irq = irq_of_parse_and_map(np, 0); but now I see that it's mapping irq 0 of the parent interrupt controller. I'll add a comment before merging it. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3] irqchip: Add TB10x interrupt controller driver
On Fri, 31 May 2013, Christian Ruppert wrote:
The SOC interrupt controller driver for the Abilis Systems TB10x series of
SOCs based on ARC700 CPUs.
This patch depends on commits eb76bdd407d8a90e59a06cb0158886df390e5d1c and
712bc93df9e7f14b8a163148d2aa7c778e151627 from branch irq/for-arm of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git.
That branch can be pulled into ARC as well. It only contains the
changes, which are necessary for the irq domain support of the generic
irq chip.
+static void tb10x_irq_cascade(unsigned int irq, struct irq_desc *desc)
+{
+ struct irq_domain *domain = irq_desc_get_handler_data(desc);
+
+ generic_handle_irq(irq_find_mapping(domain, irq));
+}
...
+ for (i = 0; i nrirqs; i++) {
+ unsigned int irq = irq_of_parse_and_map(ictl, i);
+
+ irq_set_handler_data(irq, domain);
+ irq_set_chained_handler(irq, tb10x_irq_cascade);
+ }
I might be completely confused, but this does not make any sense at
all.
You allocate a linear domain and then map the interrupts in the
domain. The mapping function retrieves the hardware interrupt number
and creates a virtual interrupt number, installs the chip and the
handler for the interrupt and finally returns the virtual interrupt
number.
Now you take that virtual interrupt number and install
tb10x_irq_cascade as the handler. irq_set_chained_handler() will
startup (unmask) the interrupt right away.
In the cascade handler you take the virtual interrupt number, which
you get as argument, and find the mapping, i.e. the matching VIRTUAL
interrupt number for the VIRTUAL interrupt number and then call the
handler.
How is this supposed to work?
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [patch 7/8] genirq: generic chip: Add linear irq domain support
On Wed, 29 May 2013, Grant Likely wrote:
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -678,6 +678,8 @@ struct irq_chip_type {
* @wake_active: Interrupt is marked as an wakeup from suspend source
* @num_ct:Number of available irq_chip_type instances
(usually 1)
* @private: Private data for non generic chip callbacks
+ * @installed: bitfield to denote installed interrupts
+ * @domain:irq domain pointer
* @list: List head for keeping track of instances
* @chip_types:Array of interrupt irq_chip_types
*
@@ -699,6 +701,8 @@ struct irq_chip_generic {
u32 wake_active;
unsigned intnum_ct;
void*private;
+ unsigned long installed;
This is probably something that the irqdomain should be keeping track of
internally, but that's an issue for a separate patch series.
Ok. I just need access to that information, so I can figure out if
it's the first irq of the chip which gets mapped. I need this for
initializing the mask cache.
[...]
+struct irq_domain_ops irq_generic_chip_ops = {
+ .map = irq_map_generic_chip,
+ .xlate = irq_domain_xlate_onecell,
As discussed on IRC, should use onetwocell here for greater
compatibility with existing bindings.
Changed that.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH REBASE] irqchip: Add TB10x interrupt controller driver
On Tue, 7 May 2013, Christian Ruppert wrote:
+#include linux/interrupt.h
+#include linux/irqdomain.h
+#include linux/of_irq.h
+#include linux/of_address.h
+#include linux/of_platform.h
+#include linux/io.h
+#include linux/slab.h
+#include ../../drivers/irqchip/irqchip.h
#include irqchip.h perhaps?
+#define AB_IRQCTL_INT_ENABLE (0x00)
What's the purpose of the parens? Decrease readability?
+static void tb10x_irq_unmask(struct irq_data *data)
+{
+ struct tb10x_ictl *ictl = irq_data_get_irq_chip_data(data);
+ unsigned int hwirq = irqd_to_hwirq(data);
+
+ ab_irqctl_writereg(ictl, AB_IRQCTL_INT_ENABLE,
+ ab_irqctl_readreg(ictl, AB_IRQCTL_INT_ENABLE) | (1 hwirq));
Another implementation of the generic interrupt chip functionality.
+static int tb10x_irq_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ irq_set_chip_data(irq, d-host_data);
+ irq_set_chip_and_handler(irq, irq_tb10x_chip, handle_level_irq);
+
+ return 0;
+}
+
+static int tb10x_irq_xlate(struct irq_domain *d, struct device_node *node,
+ const u32 *intspec, unsigned int intsize,
+ unsigned long *out_hwirq, unsigned int *out_type)
+{
+ static const unsigned int tb10x_xlate_types[] = {
+ IRQ_TYPE_EDGE_RISING,
+ IRQ_TYPE_LEVEL_LOW,
+ IRQ_TYPE_LEVEL_HIGH,
+ IRQ_TYPE_EDGE_FALLING,
Why do you need to introduce another format for specifying the
interrupt type instead of using the IRQ_TYPE values and
irq_domain_xlate_twocell()?
+static void tb10x_irq_cascade(unsigned int irq, struct irq_desc *desc)
+{
+ struct tb10x_ictl *ictl = irq_desc_get_handler_data(desc);
+
+ generic_handle_irq(irq_find_mapping(ictl-domain, irq));
+}
+
+static int __init of_tb10x_init_irq(struct device_node *ictl,
+ struct device_node *parent)
+{
+ int i;
+ int ret;
+ int nrirqs = of_irq_count(ictl);
One line for all those ints please
+ ic-domain = irq_domain_add_tree(ictl, irq_tb10x_domain_ops, ic);
Why do you want a tree, if you require that the interrupts are mapped
1:1 to the hardware interrupt numbers? This doesn't make any sense at
all and makes the above cascade handler a pointless waste of cpu cycles.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC patch 7/8] fixup 1/2: genirq: generic chip: Add linear irq domain support
On Mon, 6 May 2013, Sebastian Hesselbarth wrote: irq_domain_chip_generic is allocating and indexing irq_chip_generic itself. However, the size of irq_chip_generic varies with number of irq_chip_types. This fixup moves irq_chip_generic helt by irq_domain_chip_generic to array of ptr and fixes the pointer arith used by irq_alloc_domain_generic_chip. Thanks. I folded it in. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC patch 7/8] fixup 2/2: genirq: generic chip: Add linear irq domain support
On Mon, 6 May 2013, Sebastian Hesselbarth wrote: mask_cache pointer also needs to be initialized for domain generic chips. It's not only the mask cache pointer. We also need to initialize the mask cache itself. I solved it by spitting out that code from irq_setup_generic_chip(). Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[patch 1/8] genirq: generic chip: Remove the local cur_regs() function
From: Gerlando Falauto gerlando.fala...@keymile.com
Since we already have an irq_data_get_chip_type() function which returns
a pointer to irq_chip_type, use that instead of cur_regs().
Signed-off-by: Gerlando Falauto gerlando.fala...@keymile.com
Cc: Lennert Buytenhek ker...@wantstofly.org
Cc: Simon Guinot si...@sequanux.org
Cc: Joey Oravec jora...@drewtech.com
Cc: Ben Dooks ben-li...@fluff.org
Cc: Nicolas Pitre n...@fluxnic.net
Cc: Jason Cooper ja...@lakedaemon.net
Cc: Andrew Lunn and...@lunn.ch
Cc: Holger Brunck holger.bru...@keymile.com
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
kernel/irq/generic-chip.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
Index: tip/kernel/irq/generic-chip.c
===
--- tip.orig/kernel/irq/generic-chip.c
+++ tip/kernel/irq/generic-chip.c
@@ -16,11 +16,6 @@
static LIST_HEAD(gc_list);
static DEFINE_RAW_SPINLOCK(gc_lock);
-static inline struct irq_chip_regs *cur_regs(struct irq_data *d)
-{
- return container_of(d-chip, struct irq_chip_type, chip)-regs;
-}
-
/**
* irq_gc_noop - NOOP function
* @d: irq_data
@@ -39,10 +34,11 @@ void irq_gc_noop(struct irq_data *d)
void irq_gc_mask_disable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-disable);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.disable);
gc-mask_cache = ~mask;
irq_gc_unlock(gc);
}
@@ -57,11 +53,12 @@ void irq_gc_mask_disable_reg(struct irq_
void irq_gc_mask_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
gc-mask_cache |= mask;
- irq_reg_writel(gc-mask_cache, gc-reg_base + cur_regs(d)-mask);
+ irq_reg_writel(gc-mask_cache, gc-reg_base + ct-regs.mask);
irq_gc_unlock(gc);
}
@@ -75,11 +72,12 @@ void irq_gc_mask_set_bit(struct irq_data
void irq_gc_mask_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
gc-mask_cache = ~mask;
- irq_reg_writel(gc-mask_cache, gc-reg_base + cur_regs(d)-mask);
+ irq_reg_writel(gc-mask_cache, gc-reg_base + ct-regs.mask);
irq_gc_unlock(gc);
}
@@ -93,10 +91,11 @@ void irq_gc_mask_clr_bit(struct irq_data
void irq_gc_unmask_enable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-enable);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.enable);
gc-mask_cache |= mask;
irq_gc_unlock(gc);
}
@@ -108,10 +107,11 @@ void irq_gc_unmask_enable_reg(struct irq
void irq_gc_ack_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
irq_gc_unlock(gc);
}
@@ -122,10 +122,11 @@ void irq_gc_ack_set_bit(struct irq_data
void irq_gc_ack_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = ~(1 (d-irq - gc-irq_base));
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
irq_gc_unlock(gc);
}
@@ -136,11 +137,12 @@ void irq_gc_ack_clr_bit(struct irq_data
void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-mask);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.mask);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
irq_gc_unlock(gc);
}
@@ -151,10 +153,11 @@ void irq_gc_mask_disable_reg_and_ack(str
void irq_gc_eoi(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct
[patch 2/8] genirq: generic chip: Add support for per chip type mask cache
From: Gerlando Falauto gerlando.fala...@keymile.com
Today the same interrupt mask cache (stored within struct irq_chip_generic)
is shared between all the irq_chip_type instances. As there are instances
where each irq_chip_type uses a distinct mask register (as it is the case
for Orion SoCs), sharing a single mask cache may be incorrect.
So add a distinct pointer for each irq_chip_type, which for now
points to the original mask register within irq_chip_generic.
So no functional changes here.
[ tglx: Minor cosmetic tweaks ]
Reported-by: Joey Oravec jora...@drewtech.com
Signed-off-by: Simon Guinot sgui...@lacie.com
Signed-off-by: Holger Brunck holger.bru...@keymile.com
Signed-off-by: Gerlando Falauto gerlando.fala...@keymile.com
Cc: Lennert Buytenhek ker...@wantstofly.org
Cc: Simon Guinot si...@sequanux.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: Nicolas Pitre n...@fluxnic.net
Cc: Jason Cooper ja...@lakedaemon.net
Cc: Andrew Lunn and...@lunn.ch
Cc: Holger Brunck holger.bru...@keymile.com
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h |6 +-
kernel/irq/generic-chip.c | 16 ++--
2 files changed, 15 insertions(+), 7 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -644,6 +644,8 @@ struct irq_chip_regs {
* @regs: Register offsets for this chip
* @handler: Flow handler associated with this chip
* @type: Chip can handle these flow types
+ * @mask_cache_priv: Cached mask register private to the chip type
+ * @mask_cache:Pointer to cached mask register
*
* A irq_generic_chip can have several instances of irq_chip_type when
* it requires different functions and register offsets for different
@@ -654,6 +656,8 @@ struct irq_chip_type {
struct irq_chip_regsregs;
irq_flow_handler_t handler;
u32 type;
+ u32 mask_cache_priv;
+ u32 *mask_cache;
};
/**
@@ -662,7 +666,7 @@ struct irq_chip_type {
* @reg_base: Register base address (virtual)
* @irq_base: Interrupt base nr for this chip
* @irq_cnt: Number of interrupts handled by this chip
- * @mask_cache:Cached mask register
+ * @mask_cache:Cached mask register shared between all chip
types
* @type_cache:Cached type register
* @polarity_cache:Cached polarity register
* @wake_enabled: Interrupt can wakeup from suspend
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -39,7 +39,7 @@ void irq_gc_mask_disable_reg(struct irq_
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.disable);
- gc-mask_cache = ~mask;
+ *ct-mask_cache = ~mask;
irq_gc_unlock(gc);
}
@@ -57,8 +57,8 @@ void irq_gc_mask_set_bit(struct irq_data
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- gc-mask_cache |= mask;
- irq_reg_writel(gc-mask_cache, gc-reg_base + ct-regs.mask);
+ *ct-mask_cache |= mask;
+ irq_reg_writel(*ct-mask_cache, gc-reg_base + ct-regs.mask);
irq_gc_unlock(gc);
}
@@ -76,8 +76,8 @@ void irq_gc_mask_clr_bit(struct irq_data
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- gc-mask_cache = ~mask;
- irq_reg_writel(gc-mask_cache, gc-reg_base + ct-regs.mask);
+ *ct-mask_cache = ~mask;
+ irq_reg_writel(*ct-mask_cache, gc-reg_base + ct-regs.mask);
irq_gc_unlock(gc);
}
@@ -96,7 +96,7 @@ void irq_gc_unmask_enable_reg(struct irq
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.enable);
- gc-mask_cache |= mask;
+ *ct-mask_cache |= mask;
irq_gc_unlock(gc);
}
@@ -250,6 +250,10 @@ void irq_setup_generic_chip(struct irq_c
if (flags IRQ_GC_INIT_MASK_CACHE)
gc-mask_cache = irq_reg_readl(gc-reg_base + ct-regs.mask);
+ /* Initialize mask cache pointer */
+ for (i = 0; i gc-num_ct; i++)
+ ct[i].mask_cache = gc-mask_cache;
+
for (i = gc-irq_base; msk; msk = 1, i++) {
if (!(msk 0x01))
continue;
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[patch 0/8] genirq: Support for irq domains in generic irq chip - V2
Changes vs. V1: - Fixed the generic chip pointer thinko (Sebastian Hesselbarth) - Proper support for mask cache - Read mask hardware only for the first map of an generic chip instance - sun4i prefix irq functions proper Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[patch 6/8] genirq: Split out code in generic chip
Preparatory patch for linear interrupt domains.
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
kernel/irq/generic-chip.c | 50 +++---
1 file changed, 34 insertions(+), 16 deletions(-)
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -186,6 +186,19 @@ int irq_gc_set_wake(struct irq_data *d,
return 0;
}
+static void
+irq_init_generic_chip(struct irq_chip_generic *gc, const char *name,
+ int num_ct, unsigned int irq_base,
+ void __iomem *reg_base, irq_flow_handler_t handler)
+{
+ raw_spin_lock_init(gc-lock);
+ gc-num_ct = num_ct;
+ gc-irq_base = irq_base;
+ gc-reg_base = reg_base;
+ gc-chip_types-chip.name = name;
+ gc-chip_types-handler = handler;
+}
+
/**
* irq_alloc_generic_chip - Allocate a generic chip and initialize it
* @name: Name of the irq chip
@@ -206,17 +219,31 @@ irq_alloc_generic_chip(const char *name,
gc = kzalloc(sz, GFP_KERNEL);
if (gc) {
- raw_spin_lock_init(gc-lock);
- gc-num_ct = num_ct;
- gc-irq_base = irq_base;
- gc-reg_base = reg_base;
- gc-chip_types-chip.name = name;
- gc-chip_types-handler = handler;
+ irq_init_generic_chip(gc, name, num_ct, irq_base, reg_base,
+ handler);
}
return gc;
}
EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
+static void
+irq_gc_init_mask_cache(struct irq_chip_generic *gc, enum irq_gc_flags flags)
+{
+ struct irq_chip_type *ct = gc-chip_types;
+ u32 *mskptr = gc-mask_cache, mskreg = ct-regs.mask;
+ int i;
+
+ for (i = 0; i gc-num_ct; i++) {
+ if (flags IRQ_GC_MASK_CACHE_PER_TYPE) {
+ mskptr = ct[i].mask_cache_priv;
+ mskreg = ct[i].regs.mask;
+ }
+ ct[i].mask_cache = mskptr;
+ if (flags IRQ_GC_INIT_MASK_CACHE)
+ *mskptr = irq_reg_readl(gc-reg_base + mskreg);
+ }
+}
+
/*
* Separate lockdep class for interrupt chip which can nest irq_desc
* lock.
@@ -242,21 +269,12 @@ void irq_setup_generic_chip(struct irq_c
struct irq_chip_type *ct = gc-chip_types;
struct irq_chip *chip = ct-chip;
unsigned int i;
- u32 *mskptr = gc-mask_cache, mskreg = ct-regs.mask;
raw_spin_lock(gc_lock);
list_add_tail(gc-list, gc_list);
raw_spin_unlock(gc_lock);
- for (i = 0; i gc-num_ct; i++) {
- if (flags IRQ_GC_MASK_CACHE_PER_TYPE) {
- mskptr = ct[i].mask_cache_priv;
- mskreg = ct[i].regs.mask;
- }
- ct[i].mask_cache = mskptr;
- if (flags IRQ_GC_INIT_MASK_CACHE)
- *mskptr = irq_reg_readl(gc-reg_base + mskreg);
- }
+ irq_gc_init_mask_cache(gc, flags);
for (i = gc-irq_base; msk; msk = 1, i++) {
if (!(msk 0x01))
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[patch 4/8] genirq: generic chip: Cache per irq bit mask
Cache the per irq bit mask instead of recalculating it over and over.
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h |4
kernel/irq/generic-chip.c | 23 ++-
2 files changed, 18 insertions(+), 9 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -119,6 +119,7 @@ struct irq_domain;
/**
* struct irq_data - per irq and irq chip data passed down to chip functions
+ * @mask: precomputed bitmask for accessing the chip registers
* @irq: interrupt number
* @hwirq: hardware interrupt number, local to the interrupt domain
* @node: node index useful for balancing
@@ -138,6 +139,7 @@ struct irq_domain;
* irq_data.
*/
struct irq_data {
+ u32 mask;
unsigned intirq;
unsigned long hwirq;
unsigned intnode;
@@ -705,11 +707,13 @@ struct irq_chip_generic {
* irq chips which need to call irq_set_wake() on
* the parent irq. Usually GPIO implementations
* @IRQ_GC_MASK_CACHE_PER_TYPE:Mask cache is chip type private
+ * @IRQ_GC_NO_MASK:Do not calculate irq_data-mask
*/
enum irq_gc_flags {
IRQ_GC_INIT_MASK_CACHE = 1 0,
IRQ_GC_INIT_NESTED_LOCK = 1 1,
IRQ_GC_MASK_CACHE_PER_TYPE = 1 2,
+ IRQ_GC_NO_MASK = 1 3,
};
/* Generic chip callback functions */
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -35,7 +35,7 @@ void irq_gc_mask_disable_reg(struct irq_
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.disable);
@@ -54,7 +54,7 @@ void irq_gc_mask_set_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
*ct-mask_cache |= mask;
@@ -73,7 +73,7 @@ void irq_gc_mask_clr_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
*ct-mask_cache = ~mask;
@@ -92,7 +92,7 @@ void irq_gc_unmask_enable_reg(struct irq
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.enable);
@@ -108,7 +108,7 @@ void irq_gc_ack_set_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
@@ -123,7 +123,7 @@ void irq_gc_ack_clr_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = ~(1 (d-irq - gc-irq_base));
+ u32 mask = ~d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
@@ -138,7 +138,7 @@ void irq_gc_mask_disable_reg_and_ack(str
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.mask);
@@ -154,7 +154,7 @@ void irq_gc_eoi(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.eoi);
@@ -172,7 +172,7 @@ void irq_gc_eoi(struct irq_data *d)
int irq_gc_set_wake(struct irq_data *d, unsigned int on)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
if (!(mask gc-wake_enabled))
return -EINVAL;
@@ -264,6 +264,11 @@ void irq_setup_generic_chip
[patch 5/8] genirq: Add a mask calculation function
Some chips have weird bit mask access patterns instead of the linear
you expect. Allow them to calculate the cached mask themself.
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h |3 +++
kernel/irq/generic-chip.c |8 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -296,6 +296,7 @@ static inline irq_hw_number_t irqd_to_hw
* @irq_suspend: function called from core code on suspend once per chip
* @irq_resume:function called from core code on resume once
per chip
* @irq_pm_shutdown: function called from core code on shutdown once per chip
+ * @irq_calc_mask: Optional function to set irq_data.mask for special cases
* @irq_print_chip:optional to print special chip info in show_interrupts
* @flags: chip specific flags
*/
@@ -327,6 +328,8 @@ struct irq_chip {
void(*irq_resume)(struct irq_data *data);
void(*irq_pm_shutdown)(struct irq_data *data);
+ void(*irq_calc_mask)(struct irq_data *data);
+
void(*irq_print_chip)(struct irq_data *data, struct
seq_file *p);
unsigned long flags;
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -240,6 +240,7 @@ void irq_setup_generic_chip(struct irq_c
unsigned int set)
{
struct irq_chip_type *ct = gc-chip_types;
+ struct irq_chip *chip = ct-chip;
unsigned int i;
u32 *mskptr = gc-mask_cache, mskreg = ct-regs.mask;
@@ -267,9 +268,12 @@ void irq_setup_generic_chip(struct irq_c
if (!(flags IRQ_GC_NO_MASK)) {
struct irq_data *d = irq_get_irq_data(i);
- d-mask = 1 (i - gc-irq_base);
+ if (chip-irq_calc_mask)
+ chip-irq_calc_mask(d);
+ else
+ d-mask = 1 (i - gc-irq_base);
}
- irq_set_chip_and_handler(i, ct-chip, ct-handler);
+ irq_set_chip_and_handler(i, chip, ct-handler);
irq_set_chip_data(i, gc);
irq_modify_status(i, clr, set);
}
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[patch 7/8] genirq: generic chip: Add linear irq domain support
Provide infrastructure for irq chip implementations which work on
linear irq domains.
- Interface to allocate multiple generic chips which are associated to
the irq domain.
- Interface to get the generic chip pointer for a particular hardware
interrupt in the domain.
- irq domain mapping function to install the chip for a particular
interrupt.
Note: This lacks a removal function for now, but this is a draft patch
the ARM folks to work on.
[ Sebastian Hesselbarth: Mask cache and pointer math fixups ]
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h | 30 +++
include/linux/irqdomain.h | 12 ++
kernel/irq/generic-chip.c | 187 --
kernel/irq/irqdomain.c|6 -
4 files changed, 223 insertions(+), 12 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -678,6 +678,8 @@ struct irq_chip_type {
* @wake_active: Interrupt is marked as an wakeup from suspend source
* @num_ct:Number of available irq_chip_type instances (usually 1)
* @private: Private data for non generic chip callbacks
+ * @installed: bitfield to denote installed interrupts
+ * @domain:irq domain pointer
* @list: List head for keeping track of instances
* @chip_types:Array of interrupt irq_chip_types
*
@@ -699,6 +701,8 @@ struct irq_chip_generic {
u32 wake_active;
unsigned intnum_ct;
void*private;
+ unsigned long installed;
+ struct irq_domain *domain;
struct list_headlist;
struct irq_chip_typechip_types[0];
};
@@ -719,6 +723,24 @@ enum irq_gc_flags {
IRQ_GC_NO_MASK = 1 3,
};
+/*
+ * struct irq_domain_chip_generic - Generic irq chip data structure for irq
domains
+ * @irqs_per_chip: Number of interrupts per chip
+ * @num_chips: Number of chips
+ * @irq_flags_to_set: IRQ* flags to set on irq setup
+ * @irq_flags_to_clear:IRQ* flags to clear on irq setup
+ * @gc_flags: Generic chip specific setup flags
+ * @gc:Array of pointers to generic interrupt chips
+ */
+struct irq_domain_chip_generic {
+ unsigned intirqs_per_chip;
+ unsigned intnum_chips;
+ unsigned intirq_flags_to_clear;
+ unsigned intirq_flags_to_set;
+ enum irq_gc_flags gc_flags;
+ struct irq_chip_generic *gc[0];
+};
+
/* Generic chip callback functions */
void irq_gc_noop(struct irq_data *d);
void irq_gc_mask_disable_reg(struct irq_data *d);
@@ -742,6 +764,14 @@ int irq_setup_alt_chip(struct irq_data *
void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
unsigned int clr, unsigned int set);
+struct irq_chip_generic *irq_get_domain_generic_chip(struct irq_domain *d,
unsigned int hw_irq);
+int irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip,
+ int num_ct, const char *name,
+ irq_flow_handler_t handler,
+ unsigned int clr, unsigned int set,
+ enum irq_gc_flags flags);
+
+
static inline struct irq_chip_type *irq_data_get_chip_type(struct irq_data *d)
{
return container_of(d-chip, struct irq_chip_type, chip);
Index: linux-2.6/include/linux/irqdomain.h
===
--- linux-2.6.orig/include/linux/irqdomain.h
+++ linux-2.6/include/linux/irqdomain.h
@@ -66,6 +66,10 @@ struct irq_domain_ops {
unsigned long *out_hwirq, unsigned int *out_type);
};
+extern struct irq_domain_ops irq_generic_chip_ops;
+
+struct irq_domain_chip_generic;
+
/**
* struct irq_domain - Hardware interrupt number translation object
* @link: Element in global irq_domain list.
@@ -109,8 +113,16 @@ struct irq_domain {
/* Optional device node pointer */
struct device_node *of_node;
+ /* Optional pointer to generic interrupt chips */
+ struct irq_domain_chip_generic *gc;
};
+#define IRQ_DOMAIN_MAP_LEGACY 0 /* driver allocated fixed range of irqs.
+* ie. legacy 8259, gets irqs 1..15 */
+#define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
+#define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
+#define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
+
#ifdef CONFIG_IRQ_DOMAIN
struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
unsigned int size,
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq
[patch 8/8] irqchip: sun4i: Convert to generic irq chip
Proof of concept patch to demonstrate the new irqdomain support for
the generic irq chip. Untested !!
Signed-off-by: Thomas Gleixner t...@linutronix.de
Cc: Maxime Ripard maxime.rip...@free-electrons.com
---
drivers/irqchip/irq-sun4i.c | 100
1 file changed, 29 insertions(+), 71 deletions(-)
Index: linux-2.6/drivers/irqchip/irq-sun4i.c
===
--- linux-2.6.orig/drivers/irqchip/irq-sun4i.c
+++ linux-2.6/drivers/irqchip/irq-sun4i.c
@@ -32,70 +32,43 @@
#define SUN4I_IRQ_FIQ_PENDING_REG(x) (0x20 + 0x4 * x)
#define SUN4I_IRQ_ENABLE_REG(x)(0x40 + 0x4 * x)
#define SUN4I_IRQ_MASK_REG(x) (0x50 + 0x4 * x)
+#define SUN4I_NUM_CHIPS3
+#define SUN4I_IRQS_PER_CHIP32
static void __iomem *sun4i_irq_base;
static struct irq_domain *sun4i_irq_domain;
static asmlinkage void __exception_irq_entry sun4i_handle_irq(struct pt_regs
*regs);
-void sun4i_irq_ack(struct irq_data *irqd)
+static int __init sun4i_init_domain_chips(void)
{
- unsigned int irq = irqd_to_hwirq(irqd);
- unsigned int irq_off = irq % 32;
- int reg = irq / 32;
- u32 val;
-
- val = readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg));
- writel(val | (1 irq_off),
- sun4i_irq_base + SUN4I_IRQ_PENDING_REG(reg));
-}
-
-static void sun4i_irq_mask(struct irq_data *irqd)
-{
- unsigned int irq = irqd_to_hwirq(irqd);
- unsigned int irq_off = irq % 32;
- int reg = irq / 32;
- u32 val;
-
- val = readl(sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
- writel(val ~(1 irq_off),
- sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
-}
-
-static void sun4i_irq_unmask(struct irq_data *irqd)
-{
- unsigned int irq = irqd_to_hwirq(irqd);
- unsigned int irq_off = irq % 32;
- int reg = irq / 32;
- u32 val;
-
- val = readl(sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
- writel(val | (1 irq_off),
- sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
-}
-
-static struct irq_chip sun4i_irq_chip = {
- .name = sun4i_irq,
- .irq_ack= sun4i_irq_ack,
- .irq_mask = sun4i_irq_mask,
- .irq_unmask = sun4i_irq_unmask,
-};
-
-static int sun4i_irq_map(struct irq_domain *d, unsigned int virq,
-irq_hw_number_t hw)
-{
- irq_set_chip_and_handler(virq, sun4i_irq_chip,
-handle_level_irq);
- set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
-
+ unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
+ struct irq_chip_generic *gc;
+ int i, ret, base = 0;
+
+ ret = irq_alloc_domain_generic_chips(d, SUN4I_IRQS_PER_CHIP, 1,
+sun4i_irq, handle_level_irq,
+clr, 0, IRQ_GC_INIT_MASK_CACHE);
+ if (ret)
+ return ret;
+
+ for (i = 0; i SUN4I_NUM_CHIPS; i++, base += SUN4I_IRQS_PER_CHIP) {
+ gc = irq_get_domain_generic_chip(sun4i_irq_domain, base);
+ gc-reg_base = sun4i_irq_base;
+ gc-chip_types[0].regs.mask = SUN4I_IRQ_ENABLE_REG(i);
+ gc-chip_types[0].regs.ack = SUN4I_IRQ_PENDING_REG(i);
+ gc-chip_types[0].chip.mask = irq_gc_mask_clr_bit;
+ gc-chip_types[0].chip.ack = irq_gc_ack_set_bit;
+ gc-chip_types[0].chip.unmask = irq_gc_mask_set_bit;
+
+ /* Disable, mask and clear all pending interrupts */
+ writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(i));
+ writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(i));
+ writel(0x, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(i));
+ }
return 0;
}
-static struct irq_domain_ops sun4i_irq_ops = {
- .map = sun4i_irq_map,
- .xlate = irq_domain_xlate_onecell,
-};
-
static int __init sun4i_of_init(struct device_node *node,
struct device_node *parent)
{
@@ -104,21 +77,6 @@ static int __init sun4i_of_init(struct d
panic(%s: unable to map IC registers\n,
node-full_name);
- /* Disable all interrupts */
- writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(0));
- writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(1));
- writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(2));
-
- /* Mask all the interrupts */
- writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(0));
- writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(1));
- writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(2));
-
- /* Clear all the pending interrupts */
- writel(0x, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(0));
- writel(0x, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(1));
- writel(0x, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(2
Re: [RFC patch 7/8] genirq: generic chip: Add linear irq domain support
On Sat, 4 May 2013, Sebastian Hesselbarth wrote:
On 05/03/2013 11:50 PM, Thomas Gleixner wrote:
Provide infrastructure for irq chip implementations which work on
linear irq domains.
Thomas,
I am happy that I put you into rant mode. It took me little more
than an hour to read through your patches, prepare orion irqchip
driver on top of them and finally got it working.
Cool.
Anyway, I found some more issues.
That was expected. :)
+ for (i = 0; i numchips; i++, gc++) {
The memory you allocated for gc, num_ct * ct, and dgc doesn't allow
to increment through gc. gc is struct irq_chip_generic * but next
gc is at sizeof(*gc) + num_ct * sizeof(struct irq_chip_type).
This also affects indexing dgc-gc later.
Indeed.
I chose to fix it by having an index helper but that first maps
dgc-gc to unsigned char * and then adds the correct offset. Not
void * is the preferred over uchar *
nice but it works. Maybe having real array of ptr to gc is more
intuitive here even if we will have to have split kzallocs.
No, you still can have a single kzalloc. It's just a matter of setting
the pointers correctly.
+ irq_init_generic_chip(gc, name, num_ct, i * irqs_per_chip,
+ NULL, handler);
irq_init_generic_chip does not take care of initalizing ct
mask_cache ptr. This should be done here.
Right.
+ gc-domain = d;
+ raw_spin_lock_irqsave(gc_lock, flags);
+ list_add_tail(gc-list, gc_list);
+ raw_spin_unlock_irqrestore(gc_lock, flags);
+ }
+ d-gc = dgc;
Moving this assignment above the for loop allows to get
gc by index as indexing helper relies on domain, not domain
generic chip.
You want me to prepare patches for the above? Maybe you can
That'd be nice.
split your RFC into 1-6 and 7-8. Then you can have 1-6 applied
independently of irq_domain_generic_chip stuff.
Thanks for the RFC again!
Welcome. Have fun!
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/5] irqchip: add support for Marvell Orion SoCs
On Fri, 3 May 2013, Sebastian Hesselbarth wrote:
On 05/03/13 14:55, Russell King - ARM Linux wrote:
This is where it starts to get tricky, because I can't see how you'd
merge the irq_alloc_generic_chip() and irq_setup_generic_chip() with
this. Maybe you don't need to do anything here and just do all that
in orion_of_init() instead? But then you seem to need to know the
virq range before hand, and I can't see how that's known. Maybe Thomas
can provide some enlightenment about how the gc irqchip stuff works
with the irq domain stuff...
Exactly, and that is what I am looking into right now. But hell, I am
not an expert in linux irq yet. Moreover, I am not even sure if it is
okay to rely on irqdomain or at least irq_data-hw_irq at all.
Here is a solution to that problem.
1) It adds a mask field to irq_data so we dont have to compute the
mask over and over
2) For compability with existing users it populates the mask with
1 (d-irq - gc-irq_base)
3) It gives you the option to disable that mask setup or let it
generate from d-hwirq
I'm still looking into a way how to proper support the generic chip /
linear domain mapping in the setup path. Will send you a draft patch
to play with later.
Thanks,
tglx
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -119,6 +119,7 @@ struct irq_domain;
/**
* struct irq_data - per irq and irq chip data passed down to chip functions
+ * @mask: precomputed bitmask for accessing the chip registers
* @irq: interrupt number
* @hwirq: hardware interrupt number, local to the interrupt domain
* @node: node index useful for balancing
@@ -138,6 +139,7 @@ struct irq_domain;
* irq_data.
*/
struct irq_data {
+ u32 mask;
unsigned intirq;
unsigned long hwirq;
unsigned intnode;
@@ -700,10 +702,14 @@ struct irq_chip_generic {
* @IRQ_GC_INIT_NESTED_LOCK: Set the lock class of the irqs to nested for
* irq chips which need to call irq_set_wake() on
* the parent irq. Usually GPIO implementations
+ * @IRQ_GC_NO_MASK:Do not calculate irq_data-mask
+ * @IRQ_GC_MASK_FROM_HWIRQ:Calculate irq_data-mask from the hwirq number
*/
enum irq_gc_flags {
IRQ_GC_INIT_MASK_CACHE = 1 0,
IRQ_GC_INIT_NESTED_LOCK = 1 1,
+ IRQ_GC_NO_MASK = 1 2,
+ IRQ_GC_MASK_FROM_HWIRQ = 1 4,
};
/* Generic chip callback functions */
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -39,7 +39,7 @@ void irq_gc_noop(struct irq_data *d)
void irq_gc_mask_disable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + cur_regs(d)-disable);
@@ -57,7 +57,7 @@ void irq_gc_mask_disable_reg(struct irq_
void irq_gc_mask_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
gc-mask_cache |= mask;
@@ -75,7 +75,7 @@ void irq_gc_mask_set_bit(struct irq_data
void irq_gc_mask_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
gc-mask_cache = ~mask;
@@ -93,7 +93,7 @@ void irq_gc_mask_clr_bit(struct irq_data
void irq_gc_unmask_enable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + cur_regs(d)-enable);
@@ -108,7 +108,7 @@ void irq_gc_unmask_enable_reg(struct irq
void irq_gc_ack_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
@@ -122,7 +122,7 @@ void irq_gc_ack_set_bit(struct irq_data
void irq_gc_ack_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = ~(1 (d-irq - gc-irq_base));
+ u32 mask = ~d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
@@ -136,7 +136,7 @@ void irq_gc_ack_clr_bit(struct irq_data
void
[RFC patch 1/8] genirq: generic chip: Remove the local cur_regs() function
From: Gerlando Falauto gerlando.fala...@keymile.com
Since we already have an irq_data_get_chip_type() function which returns
a pointer to irq_chip_type, use that instead of cur_regs().
Signed-off-by: Gerlando Falauto gerlando.fala...@keymile.com
Cc: Lennert Buytenhek ker...@wantstofly.org
Cc: Simon Guinot si...@sequanux.org
Cc: Joey Oravec jora...@drewtech.com
Cc: Ben Dooks ben-li...@fluff.org
Cc: Nicolas Pitre n...@fluxnic.net
Cc: Jason Cooper ja...@lakedaemon.net
Cc: Andrew Lunn and...@lunn.ch
Cc: Holger Brunck holger.bru...@keymile.com
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
kernel/irq/generic-chip.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
Index: tip/kernel/irq/generic-chip.c
===
--- tip.orig/kernel/irq/generic-chip.c
+++ tip/kernel/irq/generic-chip.c
@@ -16,11 +16,6 @@
static LIST_HEAD(gc_list);
static DEFINE_RAW_SPINLOCK(gc_lock);
-static inline struct irq_chip_regs *cur_regs(struct irq_data *d)
-{
- return container_of(d-chip, struct irq_chip_type, chip)-regs;
-}
-
/**
* irq_gc_noop - NOOP function
* @d: irq_data
@@ -39,10 +34,11 @@ void irq_gc_noop(struct irq_data *d)
void irq_gc_mask_disable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-disable);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.disable);
gc-mask_cache = ~mask;
irq_gc_unlock(gc);
}
@@ -57,11 +53,12 @@ void irq_gc_mask_disable_reg(struct irq_
void irq_gc_mask_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
gc-mask_cache |= mask;
- irq_reg_writel(gc-mask_cache, gc-reg_base + cur_regs(d)-mask);
+ irq_reg_writel(gc-mask_cache, gc-reg_base + ct-regs.mask);
irq_gc_unlock(gc);
}
@@ -75,11 +72,12 @@ void irq_gc_mask_set_bit(struct irq_data
void irq_gc_mask_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
gc-mask_cache = ~mask;
- irq_reg_writel(gc-mask_cache, gc-reg_base + cur_regs(d)-mask);
+ irq_reg_writel(gc-mask_cache, gc-reg_base + ct-regs.mask);
irq_gc_unlock(gc);
}
@@ -93,10 +91,11 @@ void irq_gc_mask_clr_bit(struct irq_data
void irq_gc_unmask_enable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-enable);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.enable);
gc-mask_cache |= mask;
irq_gc_unlock(gc);
}
@@ -108,10 +107,11 @@ void irq_gc_unmask_enable_reg(struct irq
void irq_gc_ack_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
irq_gc_unlock(gc);
}
@@ -122,10 +122,11 @@ void irq_gc_ack_set_bit(struct irq_data
void irq_gc_ack_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = ~(1 (d-irq - gc-irq_base));
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
irq_gc_unlock(gc);
}
@@ -136,11 +137,12 @@ void irq_gc_ack_clr_bit(struct irq_data
void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
u32 mask = 1 (d-irq - gc-irq_base);
irq_gc_lock(gc);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-mask);
- irq_reg_writel(mask, gc-reg_base + cur_regs(d)-ack);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.mask);
+ irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
irq_gc_unlock(gc);
}
@@ -151,10 +153,11 @@ void irq_gc_mask_disable_reg_and_ack(str
void irq_gc_eoi(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct
[RFC patch 3/8] genirq: generic chip: Handle separate mask registers
From: Gerlando Falauto gerlando.fala...@keymile.com
There are cases where all irq_chip_type instances have separate mask
registers, making a shared mask register cache unsuitable for the
purpose.
Introduce a new flag IRQ_GC_MASK_CACHE_PER_TYPE. If set, point the per
chip mask pointer to the per chip private mask cache instead.
[ tglx: Simplified code, renamed flag and massaged changelog ]
Signed-off-by: Gerlando Falauto gerlando.fala...@keymile.com
Cc: Lennert Buytenhek ker...@wantstofly.org
Cc: Simon Guinot si...@sequanux.org
Cc: Joey Oravec jora...@drewtech.com
Cc: Ben Dooks ben-li...@fluff.org
Cc: Nicolas Pitre n...@fluxnic.net
Cc: Jason Cooper ja...@lakedaemon.net
Cc: Andrew Lunn and...@lunn.ch
Cc: Holger Brunck holger.bru...@keymile.com
Cc: linux-arm-ker...@lists.infradead.org
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h |2 ++
kernel/irq/generic-chip.c | 17 ++---
2 files changed, 12 insertions(+), 7 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -704,10 +704,12 @@ struct irq_chip_generic {
* @IRQ_GC_INIT_NESTED_LOCK: Set the lock class of the irqs to nested for
* irq chips which need to call irq_set_wake() on
* the parent irq. Usually GPIO implementations
+ * @IRQ_GC_MASK_CACHE_PER_TYPE:Mask cache is chip type private
*/
enum irq_gc_flags {
IRQ_GC_INIT_MASK_CACHE = 1 0,
IRQ_GC_INIT_NESTED_LOCK = 1 1,
+ IRQ_GC_MASK_CACHE_PER_TYPE = 1 2,
};
/* Generic chip callback functions */
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -241,18 +241,21 @@ void irq_setup_generic_chip(struct irq_c
{
struct irq_chip_type *ct = gc-chip_types;
unsigned int i;
+ u32 *mskptr = gc-mask_cache, mskreg = ct-regs.mask;
raw_spin_lock(gc_lock);
list_add_tail(gc-list, gc_list);
raw_spin_unlock(gc_lock);
- /* Init mask cache ? */
- if (flags IRQ_GC_INIT_MASK_CACHE)
- gc-mask_cache = irq_reg_readl(gc-reg_base + ct-regs.mask);
-
- /* Initialize mask cache pointer */
- for (i = 0; i gc-num_ct; i++)
- ct[i].mask_cache = gc-mask_cache;
+ for (i = 0; i gc-num_ct; i++) {
+ if (flags IRQ_GC_MASK_CACHE_PER_TYPE) {
+ mskptr = ct[i].mask_cache_priv;
+ mskreg = ct[i].regs.mask;
+ }
+ ct[i].mask_cache = mskptr;
+ if (flags IRQ_GC_INIT_MASK_CACHE)
+ *mskptr = irq_reg_readl(gc-reg_base + mskreg);
+ }
for (i = gc-irq_base; msk; msk = 1, i++) {
if (!(msk 0x01))
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[RFC patch 4/8] genirq: generic chip: Cache per irq bit mask
Cache the per irq bit mask instead of recalculating it over and over.
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h |4
kernel/irq/generic-chip.c | 23 ++-
2 files changed, 18 insertions(+), 9 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -119,6 +119,7 @@ struct irq_domain;
/**
* struct irq_data - per irq and irq chip data passed down to chip functions
+ * @mask: precomputed bitmask for accessing the chip registers
* @irq: interrupt number
* @hwirq: hardware interrupt number, local to the interrupt domain
* @node: node index useful for balancing
@@ -138,6 +139,7 @@ struct irq_domain;
* irq_data.
*/
struct irq_data {
+ u32 mask;
unsigned intirq;
unsigned long hwirq;
unsigned intnode;
@@ -705,11 +707,13 @@ struct irq_chip_generic {
* irq chips which need to call irq_set_wake() on
* the parent irq. Usually GPIO implementations
* @IRQ_GC_MASK_CACHE_PER_TYPE:Mask cache is chip type private
+ * @IRQ_GC_NO_MASK:Do not calculate irq_data-mask
*/
enum irq_gc_flags {
IRQ_GC_INIT_MASK_CACHE = 1 0,
IRQ_GC_INIT_NESTED_LOCK = 1 1,
IRQ_GC_MASK_CACHE_PER_TYPE = 1 2,
+ IRQ_GC_NO_MASK = 1 3,
};
/* Generic chip callback functions */
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -35,7 +35,7 @@ void irq_gc_mask_disable_reg(struct irq_
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.disable);
@@ -54,7 +54,7 @@ void irq_gc_mask_set_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
*ct-mask_cache |= mask;
@@ -73,7 +73,7 @@ void irq_gc_mask_clr_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
*ct-mask_cache = ~mask;
@@ -92,7 +92,7 @@ void irq_gc_unmask_enable_reg(struct irq
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.enable);
@@ -108,7 +108,7 @@ void irq_gc_ack_set_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
@@ -123,7 +123,7 @@ void irq_gc_ack_clr_bit(struct irq_data
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = ~(1 (d-irq - gc-irq_base));
+ u32 mask = ~(d-mask);
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.ack);
@@ -138,7 +138,7 @@ void irq_gc_mask_disable_reg_and_ack(str
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.mask);
@@ -154,7 +154,7 @@ void irq_gc_eoi(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct irq_chip_type *ct = irq_data_get_chip_type(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
irq_gc_lock(gc);
irq_reg_writel(mask, gc-reg_base + ct-regs.eoi);
@@ -172,7 +172,7 @@ void irq_gc_eoi(struct irq_data *d)
int irq_gc_set_wake(struct irq_data *d, unsigned int on)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 (d-irq - gc-irq_base);
+ u32 mask = d-mask;
if (!(mask gc-wake_enabled))
return -EINVAL;
@@ -264,6 +264,11 @@ void irq_setup_generic_chip
[RFC patch 0/8] genirq: Support for irq domains in generic irq chip
The ongoing device tree support for ARM is creating new irq chip drivers in drivers/irqchip/ in a frenzy. Quite some of them are ripping out the generic irq chip implementation from arch/arm/* and just creating the same mess of duplicated code again, which was cleaned up with the generic irq chip implementation with a lot of effort. Sigh! I already prodded a few people in reviews to tackle that issue with no outcome. Even more sigh! Poor Sebastian triggered me into rant mode, but he ad hoc volunteered to give it a try. YAY! Though he asked for a bit of kickstart help. So I squeezed out a few spare cycles and implemented the basics as far as I think that they should work. The following series contains the missing bits and pieces including a somehow forgotten and now slightly modified series from Gerlando adding support for irq chips which need separate mask caches for different chip (control flow) types. At the moment this supports only linear irq domains, but it could be extended to other types as well if the need arises. Though the ARM chips are pretty much all about linear domains AFAICT. It also lacks support for removing an irq domain at the moment, but that should be rather trivial to fix. The last patch in the series is a blind conversion of the irq-sun4i irq chip driver, completely untested and not even compiled. I just added it for demonstration purposes. As Russell expected, there is a lot of consolidation potential. The changelog of that patch is: 1 file changed, 29 insertions(+), 71 deletions(-) The preparing series has 4 files changed, 294 insertions(+), 50 deletions(-) So for removing 42 lines in a single driver the core grows 244 lines including header changes and comments. Convert 6 drivers and we are more than even because we get the benefit of sharing and therefor exposing the same code to broader testing and utilization. We have already 11 of those candidates in drivers/irqchips and new ones are knocking on the door. There might be even more consolidation potential, but I leave that to the DT/irq domain experts. WARNING: It's compile tested only. So if you find bugs you can keep them and fix them yourself :) Thanks, tglx --- drivers/irqchip/irq-sun4i.c | 100 --- include/linux/irq.h | 45 ++- include/linux/irqdomain.h | 12 + kernel/irq/generic-chip.c | 281 +--- kernel/irq/irqdomain.c |6 5 files changed, 323 insertions(+), 121 deletions(-) ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[RFC patch 7/8] genirq: generic chip: Add linear irq domain support
Provide infrastructure for irq chip implementations which work on
linear irq domains.
- Interface to allocate multiple generic chips which are associated to
the irq domain.
- Interface to get the generic chip pointer for a particular hardware
interrupt in the domain.
- irq domain mapping function to install the chip for a particular
interrupt.
Note: This lacks a removal function for now, but this is a draft patch
the ARM folks to work on.
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
include/linux/irq.h | 30 +++
include/linux/irqdomain.h | 12 +++
kernel/irq/generic-chip.c | 179 --
kernel/irq/irqdomain.c|6 -
4 files changed, 215 insertions(+), 12 deletions(-)
Index: linux-2.6/include/linux/irq.h
===
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -678,6 +678,8 @@ struct irq_chip_type {
* @wake_active: Interrupt is marked as an wakeup from suspend source
* @num_ct:Number of available irq_chip_type instances (usually 1)
* @private: Private data for non generic chip callbacks
+ * @installed: bitfield to denote installed interrupts
+ * @domain:irq domain pointer
* @list: List head for keeping track of instances
* @chip_types:Array of interrupt irq_chip_types
*
@@ -699,6 +701,8 @@ struct irq_chip_generic {
u32 wake_active;
unsigned intnum_ct;
void*private;
+ unsigned long installed;
+ struct irq_domain *domain;
struct list_headlist;
struct irq_chip_typechip_types[0];
};
@@ -719,6 +723,24 @@ enum irq_gc_flags {
IRQ_GC_NO_MASK = 1 3,
};
+/*
+ * struct irq_domain_chip_generic - Generic irq chip data structure for irq
domains
+ * @irqs_per_chip: Number of interrupts per chip
+ * @num_chips: Number of chips
+ * @irq_flags_to_set: IRQ* flags to set on irq setup
+ * @irq_flags_to_clear:IRQ* flags to clear on irq setup
+ * @gc_flags: Generic chip specific setup flags
+ * @gc:Array of generic interrupt chips
+ */
+struct irq_domain_chip_generic {
+ unsigned intirqs_per_chip;
+ unsigned intnum_chips;
+ unsigned intirq_flags_to_clear;
+ unsigned intirq_flags_to_set;
+ enum irq_gc_flags gc_flags;
+ struct irq_chip_generic gc[0];
+};
+
/* Generic chip callback functions */
void irq_gc_noop(struct irq_data *d);
void irq_gc_mask_disable_reg(struct irq_data *d);
@@ -742,6 +764,14 @@ int irq_setup_alt_chip(struct irq_data *
void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
unsigned int clr, unsigned int set);
+struct irq_chip_generic *irq_get_domain_generic_chip(struct irq_domain *d,
unsigned int hw_irq);
+int irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip,
+ int num_ct, const char *name,
+ irq_flow_handler_t handler,
+ unsigned int clr, unsigned int set,
+ enum irq_gc_flags flags);
+
+
static inline struct irq_chip_type *irq_data_get_chip_type(struct irq_data *d)
{
return container_of(d-chip, struct irq_chip_type, chip);
Index: linux-2.6/include/linux/irqdomain.h
===
--- linux-2.6.orig/include/linux/irqdomain.h
+++ linux-2.6/include/linux/irqdomain.h
@@ -66,6 +66,10 @@ struct irq_domain_ops {
unsigned long *out_hwirq, unsigned int *out_type);
};
+extern struct irq_domain_ops irq_generic_chip_ops;
+
+struct irq_domain_chip_generic;
+
/**
* struct irq_domain - Hardware interrupt number translation object
* @link: Element in global irq_domain list.
@@ -109,8 +113,16 @@ struct irq_domain {
/* Optional device node pointer */
struct device_node *of_node;
+ /* Optional pointer to generic interrupt chips */
+ struct irq_domain_chip_generic *gc;
};
+#define IRQ_DOMAIN_MAP_LEGACY 0 /* driver allocated fixed range of irqs.
+* ie. legacy 8259, gets irqs 1..15 */
+#define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
+#define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
+#define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
+
#ifdef CONFIG_IRQ_DOMAIN
struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
unsigned int size,
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -7,6 +7,7
[RFC patch 6/8] genirq: Split out code in generic chip
Preparatory patch for linear interrupt domains.
Signed-off-by: Thomas Gleixner t...@linutronix.de
---
kernel/irq/generic-chip.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
Index: linux-2.6/kernel/irq/generic-chip.c
===
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -186,6 +186,19 @@ int irq_gc_set_wake(struct irq_data *d,
return 0;
}
+static void
+irq_init_generic_chip(struct irq_chip_generic *gc, const char *name,
+ int num_ct, unsigned int irq_base,
+ void __iomem *reg_base, irq_flow_handler_t handler)
+{
+ raw_spin_lock_init(gc-lock);
+ gc-num_ct = num_ct;
+ gc-irq_base = irq_base;
+ gc-reg_base = reg_base;
+ gc-chip_types-chip.name = name;
+ gc-chip_types-handler = handler;
+}
+
/**
* irq_alloc_generic_chip - Allocate a generic chip and initialize it
* @name: Name of the irq chip
@@ -206,12 +219,8 @@ irq_alloc_generic_chip(const char *name,
gc = kzalloc(sz, GFP_KERNEL);
if (gc) {
- raw_spin_lock_init(gc-lock);
- gc-num_ct = num_ct;
- gc-irq_base = irq_base;
- gc-reg_base = reg_base;
- gc-chip_types-chip.name = name;
- gc-chip_types-handler = handler;
+ irq_init_generic_chip(gc, name, num_ct, irq_base, reg_base,
+ handler);
}
return gc;
}
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC patch 7/8] genirq: generic chip: Add linear irq domain support
On Fri, 3 May 2013, Russell King - ARM Linux wrote:
On Fri, May 03, 2013 at 09:50:53PM -, Thomas Gleixner wrote:
+ /* Init mask cache ? */
+ if (dgc-gc_flags IRQ_GC_INIT_MASK_CACHE) {
+ raw_spin_lock_irqsave(gc-lock, flags);
+ gc-mask_cache = irq_reg_readl(gc-reg_base + ct-regs.mask);
+ raw_spin_unlock_irqrestore(gc-lock, flags);
+ }
This looks a little weird to me - it seems that it'll re-read this
each time any irq is mapped in the domain, which is probably not
wanted.
Yes, it's sloppy in two aspects.
1) It does not respect the per irq type mask cache, which got
introduced in the same series
2) It rereads the mask cache for each mapping, but thats harmless
because it's proper serialized. We can avoid that by clearing the
IRQ_GC_INIT_MASK_CACHE bit when the first irq of that chip is
mapped.
Congrats, you found a bug and as I said:
WARNING: It's compile tested only. So if you find bugs you can keep
them and fix them yourself :)
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC patch 4/8] genirq: generic chip: Cache per irq bit mask
On Fri, 3 May 2013, Russell King - ARM Linux wrote: On Fri, May 03, 2013 at 09:50:50PM -, Thomas Gleixner wrote: - u32 mask = ~(1 (d-irq - gc-irq_base)); + u32 mask = ~(d-mask); I suspect the above changes are all a result of a search-and-replace which results in the cosmetic parens remaining. Would be nice to get rid of them too as they're completely unnecessary. Fair enough. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] irqchip: add support for Marvell Orion SoCs
Sebastian,
please do not take the rant below personally. You just happen to
trigger it.
On Thu, 2 May 2013, Sebastian Hesselbarth wrote:
+static void orion_irq_mask(struct irq_data *irqd)
+{
+ unsigned int irq = irqd_to_hwirq(irqd);
+ unsigned int irq_off = irq % 32;
+ int reg = irq / 32;
+ u32 val;
+
+ val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
+ writel(val ~(1 irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
+}
+
+static void orion_irq_unmask(struct irq_data *irqd)
+{
+ unsigned int irq = irqd_to_hwirq(irqd);
+ unsigned int irq_off = irq % 32;
+ int reg = irq / 32;
+ u32 val;
+
+ val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
+ writel(val | (1 irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
+}
I'm really tired of looking at the next incarnation of an OF/DT irq
chip driver, which reimplements stuff which I have consolidated in the
generic irq chip implementation with a lot of effort.
Just look at the various implementations in drivers/irqchip/ and find
out how similar they are. Moving code to drivers/irqchip/ does not
make an excuse for reestablishing the mess which was addressed by the
generic irq chip implementation.
Can you - and that means all of you ARM folks - please get your gear
together and add the missing features to the generic irq chip
implementation? I'm not going to accept more of that OF/DT frenzy.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/8] irq-imgpdc: add ImgTec PDC irqchip driver
On Wed, 24 Apr 2013, James Hogan wrote:
Thanks for the review Thomas!
On 23/04/13 16:09, Thomas Gleixner wrote:
On Tue, 23 Apr 2013, James Hogan wrote:
+ spinlock_t lock;
raw_spinlock_t please
Okay.
If I understand right, this would be because on RT, spinlock_t becomes a
mutex and won't work correctly with irqs actually disabled for the irq
callbacks below, is that right?
Yep.
I'll look into this. kernel/irq/generic-chip.c was added after this
driver was written.
Fair enough.
+static void pdc_intc_perip_isr(unsigned int irq, struct irq_desc *desc)
+{
+ struct pdc_intc_priv *priv;
+ unsigned int i, irq_no;
+
+ priv = (struct pdc_intc_priv *)irq_desc_get_handler_data(desc);
+
+ /* find the peripheral number */
+ for (i = 0; i priv-nr_perips; ++i)
+ if (irq == priv-perip_irqs[i])
+ goto found;
Whee. Aren't these numbers linear ?
Not necessarily as they're virtual irq numbers obtained via
platform_get_irq(), which come individually from device tree. Even their
hardware irq numbers aren't linear as they're not wired to their irqchip
in the same order:
pdc: pdc@0x02006000 {
interrupt-controller;
#interrupt-cells = 3;
reg = 0x02006000 0x1000;
compatible = img,pdc-intc;
num-perips = 3;
num-syswakes = 3;
interrupts = 18 4 /* level */, /* Syswakes */
30 4 /* level */, /* Perip 0 (RTC) */
29 4 /* level */, /* Perip 1 (IR) */
31 4 /* level */; /* Perip 2 (WDT) */
};
Interesting.
+static int pdc_intc_remove(struct platform_device *pdev)
+{
+ struct pdc_intc_priv *priv = platform_get_drvdata(pdev);
+
+ irq_domain_remove(priv-domain);
And the rest of the resources is still there?
I was under the impression devm_kzalloc and devm_ioremap took care of
that in both the probe error case and the remove case:
* devm_kzalloc - Resource-managed kzalloc
* Managed kzalloc. Memory allocated with this function is
* automatically freed on driver detach.
* devm_ioremap - Managed ioremap()
* Managed ioremap(). Map is automatically unmapped on driver detach.
I may have misunderstood the whole point of their existence though?
No, that was just me missing the devm_ in front of
kzalloc/ioremap. So you're good.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/8] irq-imgpdc: add ImgTec PDC irqchip driver
On Tue, 23 Apr 2013, James Hogan wrote:
+/**
+ * struct pdc_intc_priv - private pdc interrupt data.
+ * @nr_perips: Number of peripheral interrupt signals.
+ * @nr_syswakes: Number of syswake signals.
+ * @perip_irqs: List of peripheral IRQ numbers handled.
+ * @syswake_irq: Shared PDC syswake IRQ number.
+ * @domain: IRQ domain for PDC peripheral and syswake IRQs.
+ * @pdc_base:Base of PDC registers.
+ * @lock:Lock to protect the PDC syswake registers.
+ */
+struct pdc_intc_priv {
+ unsigned intnr_perips;
+ unsigned intnr_syswakes;
+ unsigned int*perip_irqs;
+ unsigned intsyswake_irq;
+ struct irq_domain *domain;
+ void __iomem*pdc_base;
+
+ spinlock_t lock;
raw_spinlock_t please
+/* Generic IRQ callbacks */
+
+#define SYS0_HWIRQ 8
+
+static unsigned int hwirq_is_syswake(irq_hw_number_t hw)
+{
+ return hw = SYS0_HWIRQ;
+}
+
+static unsigned int hwirq_to_syswake(irq_hw_number_t hw)
+{
+ return hw - SYS0_HWIRQ;
+}
+
+static irq_hw_number_t syswake_to_hwirq(unsigned int syswake)
+{
+ return SYS0_HWIRQ + syswake;
+}
+
+static struct pdc_intc_priv *irqd_to_priv(struct irq_data *data)
+{
+ return (struct pdc_intc_priv *)data-domain-host_data;
+}
+
+static void perip_irq_mask(struct irq_data *data)
+{
+ struct pdc_intc_priv *priv = irqd_to_priv(data);
+ unsigned int irq_route;
+ unsigned long flags;
+
+ spin_lock_irqsave(priv-lock, flags);
This is always called with interrupts disabled.
+ irq_route = pdc_read(priv, PDC_IRQ_ROUTE);
+ irq_route = ~(1 data-hwirq);
Why not cache the mask value ?
+ pdc_write(priv, PDC_IRQ_ROUTE, irq_route);
+ spin_unlock_irqrestore(priv-lock, flags);
+}
+
+static void perip_irq_unmask(struct irq_data *data)
+{
+ struct pdc_intc_priv *priv = irqd_to_priv(data);
+ unsigned int irq_route;
+ unsigned long flags;
+
+ spin_lock_irqsave(priv-lock, flags);
+ irq_route = pdc_read(priv, PDC_IRQ_ROUTE);
+ irq_route |= 1 data-hwirq;
+ pdc_write(priv, PDC_IRQ_ROUTE, irq_route);
This code is another slightly different copy of stuff which is
available in kernel/irq/generic-chip.c
Can we please stop the code duplication and reuse existing
infrastructure? Don't tell me it does not work for you. I sent out a
patch yesterday which makes the code suitable for irq domains.
+static int syswake_irq_set_type(struct irq_data *data, unsigned int
flow_type)
+{
+ struct pdc_intc_priv *priv = irqd_to_priv(data);
+ unsigned int syswake = hwirq_to_syswake(data-hwirq);
+ unsigned int irq_mode;
+ unsigned int soc_sys_wake_regoff, soc_sys_wake;
+ unsigned long flags;
+
+ /* translate to syswake IRQ mode */
+ switch (flow_type) {
+ case IRQ_TYPE_EDGE_BOTH:
+ irq_mode = PDC_SYS_WAKE_INT_CHANGE;
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ irq_mode = PDC_SYS_WAKE_INT_UP;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ irq_mode = PDC_SYS_WAKE_INT_DOWN;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ irq_mode = PDC_SYS_WAKE_INT_HIGH;
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ irq_mode = PDC_SYS_WAKE_INT_LOW;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ spin_lock_irqsave(priv-lock, flags);
All irqchip callbacks are called with interrupts disabled.
+static void pdc_intc_perip_isr(unsigned int irq, struct irq_desc *desc)
+{
+ struct pdc_intc_priv *priv;
+ unsigned int i, irq_no;
+
+ priv = (struct pdc_intc_priv *)irq_desc_get_handler_data(desc);
+
+ /* find the peripheral number */
+ for (i = 0; i priv-nr_perips; ++i)
+ if (irq == priv-perip_irqs[i])
+ goto found;
Whee. Aren't these numbers linear ?
+ /* should never get here */
+ return;
+found:
+
+ /* pass on the interrupt */
+ irq_no = irq_linear_revmap(priv-domain, i);
+ generic_handle_irq(irq_no);
+}
+
+static void pdc_intc_syswake_isr(unsigned int irq, struct irq_desc *desc)
+{
+ struct pdc_intc_priv *priv;
+ unsigned int syswake, irq_no;
+ unsigned int status;
+
+ priv = (struct pdc_intc_priv *)irq_desc_get_handler_data(desc);
+
+ spin_lock(priv-lock);
+ status = pdc_read(priv, PDC_IRQ_STATUS);
+ status = pdc_read(priv, PDC_IRQ_ENABLE);
+ spin_unlock(priv-lock);
+
+ status = (1 priv-nr_syswakes) - 1;
+
+ for (syswake = 0; status; status = 1, ++syswake) {
+ /* Has this sys_wake triggered? */
+ if (!(status 1))
+ continue;
+
+ irq_no = irq_linear_revmap(priv-domain,
+
Re: [PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)
On Wed, 13 Mar 2013, John Stultz wrote:
On 03/13/2013 02:27 PM, Christian Daudt wrote:
This adds support for the Broadcom timer, used in the following SoCs:
BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
[snip]
Signed-off-by: Christian Daudt c...@broadcom.com
Acked-by: Arnd Bergmann a...@arndb.de
Acked-by: John Stultz john.stu...@linaro.org
Reviewed-by: Stephen Warren swar...@nvidia.com
Hey Olof,
So Christian mentioned you were hoping I'd pull these in through my tree,
but I'd really rather these go via the arch-soc tree, since that is more
likely where they will be tested and more intelligently reviewed.
I've mentioned before my distaste for the drivers/clocksource directory
becoming a dumping ground for arch specific (for the most part arm)
clocksource, and more recently clockevent, drivers. I initially wanted the
OTOH, if they are all in one place we have a better chance to analyze
them in bulk, find similarities and patterns for consolidation. And
it's simpler for the developer of a new SoC support to find the
matching driver for his IP block instead of copying stuff from one
mach directory to the next. We've been there :)
But I'd really rather the patch flow for arch specific clocksource and
clockevent drivers go through the arch tree, since there's a better chance
folks will be building and testing against other arch specific changes that
might cause problems. There's just no way for me to be able to intelligently
review these sorts of changes.
I agree with the build and test part, but you can still review the
code in general - correctness of the particular hw details aside. I
just opened a random one there and found:
static cycle_t vt8500_timer_read(struct clocksource *cs)
{
int loops = msecs_to_loops(10);
writel(3, regbase + TIMER_CTRL_VAL);
while ((readl((regbase + TIMER_AS_VAL)) TIMER_COUNT_R_ACTIVE)
--loops)
cpu_relax();
return readl(regbase + TIMER_COUNT_VAL);
You don't need particular hw knowledge to see that this looks more
than fishy at least without comments describing the hardware designers
brainfart.
Now, if there are changes to the clocksource core code, then I definitely want
to be in the path there. Additionally meta-drivers like mmio, I should
probably be more involved with, so they can be more properly integrated into
the clocksource core. Also for drivers that are actually arch shared (ie:
things like hpet/acpi_pm where ia64 and x86 share them).
But if its arch specific for hardware I don't have, I'd really prefer the arch
maintainer be the upstream path.
Thomas: Am I being too obstinate here? If not, should we drop F:
drivers/clocksource from the MAINTAINERS entry?
Maybe we should move the ARM specific ones into
drivers/clocksource/arm/ ?
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Thu, 12 Apr 2012, Benjamin Herrenschmidt wrote: On Wed, 2012-04-11 at 14:57 -0600, Grant Likely wrote: Yeah, I've got a different way to fix it though. There is exactly one user of irq_virq_count in-tree right now: PS3. Also, irq_virq_count is only useful for the NOMAP mapping. So, instead of having a single global irq_virq_count values, I've dropped it entirely and added a max_irq argument to irq_domain_add_nomap(). That makes it a property of an individual nomap irq domain instead of a global system settting. Hopefully I'll have a draft patch ready today. That works for me. I'll send patches for cleanup MPIC as well. One thing tho (Thomas, Russell) is that I like using set_irq_trigger to establish the defaults in mpic, ie, it does the descriptor locking etc... for me, I'd rather avoid open coding all of that. What I need is a variant that doesn't actually change the trigger but instead initializes the irq_desc with whatever settings the HW currently has (ie, I need to make sure things are properly in sync) though other implementations may want to use that for defaults. Any objection to defining something like IRQ_TYPE_DEFAULT ? I was thinking about making it equal to IRQ_TYPE_SENSE_MASK since that can obviously not be a valid trigger value and is distinct from IRQ_TYPE_NONE. No objections. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Thu, 5 Apr 2012, Andreas Schwab wrote: Grant Likely grant.lik...@secretlab.ca writes: I bet it is NR_IRQS related. You have SPARSE_IRQ enabled, which means the maximum number of irq_descs is IRQ_BITMAP_BITS (NR_IRQS + 8192). The actual definition uses NR_IRQS + 8196. Guess that's a typo. (Does it really make sense to add NR_IRQS here?) diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h index cf417e51..9edf499 100644 --- a/arch/powerpc/include/asm/irq.h +++ b/arch/powerpc/include/asm/irq.h @@ -20,7 +20,7 @@ /* Define a way to iterate across irqs. */ #define for_each_irq(i) \ - for ((i) = 0; (i) NR_IRQS; ++(i)) + for ((i) = 0; (i) nr_irqs; ++(i)) There are exactly two uses of for_each_irq, one is related to cpu hotplug, the other to kexec, so that cannot make any difference. Though that wants to be fixed nevertheless. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Fri, 6 Apr 2012, Andreas Schwab wrote:
Grant Likely grant.lik...@secretlab.ca writes:
Can you attach console output logs for each of configs above and also
with NR_IRQS=128? That might give me some clues as to which specific
code is causing the issues.
It really looks like the issue starts when irq_expand_nr_irqs is called
the first time to make nr_irqs bigger than NR_IRQS.
And it looks like the irqdomain code is the real culprit.
void irq_set_virq_count(unsigned int count)
{
pr_debug(irq: Trying to set virq count to %d\n, count);
BUG_ON(count NUM_ISA_INTERRUPTS);
if (count NR_IRQS)
irq_virq_count = count;
}
That looks simply wrong.
s/NR_IRQS/nr_irqs/ should do the trick.
Thanks,
tglx
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Fri, 6 Apr 2012, Andreas Schwab wrote:
Thomas Gleixner t...@linutronix.de writes:
And it looks like the irqdomain code is the real culprit.
void irq_set_virq_count(unsigned int count)
{
pr_debug(irq: Trying to set virq count to %d\n, count);
BUG_ON(count NUM_ISA_INTERRUPTS);
if (count NR_IRQS)
irq_virq_count = count;
}
That looks simply wrong.
There is a single use of irq_set_virq_count, which is only relevant to
the PS3.
Though irq_virq_count is statically initialized to NR_IRQS and it's
used in the code more than once.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Tue, 3 Apr 2012, Benjamin Herrenschmidt wrote: On Mon, 2012-04-02 at 22:55 +0100, Russell King - ARM Linux wrote: Well, presumably someone is calling irq_set_irq_type() asking explicitly for IRQ_TYPE_NONE. The code will now (as it always used to before David's change) do exactly what you ask this to: it will ask the type to be set to none. If you don't want to set the type to none, don't call the interface asking for that to happen. I think part of the idea with NONE is to use it as reset that interrupt to the default setting, whatever that means ... Well, the ship side set_type function could simply leave it alone and not touch the thing at all. That's how the core code did until we discovered that we broke Russells toys that way. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Mon, 2 Apr 2012, Russell King - ARM Linux wrote: If we want to fix it a better way, then sure, that'll be good. But what we shouldn't do is re-introduce one regression to fix a different regression. So, Thomas, what do you think about providing a way that a disabled interrupt could have its pending status cleared such that when it's enabled, any queued events are ignored? Maybe an enable_and_clear_irq() ? We can make it a flag, which you can either add to the irq itself or hand it in on request_irq(). Does that require a special chip-callback() function as well ? Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Mon, 2 Apr 2012, Andreas Schwab wrote: Andreas Schwab sch...@linux-m68k.org writes: Grant Likely grant.lik...@secretlab.ca writes: This patch drops the powerpc-specific irq_map table and replaces it with directly using the irq_alloc_desc()/irq_free_desc() interfaces for allocating and freeing irq_desc structures. This breaks irqs on PowerMac G5. I see lost irq errors from the sata driver. When I revert a09b659cd68c10ec6a30cb91ebd2c327fcd5bfe5 (genirq: Fix long-term regression in genirq irq_set_irq_type() handling) on top of 3.4-rc1 the sata irq errors disappear, but I see a lot of spurious Hmm. Which irq chip is handling the interrupt for that sata irq ? Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
On Tue, 3 Apr 2012, Benjamin Herrenschmidt wrote: On Mon, 2012-04-02 at 22:52 +0200, Thomas Gleixner wrote: When I revert a09b659cd68c10ec6a30cb91ebd2c327fcd5bfe5 (genirq: Fix long-term regression in genirq irq_set_irq_type() handling) on top of 3.4-rc1 the sata irq errors disappear, but I see a lot of spurious Hmm. Which irq chip is handling the interrupt for that sata irq ? MPIC. The irq is a PCI IRQ, and so should be level-low, it should have been mapped by the PCI code using the of_* calls. So it's covered by this section: mpic_set_irq_type() if (flow_type == IRQ_TYPE_NONE) if (mpic-senses src mpic-senses_count) flow_type = mpic-senses[src]; if (flow_type == IRQ_TYPE_NONE) flow_type = IRQ_TYPE_LEVEL_LOW; And with IRQ_TYPE_NONE this is always going to end up with IRQ_TYPE_LEVEL_LOW simply because the only function which might set mpic-senses is mpic_set_default_senses(). And this function does not have a single caller . /me scratches head ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 06/14] ARM: kirkwood: convert uart0 to devicetree.
On Thu, 8 Mar 2012, Arnd Bergmann wrote: On Thursday 08 March 2012, Jason wrote: On Wed, Mar 07, 2012 at 09:13:04PM +, Arnd Bergmann wrote: Finally, something could be wrong with the interrupt controller. AFAICT, you register it through the device tree now, but it's also getting initialized through kirkwood_init_irq, so the numbers would all be wrong. Yes, I'm working through this now. mv_cesa silently fails to come up if it can't find the interrupt controller via fdt. So, it needs to get done. Right now, I'm looking at how mach-versatile does it (vic_init()) since it's also an ARM926ej-s... Ok, sounds good. It's not important whether it's an ARM9 or not btw. You should look at both anything that defines an interrupt-controller property as an example including Documentation/devicetree/bindings/arm/vic.txt and Documentation/devicetree/bindings/arm/gic.txt. (taking Thomas Gleixner on Cc) Since the orion irq chip is based on irqchip_generic, it would be perfect to have a generic irqchip binding to go along with kernel/irq/generic-chip.c. Not sure if anyone has thought about this before, but it looks like we can completely avoid using arch/arm/plat-orion/irq.c and arch/arm/mach-kirkwood/irq.c if we do that. Right. That should be trivial to implement. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 0/2] Simple irq_domain implementation
On Tue, 26 Jul 2011, Grant Likely wrote: In the interest of getting this infrastructure merged, I've reworked the basic irq_domain patch to implement only what is required for ARM device tree board support. That means it doesn't affect any architectures other than ARM, and I'll send follow-on patches targeted at v3.2 to migrate MIPS, Microblaze, x86 embedded and PowerPC over to it. There is also this GPIO driver pending for quite some time ... Thomas, can you please take a look? I've got devicetree board support queued up and ready to be merged but for this infrastructure. Looks reasonable, please merge them through your tree. Reviewed-by: Thomas Gleixner t...@linutronix.de Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/6] General device tree irq domain infrastructure
On Thu, 5 May 2011, Benjamin Herrenschmidt wrote: As for the mapping, I agree that the functionality is generally useful, I'm just not fond of the current implementation. I think it is more complex than it needs to be and I'm not excited about bring it over to the other architectures as-is. Nobody cares about the current implementation. What is important is indeed the functionality. The basic thing I think everybody agrees is that you need to extend the irq_desc (or data, whatever tglx prefers) with two bits of information: Some identifier of the domain and some identifier of the interrupt number within that domain. irq_data because that's what is handed into the callbacks and you probably want to have the HW number there. In addition, PIC code will need a way to perform efficient reverse mapping. You may decide that for simple IRQ controllers that handle a small linear range of interrupts, it's kosher to simply reserve a linear range of descriptors and use a simple offset, I'm find with that now that we no longer live in a world constrained by NR_IRQ. But the need for the radix tree remains for things that have massively large potential HW numbers such as we have on powerpc. For the majority of fixed hw interrupt controllers it is overkill. There is no need for a map table when all the irq descs (=32 of them) get allocated when the irq controller is instantiated and the mapping consists of 'virq = hw_irq + virq_base'. For instance, with the arm irq controllers, it's be more than sufficient to use irq_alloc_descs to obtain a range of irq numbers and then a simple of_irq_domain registration to handle the parsing. That's true if and only if you make NR_IRQ a non issue and if you accept the general wastage due to unused interrupts. The main problem has always been that hard limit which made me chose a more efficient mechanisms. Take a mac with 2 cascaded MPICs with 256 sources each which are mostly never used. I would need an NR_IRQs of 512 with your scheme (plus 16 because I do want to continue avoiding the ISA numbers), which is a waste of space, even with SPARSE_IRQ. Now I hope eventually NR_IRQ will go away and we'll have a more efficient mechanism to allocate descriptors and so it will become less of an issue. Well, NR_IRQS in the sparse case is irrelevant already and I'm looking into a way to remove the !SPARSE code completely. One thing we can do to avoid allocating 512 irq descriptors for the MAC case is to reserve the space and only allocate the descriptors you really need to be operational. For the cases where an interrupt controller isn't able to alloc all the irq descs at once, like for MSI, then yes the LINEAR and RADIX mappings are important. What bothers me though is the way irq_host needs to use unions and the -revmap_type value to implement multiple behaviours into a single type. That's the sort of thing that can be broken out into a library and instantiated only by the interrupt controllers that actually need it. But that's what it is really. You'll notice that on the fast path the interrupt controller code calls directly into the right type of revmap routine. You may want to refactor things a bit if you want, but the union served me well simply because I didnt have to bother doing lots of different alloc_bootmem back then. Nowadays, kmalloc is available much earlier so it might have become a non issue too. Similarly, it bothers me that that radix mapping makes up a significant portion of the code, yet it has only one user. significant ? Seriously ? Like 3 function calls ? It's nothing. We use an existing radix tree facility, and the amount of code in our irq.c is actually very small. Originally it was living in xics in fact, but I moved it out specifically because I wanted a common interface to remapping, so for example, I can expose the linux - hw mapping in debugfs generically, among others. And there is another reverse mapping implementation in the superH code. I'd be happier if each kind of mapping had its own structure that embedded a generic irq_host/irq_domain with mapping specific ops populated to manipulate it. Whatever, that's just plumbing, I don't care either way, that doesn't change the fact that the concept of domain doesn't have much to do with OF :-) Regardless, the immediate priority is to implement a mapper that will work for all architectures (or at least everything but powerpc and sparc). I oppose the implementation of a new mapper that doesn't include powerpc, that would be stupid. Either re-use ours or implement a new one that encompass our needs. I can't talk for sparc but I wouldn't be surprised if David thought about the same lines. x86 has already implemented a skeleton irq_domain because there wasn't any common code for it to use. ARM also needs the functionality immediately, and I don't want to see yet another
Re: [PATCH] x86: OLPC: add OLPC device-tree support (v3)
On Wed, 27 Oct 2010, Grant Likely wrote:
On Fri, Oct 22, 2010 at 03:58:46PM -0700, Andres Salomon wrote:
Make use of PROC_DEVICETREE to export the tree, and sparc's PROMTREE code to
call into OLPC's Open Firmware to build the tree.
v3: rename olpc_prom to olpc_dt
- rework Kconfig entries
- drop devtree build hook from proc, instead adding a call to x86's
paging_init (similarly to how sparc64 does it)
- switch allocation from using slab to alloc_bootmem. this allows
the DT to be built earlier during boot (during setup_arch); the
downside is that there are some 1200 bootmem reservations that are
done during boot. Not ideal..
- add a helper olpc_ofw_is_installed function to test for the
existence and successful detection of OLPC's OFW.
Signed-off-by: Andres Salomon dilin...@queued.net
Overall this patch looks fine, but it needs to be acked from the x86
maintainers, and I'd like it to have a cycle through linux-next before
it gets merged, so that means 2.6.38 because the 2.6.37 merge window
has already been open for almost a week.
Fine with me. That's sufficiently self contained :) So
Acked-by: Thomas Gleixner t...@linutronix.de
The promtree patches have been merged though.
Comments below.
g.
---
arch/x86/Kconfig|6 ++
arch/x86/include/asm/olpc_ofw.h |9 ++
arch/x86/include/asm/prom.h |1 +
arch/x86/kernel/Makefile|1 +
arch/x86/kernel/olpc_dt.c | 165
+++
arch/x86/kernel/olpc_ofw.c |5 +
arch/x86/mm/init_32.c |2 +
7 files changed, 189 insertions(+), 0 deletions(-)
create mode 100644 arch/x86/include/asm/prom.h
create mode 100644 arch/x86/kernel/olpc_dt.c
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cea0cd9..1d57e2b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2069,11 +2069,17 @@ config OLPC_OPENFIRMWARE
bool Support for OLPC's Open Firmware
depends on !X86_64 !X86_PAE
default y if OLPC
+ select OF
help
This option adds support for the implementation of Open Firmware
that is used on the OLPC XO-1 Children's Machine.
If unsure, say N here.
+config OLPC_OPENFIRMWARE_DT
+ bool
+ default y if OLPC_OPENFIRMWARE PROC_DEVICETREE
+ select OF_PROMTREE
+
endif # X86_32
config K8_NB
diff --git a/arch/x86/include/asm/olpc_ofw.h
b/arch/x86/include/asm/olpc_ofw.h
index 08fde47..a41250e 100644
--- a/arch/x86/include/asm/olpc_ofw.h
+++ b/arch/x86/include/asm/olpc_ofw.h
@@ -8,6 +8,8 @@
#ifdef CONFIG_OLPC_OPENFIRMWARE
+extern bool olpc_ofw_is_installed(void);
+
/* run an OFW command by calling into the firmware */
#define olpc_ofw(name, args, res) \
__olpc_ofw((name), ARRAY_SIZE(args), args, ARRAY_SIZE(res), res)
@@ -23,9 +25,16 @@ extern void setup_olpc_ofw_pgd(void);
#else /* !CONFIG_OLPC_OPENFIRMWARE */
+static inline bool olpc_ofw_is_installed(void) { return false; }
static inline void olpc_ofw_detect(void) { }
static inline void setup_olpc_ofw_pgd(void) { }
#endif /* !CONFIG_OLPC_OPENFIRMWARE */
+#ifdef CONFIG_OLPC_OPENFIRMWARE_DT
+extern void olpc_dt_build_devicetree(void);
+#else
+static inline void olpc_dt_build_devicetree(void) { }
+#endif /* CONFIG_OLPC_OPENFIRMWARE_DT */
+
#endif /* _ASM_X86_OLPC_OFW_H */
diff --git a/arch/x86/include/asm/prom.h b/arch/x86/include/asm/prom.h
new file mode 100644
index 000..b4ec95f
--- /dev/null
+++ b/arch/x86/include/asm/prom.h
@@ -0,0 +1 @@
+/* dummy prom.h; here to make linux/of.h's #includes happy */
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index fedf32a..519539a 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -107,6 +107,7 @@ scx200-y+= scx200_32.o
obj-$(CONFIG_OLPC) += olpc.o
obj-$(CONFIG_OLPC_OPENFIRMWARE)+= olpc_ofw.o
+obj-$(CONFIG_OLPC_OPENFIRMWARE_DT) += olpc_dt.o
olpc_ofw_dt.o perhaps?
obj-$(CONFIG_X86_MRST) += mrst.o
microcode-y:= microcode_core.o
diff --git a/arch/x86/kernel/olpc_dt.c b/arch/x86/kernel/olpc_dt.c
new file mode 100644
index 000..f660a11
--- /dev/null
+++ b/arch/x86/kernel/olpc_dt.c
@@ -0,0 +1,165 @@
+/*
+ * olpc_dt.c: OLPC-specific OFW device tree support code.
Nit: I personally prefer not to see the file name encoded in the
header block.
+ *
+ * Paul Mackerras August 1996.
+ * Copyright (C) 1996-2005 Paul Mackerras.
+ *
+ * Adapted for 64bit PowerPC by Dave Engebretsen and Peter Bergner.
+ *{engebret|bergn...@us.ibm.com
+ *
+ * Adapted for sparc by David S. Miller da...@davemloft.net
+ * Adapted for x86/OLPC by Andres Salomon dilin...@queued.net
+ *
+ * This program is free software; you can
Re: [PATCH] Globally s/struct irq_host/struct irq_domain/
On Mon, 4 Oct 2010, Grant Likely wrote: On Mon, Oct 4, 2010 at 5:09 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2010-10-04 at 10:52 -0600, Grant Likely wrote: On Sun, Oct 03, 2010 at 02:03:57PM -0500, Jon Loeliger wrote: Just rename the irq_host structure as irq_domain to lessen the confusion around the word host. Updated a few comments and error messages to use irq_domain when refering to the structure. Suggested by Ben Herrenschmidt on the device-tree mailing list. Signed-off-by: Jon Loeliger j...@jdl.com --- Grant requested one giant patch simply renaming the structure. Thanks Jon, However, considering that tglx has already embarked on his Grand IRQ Subsystem Cleanup, I suspect that this will end up just getting in the way unless he picks it up into his queue. It may even by that the need for irq_host will disappear once Thomas is finished by merging it into the core code. I'd like to sit tight on this until the details of what powerpc virqs can be worked out with Ben and Thomas. I doubt the need for irq_host will disappear. However I don't see that going into the core under that name, so I think the rename is a very valid thing to do now. We just need to sync the patches to avoid pure mechanical clash. Thomas initial series doesn't yet generalize the virq layer and that will take a bit longer, so I'm happy to see Jon stuff go upstream first. Alright, should tglx pick up this patch, or do you want to carry it in the powerpc tree? Should go through powerpc. tglx___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/8] posix clocks: introduce a syscall for clock tuning.
On Fri, 24 Sep 2010, Benjamin Herrenschmidt wrote: On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote: The new syscall, clock_adjtime, takes two parameters, the clock ID, and a pointer to a struct timex. The semantics of the timex struct have been expanded by one additional mode flag, which allows an absolute offset correction. When specificied, the clock offset is immediately corrected by adding the given time value to the current time value. Any reason why you CC'ed device-tree discuss ? This list is getting way too much unrelated stuff, which I find annoying, it would be nice if we were all a bit more careful here with our CC lists. Says the guy who missed to trim the useless context of the original mail, which made me scroll down all the way just to find out that there is nothing to see. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] x86: of: define irq functions to allow drivers/of/* to build on x86
On Fri, 10 Sep 2010, Grant Likely wrote: On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote: * Grant Likely grant.lik...@secretlab.ca wrote: On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote: - Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along. - Define irq_dispose_mapping for x86 to appease of_i2c.c Signed-off-by: Andres Salomon dilin...@queued.net Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch. The purpose of the patch is not clear to me. What does it do and why? It allows CONFIG_OF to be enabled on x86 without a build failure. The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take? It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does. Sounds good. We need this for other embedded x86 platforms as well and I was looking into the powerpc code before and wondered how we can make that generic. The question is whether this should be tied into drivers/of/irq or just provided as an OF independent generic facility in kernel/irq. Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] x86: of: define irq functions to allow drivers/of/* to build on x86
On Mon, 20 Sep 2010, Andres Salomon wrote: On Fri, 10 Sep 2010 12:21:35 -0600 Grant Likely grant.lik...@secretlab.ca wrote: On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote: * Grant Likely grant.lik...@secretlab.ca wrote: On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote: - Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along. - Define irq_dispose_mapping for x86 to appease of_i2c.c Signed-off-by: Andres Salomon dilin...@queued.net Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch. The purpose of the patch is not clear to me. What does it do and why? It allows CONFIG_OF to be enabled on x86 without a build failure. The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take? It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does. g. Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer. Fine with me, but you could make the mapping function an inline as well. While at it, some sensible changelog would be helpful :) Thanks, tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] x86: of: define irq functions to allow drivers/of/* to build on x86
On Tue, 21 Sep 2010, Andres Salomon wrote: On Tue, 21 Sep 2010 11:45:37 +0200 (CEST) Thomas Gleixner t...@linutronix.de wrote: Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer. Fine with me, but you could make the mapping function an inline as well. While at it, some sensible changelog would be helpful :) Thanks, tglx Hm, inlining it would require some additional x86 #ifdefs in include/linux/of_irq.h (where the mapping function is declared). You'd prefer that? Urgh, no. tglx ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Regarding hw irq to Linux irq mapping on ARM
On Tue, 21 Sep 2010, Grant Likely wrote: On Tue, Sep 21, 2010 at 7:25 AM, Shaju Abraham shaju.abra...@linaro.org wrote: Hi Grant Since there does not exist a mechanism to map the hw irq to linux irq on ARM (device tree), I would like to discuss with you the plans or ideas to implement the same. I don't have any immediate plans, but this topic has come up a lot in the last two weeks, so I guess I need to focus on it. :-) [cc'ing devicetree-discuss and linux-arm-kernel as well as Lorenzo and Eric since this is a conversation that should be had publically] Can you share with me your thoughts on it? I have browsed through the power pc code for the same. But not sure the same approach is usable on ARM as well. I haven't thought deeply about the powerpc implementation of virqs to determine if it is suitable for other architectures or not, but the concept behind it is sound. We need a method of mapping controller specific IRQ (or hw irq) numbers into the global Linux irq space (referred to a virqs from this point on). First it requires a per-controller reference which can be a pointer to a per-controller data structure, or any other unique identifier. It could even be the interrupt controller device tree node pointer. Just so long as there is a reliable method to derive the virq from the controller reference + hw irq number. There also needs to be a method for each interrupt controller to register itself and allocate a portion of the virq range. This shouldn't be too hard. PowerPC handles this with the irq_map[] flat table. This approach is limited to whatever NR_IRQs is set to, and could potentially be limited by that, but on the other hand the number of discrete IRQ sources in a system is limited so a flat table (instead of a dynamic hash table) is probably sufficient. It is certainly simpler to implement. I think the first step is to simply try generalizing the code in arch/powerpc/kernel/irq.c. It isn't very complex and it would give a better impression of what needs to be done. The ARM interrupt controller drivers would need to be modified to register with the virq infrastructure. None of this is either ARM or OF specific; it would be useful for any system than need to dynamically allocate IRQ numbers. I could see some x86 use cases (Xilinx FPGAs) where this would be useful. Add all the I2C, SPI based irq extenders to that list. They seem to pop up all over the place in rapid speed even in x86. We are happy citizens of the embedded horror^Wuniverse now. Thanks, tglx___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
