linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Brian Masney <masneyb@onstation.org>
Cc: Stephen Boyd <sboyd@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Andy Gross <andy.gross@linaro.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Lee Jones <lee.jones@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Doug Anderson <dianders@chromium.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Nicolas Dechesne <nicolas.dechesne@linaro.org>,
	Niklas Cassel <niklas.cassel@linaro.org>,
	David Brown <david.brown@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	linux-arm-msm@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH 3/9] mfd: pm8xxx: convert to v2 irq interfaces to support hierarchical IRQ chips
Date: Wed, 6 Feb 2019 11:02:08 +0100	[thread overview]
Message-ID: <CACRpkdYuH6QnumRnAUbzJCwrj58mog68FJu6Tvi_48bLzsBbZg@mail.gmail.com> (raw)
In-Reply-To: <20190125162302.14036-4-masneyb@onstation.org>

Hi Brian!

Thanks for this patch!

On Fri, Jan 25, 2019 at 5:23 PM Brian Masney <masneyb@onstation.org> wrote:

> The IRQ handler was hardcoded as handle_level_irq and this patch
> properly sets the handler to either handle_edge_irq or handle_level_irq
> depending on the IRQ type.

Done like so:

> +               flow_handler = handle_edge_irq;

This creates a problem and makes my kernel crash, because
handle_edge_irq() *unconditionally* calls chip->irq_ack(),
and since this chip does not implement .irq_ack(), it crashes.

I tried to just add a void .irq_ack(). That instead made the
kernel hang.

If I just assign handle_level_irq() as before (just replaced
all assignments of handle_edge_irq with handle_level_irq)
it boots nicely, AND the interrupts are working fine on the
ADC calibration path, and further readings of the ADC
generates new IRQs, as expected!

The handle_edge_irq and .irq_ack() call path is for hardware
that require an explicit ACK (such as writing a bit in some
register) when handling edge IRQs. This is necessary in some
hardware because of the transient nature of edge IRQs.
This is not necessary on some hardware that will simply
clear the flag when something is reading the status register.
I think this is the case with PM8xxx and you can safely use
handle_level_irq with all IRQs. It looks counterintuitive and
maybe the function handle_edge_irq() has a stupid name.
Just drop this handle_edge_irq() business for now.

This was tested on the APQ8060 DragonBoard.

However this happens:

[    3.007727] mmci-pl18x 12180000.sdcc: ignoring dependency for
device, assuming no driver
[    3.007970] mmci-pl18x 12200000.sdcc: ignoring dependency for
device, assuming no driver
[    3.014984] mpu3050-i2c 1-0068: ignoring dependency for device,
assuming no driver
[    3.023543] cm3605 cm3605: ignoring dependency for device, assuming no driver
[    3.030532] ak8975 1-000c: ignoring dependency for device, assuming no driver
[    3.037729] bmp280 1-0077: ignoring dependency for device, assuming no driver
[    3.044789] reg-fixed-voltage regulators:xc622a331mrg: ignoring
dependency for device, assuming no driver
[    3.052485] smsc911x 1b800000.ethernet-ebi2: ignoring dependency
for device, assuming no driver

But I think that is because these all use ssbi-gpio interrupts and
these are now handled properly so I will try to fix the device
tree and send a separate patch that you can include.

Yours,
Linus Walleij

  parent reply	other threads:[~2019-02-06 10:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-25 16:22 [PATCH 0/9] qcom: ssbi-gpio: add support for hierarchical IRQ chip Brian Masney
2019-01-25 16:22 ` [PATCH 1/9] pinctrl: qcom: ssbi-gpio: hardcode IRQ counts Brian Masney
2019-02-06  9:38   ` Linus Walleij
2019-02-06 12:01   ` Linus Walleij
2019-01-25 16:22 ` [PATCH 2/9] genirq: introduce irq_domain_translate_twocell Brian Masney
2019-01-30 13:54   ` Marc Zyngier
2019-01-25 16:22 ` [PATCH 3/9] mfd: pm8xxx: convert to v2 irq interfaces to support hierarchical IRQ chips Brian Masney
2019-01-30 13:27   ` Lee Jones
2019-02-04 10:20     ` Brian Masney
2019-02-06 10:02   ` Linus Walleij [this message]
2019-02-06 13:07   ` Linus Walleij
2019-02-06 14:10     ` Brian Masney
2019-02-06 14:37       ` Linus Walleij
2019-01-25 16:22 ` [PATCH 4/9] mfd: pm8xxx: disassociate old virq if hwirq mapping already exists Brian Masney
2019-01-30 13:31   ` Lee Jones
2019-01-25 16:22 ` [PATCH 5/9] qcom: ssbi-gpio: add support for hierarchical IRQ chip Brian Masney
2019-01-25 16:22 ` [PATCH 6/9] arm: dts: qcom: apq8064: add interrupt controller properties Brian Masney
2019-01-25 16:23 ` [PATCH 7/9] arm: dts: qcom: msm8660: " Brian Masney
2019-01-25 16:23 ` [PATCH 8/9] arm: dts: qcom: mdm9615: " Brian Masney
2019-01-25 16:23 ` [PATCH 9/9] mfd: pm8xxx: revert "disassociate old virq if hwirq mapping already exists" Brian Masney
2019-02-06 15:21 ` [PATCH 0/9] qcom: ssbi-gpio: add support for hierarchical IRQ chip Linus Walleij

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=CACRpkdYuH6QnumRnAUbzJCwrj58mog68FJu6Tvi_48bLzsBbZg@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=masneyb@onstation.org \
    --cc=nicolas.dechesne@linaro.org \
    --cc=niklas.cassel@linaro.org \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=thierry.reding@gmail.com \
    /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;
as well as URLs for NNTP newsgroup(s).