From: Brian Norris <briannorris@chromium.org>
To: jeffy <jeffy.chen@rock-chips.com>
Cc: linux-kernel@vger.kernel.org, tfiga@chromium.org,
heiko@sntech.de, dianders@chromium.org,
linux-rockchip@lists.infradead.org,
Linus Walleij <linus.walleij@linaro.org>,
linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH] pinctrl: rockchip: add irq_shutdown
Date: Thu, 29 Jun 2017 18:29:31 -0700 [thread overview]
Message-ID: <20170630012930.GA99272@google.com> (raw)
In-Reply-To: <5955A866.20308@rock-chips.com>
On Fri, Jun 30, 2017 at 09:24:54AM +0800, Jeffy Chen wrote:
> On 06/30/2017 06:05 AM, Brian Norris wrote:
> >On Fri, Jun 23, 2017 at 09:03:39PM +0800, Jeffy Chen wrote:
> >>Currently the rockchip pinctrl driver would try to enable/disable the
> >>gpio bank clk when enable/disable an irq.
> >>
> >>So when the irq core trying to shutdown an already disabled irq, it
> >>would result in unbalanced clk disable request:
> >>[ 35.911955] WARNING: at drivers/clk/clk.c:680
> >>...
> >>[ 37.272271] Call trace:
> >>[ 37.274729] [<ffffffc0007ac270>] clk_core_disable+0x28/0x194
> >>[ 37.280395] [<ffffffc0007ac6a8>] clk_disable+0x34/0x48
> >>[ 37.285544] [<ffffffc0004f3bf0>] rockchip_irq_disable+0x30/0x3c
> >>[ 37.291472] [<ffffffc00027a7e0>] __irq_disable+0x40/0x64
> >>[ 37.296791] [<ffffffc00027a86c>] irq_shutdown+0x68/0x8c
> >>[ 37.302023] [<ffffffc0002777fc>] __free_irq+0x110/0x218
> >>[ 37.307254] [<ffffffc0002779a8>] free_irq+0x54/0x64
> >>[ 37.312138] [<ffffffc00027ba24>] devm_irq_release+0x24/0x30
> >>
> >>Add an irq_shutdown callback, and do a sanity check for irq state to
> >>prevent that.
> >
> >IMO, this patch is completely the wrong approach. Either we:
> >
> >(a) follow the current semantics of the irqchip core handling (which
> >tglx described more or less as "no refcounting; no guarantee that
> >enable/disable are balanced") or
> >
> >(b) fix the irqchip core to provide the above guarantee
> right, since we have:
> bf22ff45 genirq: Avoid unnecessary low level irq function calls
> d829b8f genirq: Set irq masked state when initializing irq_desc
>
> this issue should be fixed now.
Oh, I didn't notice Thomas applied those. I'll have to take a look.
Brian
prev parent reply other threads:[~2017-06-30 1:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-23 13:03 [RFC PATCH] pinctrl: rockchip: add irq_shutdown Jeffy Chen
2017-06-29 22:05 ` Brian Norris
2017-06-30 1:24 ` jeffy
2017-06-30 1:29 ` Brian Norris [this message]
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=20170630012930.GA99272@google.com \
--to=briannorris@chromium.org \
--cc=dianders@chromium.org \
--cc=heiko@sntech.de \
--cc=jeffy.chen@rock-chips.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=tfiga@chromium.org \
/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).