All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javierm@redhat.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Peter Robinson <pbrobinson@gmail.com>,
	Shawn Lin <shawn.lin@rock-chips.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Rob Herring <robh@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Michal Simek <michal.simek@xilinx.com>,
	Jingoo Han <jingoohan1@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH v2] PCI: rockchip: Avoid accessing PCIe registers with clocks gated
Date: Thu, 1 Jul 2021 16:59:35 +0200	[thread overview]
Message-ID: <747df123-0aa0-7cb7-9fb3-f13c849894e5@redhat.com> (raw)
In-Reply-To: <20210701135949.GA51123@bjorn-Precision-5520>

On 7/1/21 3:59 PM, Bjorn Helgaas wrote:

[snip]

>> The IRQ handler is not called when unregistered, but it is called
>> when another handler for the shared IRQ is unregistered. In this
>> particular driver, both a "pcie-sys" and "pcie-client" handlers are
>> registered, then an error leads to "pcie-sys" being unregistered and
>> the handler for "pcie-client" being called.
> 
> Is this really true?  I think that would mean CONFIG_DEBUG_SHIRQ would
> not find this kind of bug unless we actually registered two or more
> handlers for the shared IRQ, but it's still a bug even only one
> handler is registered.
> 
> Looking at __free_irq() [1], my impression is that "action" is what
> we're removing and action->handler() is the IRQ handler we call when
> CONFIG_DEBUG_SHIRQ, so it doesn't look like it's calling the remaining
> handlers after removing one of them.
> 

Oh, you are completely right. I wrongly assumed that it was for the other
registered IRQ handlers but reading the source is clearly how you say it.

I now wonder why when debugging this I saw that the "pcie-client" handler
was called when "pcie-sys" was unregistered...

But anyways, you are correct and I'm OK with the text you shared.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer
New Platform Technologies Enablement team
RHEL Engineering


WARNING: multiple messages have this Message-ID (diff)
From: Javier Martinez Canillas <javierm@redhat.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Peter Robinson <pbrobinson@gmail.com>,
	Shawn Lin <shawn.lin@rock-chips.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Rob Herring <robh@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Michal Simek <michal.simek@xilinx.com>,
	Jingoo Han <jingoohan1@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH v2] PCI: rockchip: Avoid accessing PCIe registers with clocks gated
Date: Thu, 1 Jul 2021 16:59:35 +0200	[thread overview]
Message-ID: <747df123-0aa0-7cb7-9fb3-f13c849894e5@redhat.com> (raw)
In-Reply-To: <20210701135949.GA51123@bjorn-Precision-5520>

On 7/1/21 3:59 PM, Bjorn Helgaas wrote:

[snip]

>> The IRQ handler is not called when unregistered, but it is called
>> when another handler for the shared IRQ is unregistered. In this
>> particular driver, both a "pcie-sys" and "pcie-client" handlers are
>> registered, then an error leads to "pcie-sys" being unregistered and
>> the handler for "pcie-client" being called.
> 
> Is this really true?  I think that would mean CONFIG_DEBUG_SHIRQ would
> not find this kind of bug unless we actually registered two or more
> handlers for the shared IRQ, but it's still a bug even only one
> handler is registered.
> 
> Looking at __free_irq() [1], my impression is that "action" is what
> we're removing and action->handler() is the IRQ handler we call when
> CONFIG_DEBUG_SHIRQ, so it doesn't look like it's calling the remaining
> handlers after removing one of them.
> 

Oh, you are completely right. I wrongly assumed that it was for the other
registered IRQ handlers but reading the source is clearly how you say it.

I now wonder why when debugging this I saw that the "pcie-client" handler
was called when "pcie-sys" was unregistered...

But anyways, you are correct and I'm OK with the text you shared.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer
New Platform Technologies Enablement team
RHEL Engineering


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Javier Martinez Canillas <javierm@redhat.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Peter Robinson <pbrobinson@gmail.com>,
	Shawn Lin <shawn.lin@rock-chips.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Rob Herring <robh@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Michal Simek <michal.simek@xilinx.com>,
	Jingoo Han <jingoohan1@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH v2] PCI: rockchip: Avoid accessing PCIe registers with clocks gated
Date: Thu, 1 Jul 2021 16:59:35 +0200	[thread overview]
Message-ID: <747df123-0aa0-7cb7-9fb3-f13c849894e5@redhat.com> (raw)
In-Reply-To: <20210701135949.GA51123@bjorn-Precision-5520>

On 7/1/21 3:59 PM, Bjorn Helgaas wrote:

[snip]

>> The IRQ handler is not called when unregistered, but it is called
>> when another handler for the shared IRQ is unregistered. In this
>> particular driver, both a "pcie-sys" and "pcie-client" handlers are
>> registered, then an error leads to "pcie-sys" being unregistered and
>> the handler for "pcie-client" being called.
> 
> Is this really true?  I think that would mean CONFIG_DEBUG_SHIRQ would
> not find this kind of bug unless we actually registered two or more
> handlers for the shared IRQ, but it's still a bug even only one
> handler is registered.
> 
> Looking at __free_irq() [1], my impression is that "action" is what
> we're removing and action->handler() is the IRQ handler we call when
> CONFIG_DEBUG_SHIRQ, so it doesn't look like it's calling the remaining
> handlers after removing one of them.
> 

Oh, you are completely right. I wrongly assumed that it was for the other
registered IRQ handlers but reading the source is clearly how you say it.

I now wonder why when debugging this I saw that the "pcie-client" handler
was called when "pcie-sys" was unregistered...

But anyways, you are correct and I'm OK with the text you shared.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer
New Platform Technologies Enablement team
RHEL Engineering


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-07-01 14:59 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08  8:04 [PATCH v2] PCI: rockchip: Avoid accessing PCIe registers with clocks gated Javier Martinez Canillas
2021-06-08  8:04 ` Javier Martinez Canillas
2021-06-08  8:04 ` Javier Martinez Canillas
2021-06-12 22:02 ` Peter Robinson
2021-06-12 22:02   ` Peter Robinson
2021-06-12 22:02   ` Peter Robinson
2021-06-22 10:31 ` Lorenzo Pieralisi
2021-06-22 10:31   ` Lorenzo Pieralisi
2021-06-22 10:31   ` Lorenzo Pieralisi
2021-06-24 21:57 ` Bjorn Helgaas
2021-06-24 21:57   ` Bjorn Helgaas
2021-06-24 21:57   ` Bjorn Helgaas
2021-06-24 23:18   ` Robin Murphy
2021-06-24 23:18     ` Robin Murphy
2021-06-24 23:18     ` Robin Murphy
2021-06-24 23:28     ` Bjorn Helgaas
2021-06-24 23:28       ` Bjorn Helgaas
2021-06-24 23:28       ` Bjorn Helgaas
2021-06-24 23:51       ` Robin Murphy
2021-06-24 23:51         ` Robin Murphy
2021-06-24 23:51         ` Robin Murphy
2021-06-24 22:40 ` Bjorn Helgaas
2021-06-24 22:40   ` Bjorn Helgaas
2021-06-24 22:40   ` Bjorn Helgaas
2021-06-25  7:09   ` Javier Martinez Canillas
2021-06-25  7:09     ` Javier Martinez Canillas
2021-06-25  7:09     ` Javier Martinez Canillas
2021-06-25 14:32     ` Bjorn Helgaas
2021-06-25 14:32       ` Bjorn Helgaas
2021-06-25 14:32       ` Bjorn Helgaas
2021-06-25 18:34       ` Javier Martinez Canillas
2021-06-25 18:34         ` Javier Martinez Canillas
2021-06-25 18:34         ` Javier Martinez Canillas
2021-06-29  0:38   ` Bjorn Helgaas
2021-06-29  0:38     ` Bjorn Helgaas
2021-06-29  0:38     ` Bjorn Helgaas
2021-06-29  6:17     ` Javier Martinez Canillas
2021-06-29  6:17       ` Javier Martinez Canillas
2021-06-29  6:17       ` Javier Martinez Canillas
2021-06-29 10:52       ` Robin Murphy
2021-06-29 10:52         ` Robin Murphy
2021-06-29 10:52         ` Robin Murphy
2021-06-29 23:14         ` Bjorn Helgaas
2021-06-29 23:14           ` Bjorn Helgaas
2021-06-29 23:14           ` Bjorn Helgaas
2021-06-30  9:44           ` Robin Murphy
2021-06-30  9:44             ` Robin Murphy
2021-06-30  9:44             ` Robin Murphy
2021-06-30 18:49         ` Bjorn Helgaas
2021-06-30 18:49           ` Bjorn Helgaas
2021-06-30 18:49           ` Bjorn Helgaas
2021-06-30 18:59 ` Bjorn Helgaas
2021-06-30 18:59   ` Bjorn Helgaas
2021-06-30 18:59   ` Bjorn Helgaas
2021-06-30 19:59   ` Javier Martinez Canillas
2021-06-30 19:59     ` Javier Martinez Canillas
2021-06-30 19:59     ` Javier Martinez Canillas
2021-06-30 20:30     ` Bjorn Helgaas
2021-06-30 20:30       ` Bjorn Helgaas
2021-06-30 20:30       ` Bjorn Helgaas
2021-06-30 20:46       ` Peter Robinson
2021-06-30 20:46         ` Peter Robinson
2021-06-30 20:46         ` Peter Robinson
2021-06-30 22:09       ` Javier Martinez Canillas
2021-06-30 22:09         ` Javier Martinez Canillas
2021-06-30 22:09         ` Javier Martinez Canillas
2021-07-01 13:59         ` Bjorn Helgaas
2021-07-01 13:59           ` Bjorn Helgaas
2021-07-01 13:59           ` Bjorn Helgaas
2021-07-01 14:59           ` Javier Martinez Canillas [this message]
2021-07-01 14:59             ` Javier Martinez Canillas
2021-07-01 14:59             ` Javier Martinez Canillas

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=747df123-0aa0-7cb7-9fb3-f13c849894e5@redhat.com \
    --to=javierm@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=heiko@sntech.de \
    --cc=helgaas@kernel.org \
    --cc=jingoohan1@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=michal.simek@xilinx.com \
    --cc=pbrobinson@gmail.com \
    --cc=robh@kernel.org \
    --cc=shawn.lin@rock-chips.com \
    --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 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.