public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Petr Cvek <petrcvekcz@gmail.com>
To: hdegoede@redhat.com, tglx@linutronix.de, marc.zyngier@arm.com,
	tglx@linutronix.de
Cc: linux-kernel@vger.kernel.org, philipp.zabel@gmail.com
Subject: regression in irq sharing caused by genirq: Use irqd_get_trigger_type to compare the trigger type for shared IRQs
Date: Wed, 8 Nov 2017 00:41:13 +0100	[thread overview]
Message-ID: <1415e4e2-3ee9-bc31-374c-ba64a506a622@gmail.com> (raw)

Hello,

Commit 382bd4de61827 ("genirq: Use irqd_get_trigger_type to compare the 
trigger type for shared IRQs") causes a regression for pda-power driver 
and Magician machine (mach-pxa/magician.c).

   unsigned int oldtype = irqd_get_trigger_type(&desc->irq_data);

   ... assert:
	oldtype == 0	//new code
	old->flags == 0x83	//old code
	new->flags & IRQF_TRIGGER_MASK == 3

   if (!((old->flags & new->flags) & IRQF_SHARED) ||
	(oldtype != (new->flags & IRQF_TRIGGER_MASK)) ||
	((old->flags ^ new->flags) & IRQF_ONESHOT))
		goto mismatch;

The assert shows the new code will trigger the jump for "mismatch" error 
the old variant of code works fine.

The case for Magician machine is specific as the same interrupt line is 
requested twice from the same driver (pda-power). But it still could 
point to some hidden problem in the IRQ setup code.

I wasn't able to trace the code when desc->irq_data is getting set. The 
flags values should be (as with old->flags):

	IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING

It could be a problem with a weird IRQ sharing in magician code, but 
it's still failing the driver and the start of the charging system.

IRQ definition in arch/arm/mach-pxa/magician.c looks like this:

static struct resource power_supply_resources[] = {
	[0] = {
		.name	= "ac",
		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE |
			IORESOURCE_IRQ_LOWEDGE,
		.start	= IRQ_MAGICIAN_VBUS,
		.end	= IRQ_MAGICIAN_VBUS,
	},
	[1] = {
		.name	= "usb",
		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE |
			IORESOURCE_IRQ_LOWEDGE,
		.start	= IRQ_MAGICIAN_VBUS,
		.end	= IRQ_MAGICIAN_VBUS,
	},
};

and IRQ requests from drivers/power/supply/pda_power.c look like this:

if (ac_irq) {
	ret = request_irq(ac_irq->start, power_changed_isr,
			  get_irq_flags(ac_irq), ac_irq->name,
			  pda_psy_ac);
...
if (usb_irq) {
	ret = request_irq(usb_irq->start, power_changed_isr,
			  get_irq_flags(usb_irq),
			  usb_irq->name, pda_psy_usb);

I could rewrite the part in the magician code so it would use only one 
interrupt, but it doesn't solve why oldtype == 0 problem.

Best regards,
Petr

             reply	other threads:[~2017-11-07 23:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-07 23:41 Petr Cvek [this message]
2017-11-08 13:09 ` regression in irq sharing caused by genirq: Use irqd_get_trigger_type to compare the trigger type for shared IRQs Marc Zyngier
2017-11-08 13:11   ` Marc Zyngier
2017-11-08 13:35     ` Petr Cvek
2017-11-08 13:55       ` Marc Zyngier
2017-11-09 14:50       ` Marc Zyngier
2017-11-09 16:47         ` Petr Cvek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1415e4e2-3ee9-bc31-374c-ba64a506a622@gmail.com \
    --to=petrcvekcz@gmail.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=philipp.zabel@gmail.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox