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
WARNING: multiple messages have this Message-ID (diff)
From: briannorris@chromium.org (Brian Norris)
To: linux-arm-kernel@lists.infradead.org
Subject: [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
next prev parent reply other threads:[~2017-06-30 1:29 UTC|newest]
Thread overview: 8+ 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-23 13:03 ` Jeffy Chen
2017-06-29 22:05 ` Brian Norris
2017-06-29 22:05 ` Brian Norris
2017-06-30 1:24 ` jeffy
2017-06-30 1:24 ` jeffy
2017-06-30 1:29 ` Brian Norris [this message]
2017-06-30 1:29 ` Brian Norris
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.