Re: [PATCH] genirq: fix virtual irq demuxer related comments
On Fri, Feb 20, 2015 at 04:12:59PM +, Mark Rutland wrote: > Hi Peter, > > On Tue, Feb 10, 2015 at 04:14:25PM +0000, Peter Zijlstra wrote: > > On Tue, Feb 10, 2015 at 04:43:12PM +0100, Boris Brezillon wrote: > > > Replace remaining 'Dumb' occurrences by 'Virtual'. > > > Remove inappropriate notes in kerneldoc headers. > > > > > > Signed-off-by: Boris Brezillon > > > > Thanks, squished into the other one. > > Would you be able to drop the virtual demuxer patch(es) for now? Yeah, its commented out. With a comment saying I should track this thread ;-) -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] irqchip: Add DT binding doc for the virtual irq demuxer chip
Please change the Subject to start with [PATCH] again when including patches, otherwise its too easy for them to get lost. Esp. with excessive quoting on top. I nearly missed the patch here, seeing nothing in the first page of text. On Wed, Feb 11, 2015 at 05:13:13PM +, Mark Rutland wrote: > --- > include/linux/interrupt.h | 5 + > kernel/irq/pm.c | 44 ++-- > 2 files changed, 47 insertions(+), 2 deletions(-) > > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index d9b05b5..2b8ff50 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -57,6 +57,9 @@ > * IRQF_NO_THREAD - Interrupt cannot be threaded > * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device > *resume time. > + * IRQF_SHARED_TIMER_OK - Interrupt is safe to be shared with a timer. The > + *handler may be called spuriously during suspend > + *without issue. I feel we should do better documenting this; at the very least refer to Documentation/power/suspend-and-interrupts.txt and ideally put a scary note in telling people that if they use this as a bandaid to make the warn go away, they will end up with a broken system. Now ideally every driver that employs this should also have a comment next to it how it does indeed behave nicely, such that we can 'quickly' see people have indeed thought about things and not just slapped it on to make the WARN go away. > diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c > index 3ca5325..e4ec91a 100644 > --- a/kernel/irq/pm.c > +++ b/kernel/irq/pm.c > @@ -28,6 +28,47 @@ bool irq_pm_check_wakeup(struct irq_desc *desc) > + for (action = desc->action; action; action = action->next) > + if (!(action->flags & safe_flags)) > + return false; In general I prefer braces around the for loop even though C does not strictly require it. Its just too easy to confuse multi-line statements with multiple statements. Extra braces comfort the brain in this case. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] irqchip: Add DT binding doc for the virtual irq demuxer chip
Guys, trim your emails, please! On Wed, Feb 11, 2015 at 04:51:36PM +0100, Rafael J. Wysocki wrote: > On Wednesday, February 11, 2015 03:12:38 PM Mark Rutland wrote: > > I guess that would have to imply IRQF_SHARED, so we'd have something > > like: > > > > IRQF_SHARED_SUSPEND_OK - This handler is safe to call spuriously during > > suspend in the case the line is shared. The > > handler will not access unavailable hardware > > or kernel infrastructure during this period. > > > > #define __IRQF_SUSPEND_SPURIOUS 0x0004 > > #define IRQF_SHARED_SUSPEND_OK (IRQF_SHARED | > > __IRQF_SUSPEND_SPURIOUS) > > What about > > #define __IRQF_TIMER_SIBLING_OK 0x0004 > #define IRQF_SHARED_TIMER_OK(IRQF_SHARED | __IRQF_TIMER_SIBLING_OK) > > The "suspend" part is kind of a distraction to me here, because that really > only is about sharing an IRQ with a timer and the "your interrupt handler > may be called when the device is suspended" part is just a consequence of > that. > > So IMO it's better to have "TIMER" in the names to avoid encouraging people to > abuse this for other purposes not related to timers. Sorry to be late to the bike-shed party, but what about: Documentation/power/suspend-and-interrupts.txt:the IRQ's users. For this reason, using IRQF_NO_SUSPEND and IRQF_SHARED at the arch/arm/mach-omap2/mux.c: omap_hwmod_mux_handle_irq, IRQF_SHARED | IRQF_NO_SUSPEND, arch/arm/mach-omap2/pm34xx.c: _prcm_int_handle_io, IRQF_SHARED | IRQF_NO_SUSPEND, "pm_io", drivers/mfd/ab8500-debugfs.c: IRQF_SHARED | IRQF_NO_SUSPEND, drivers/mfd/ab8500-gpadc.c: IRQF_NO_SUSPEND | IRQF_SHARED, "ab8500-gpadc-sw", drivers/mfd/ab8500-gpadc.c: IRQF_NO_SUSPEND | IRQF_SHARED, "ab8500-gpadc-hw", drivers/pinctrl/pinctrl-single.c: IRQF_SHARED | IRQF_NO_SUSPEND, drivers/power/ab8500_btemp.c: IRQF_SHARED | IRQF_NO_SUSPEND, drivers/power/ab8500_charger.c: IRQF_SHARED | IRQF_NO_SUSPEND, drivers/power/ab8500_fg.c: IRQF_SHARED | IRQF_NO_SUSPEND, drivers/rtc/rtc-pl031.c:.irqflags = IRQF_SHARED | IRQF_NO_SUSPEND, drivers/usb/phy/phy-ab8500-usb.c: IRQF_NO_SUSPEND | IRQF_SHARED, drivers/usb/phy/phy-ab8500-usb.c: IRQF_NO_SUSPEND | IRQF_SHARED, drivers/usb/phy/phy-ab8500-usb.c: IRQF_NO_SUSPEND | IRQF_SHARED, drivers/watchdog/intel-mid_wdt.c: IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog", Is there a single legitimate user in that list? If so, the TIMER name might be misleading. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] irqchip: Add DT binding doc for the virtual irq demuxer chip
On Tue, Feb 10, 2015 at 08:48:36PM +, Mark Rutland wrote: > From f390ccbb31f06efee49b4469943c8d85d963bfb5 Mon Sep 17 00:00:00 2001 > From: Mark Rutland > Date: Tue, 10 Feb 2015 20:14:33 + > Subject: [PATCH] genirq: allow mixed IRQF_NO_SUSPEND requests > > In some cases a physical IRQ line may be shared between devices from > which we expect interrupts during suspend (e.g. timers) and those we do > not (e.g. anything we cut the power to). Where a driver did not request > the interrupt with IRQF_NO_SUSPEND, it's unlikely that it can handle > being called during suspend, and it may bring down the system. > > This patch adds logic to automatically mark the irqactions for these > potentially unsafe handlers as disabled during suspend, leaving actions > with IRQF_NO_SUSPEND enabled. If an interrupt is raised on a shared line > during suspend, only the handlers requested with IRQF_NO_SUSPEND will be > called. The handlers requested without IRQF_NO_SUSPEND will be skipped > as if they had immediately returned IRQF_NONE. > > Cc: Boris Brezillon > Cc: Jason Cooper > Cc: Nicolas Ferre > Cc: Peter Zijlstra > Cc: Rafael J. Wysocki > Cc: Thomas Gleixner > Signed-off-by: Mark Rutland Aw gawd.. not that again. So Rafael and tglx went over this a few months ago I think: lkml.kernel.org/r/26580319.ozp7jvj...@vostro.rjw.lan is the last series I could find. Maybe Rafael can summarize? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] genirq: fix virtual irq demuxer related comments
On Tue, Feb 10, 2015 at 04:43:12PM +0100, Boris Brezillon wrote: > Replace remaining 'Dumb' occurrences by 'Virtual'. > Remove inappropriate notes in kerneldoc headers. > > Signed-off-by: Boris Brezillon Thanks, squished into the other one. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/5] irqchip: add virtual demultiplexer implementation
On Thu, Jan 29, 2015 at 11:33:37AM +0100, Boris Brezillon wrote: > +#ifdef CONFIG_VIRT_IRQ_DEMUX_CHIP > +/** > + * struct irq_chip_virt_demux - Dumb demultiplexer irq chip data structure s/Dumb/Virtual/ ? > + * @domain: irq domain pointer > + * @available: Bitfield of valid irqs > + * @unmasked:Bitfield containing irqs status > + * @flags: irq_virt_demux_flags flags > + * @src_irq: irq feeding the virt demux chip > + * > + * Note, that irq_chip_generic can have multiple irq_chip_type > + * implementations which can be associated to a particular irq line of > + * an irq_chip_generic instance. That allows to share and protect > + * state in an irq_chip_generic instance when we need to implement > + * different flow mechanisms (level/edge) for it. This seems like a copy/paste from struct irq_chip_generic; it seems not relevant, seeing how irq_chip_virt_demux does not even have an irq_chip_type pointer list. Also, with a demuxer like this, we're bound to whatever flow type its host is, no? > +# Dumb interrupt demuxer chip implementation s/Dumb/Virtual/ ? > +#ifdef CONFIG_VIRT_IRQ_DEMUX_CHIP > +/** > + * handle_virt_demux_irq - Dumb demuxer irq handle function. idem > + * @irq: the interrupt number > + * @desc: the interrupt description structure for this irq > + * > + * Dumb demux interrupts are sent from a demultiplexing interrupt handler idem > + * which is not able to decide which child interrupt handler should be > + * called. > + * > + * Note: The caller is expected to handle the ack, clear, mask and > + * unmask issues if necessary. > + */ If we're calling multiple handlers, how can they all do this and not collide? Over all it looks good -- in as far as my (admittedly stale IRQ braincells) go. I'll go queue up these bits, if you could send me a delta patch addressing these 'minor' comment issues? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] ARC: Add perf support for ARC700 cores
On Mon, Nov 11, 2013 at 05:27:55PM +0100, Peter Zijlstra wrote: > On Thu, Nov 07, 2013 at 02:55:10PM +0100, Mischa Jonker wrote: > > Hi all, > > > > Please find v3 of my attempt to add support for perf for ARC700 PMU's. If > > possible, it would be nice to get some feedback on the implementation from > > non-ARC/Synopsys people familiar with the perf subsystem in Linux. > > Anything in particular you wanted feedback on? It looks like a fairly > straight fwd thingy. Ah, so its one of those that cannot sample; you might want to fail when we try and create a sampling event. Also, you could consider running an (hr)timer to periodically update the events so that you don't miss a hardware counter wrap around -- if that is a distinct possibility. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] ARC: Add perf support for ARC700 cores
On Thu, Nov 07, 2013 at 02:55:10PM +0100, Mischa Jonker wrote: > Hi all, > > Please find v3 of my attempt to add support for perf for ARC700 PMU's. If > possible, it would be nice to get some feedback on the implementation from > non-ARC/Synopsys people familiar with the perf subsystem in Linux. Anything in particular you wanted feedback on? It looks like a fairly straight fwd thingy. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] ARC: Add perf support for ARC700 cores
On Thu, Nov 07, 2013 at 02:55:11PM +0100, Mischa Jonker wrote: > This adds basic perf support for ARC700 cores. Most PERF_COUNT_HW* events > are supported now. I don't see anything particularly weird, but then I don't know/have the hardware so I suppose you know what you're doing :-) Acked-by: Peter Zijlstra -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
