From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: ryder.lee@mediatek.com, jianjun.wang@mediatek.com,
lpieralisi@kernel.org, kw@linux.com, robh@kernel.org,
bhelgaas@google.com, p.zabel@pengutronix.de,
matthias.bgg@gmail.com, linux-pci@vger.kernel.org,
linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] PCI: mediatek-gen3: Stop acquiring spinlocks in {suspend,resume}_noirq
Date: Fri, 9 Jun 2023 09:29:36 +0200 [thread overview]
Message-ID: <a5597b19-eca8-10b0-1fe4-8f6e1d1d1adf@collabora.com> (raw)
In-Reply-To: <20230608171507.GA1204894@bhelgaas>
Il 08/06/23 19:15, Bjorn Helgaas ha scritto:
> On Thu, May 04, 2023 at 01:35:08PM +0200, AngeloGioacchino Del Regno wrote:
>> In mtk_pcie_suspend_noirq() and mtk_pcie_resume_noirq() we are,
>> respectively, disabling and enabling generation of interrupts and
>> then saving and restoring the enabled interrupts register: since
>> we're using noirq PM callbacks, that can be safely done without
>> holding any spin lock.
>
> Tangent: it's annoying that pcie-mediatek.c and pcie-mediatek-gen3.c
> use identical "mtk_pcie_suspend_noirq()" names. That makes browsing
> harder than it needs to be. But I see that you refer to
> mediatek-gen3.
>
>> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>> @@ -963,8 +963,6 @@ static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
>> {
>> int i;
>>
>> - raw_spin_lock(&pcie->irq_lock);
>> -
>> pcie->saved_irq_state = readl_relaxed(pcie->base + PCIE_INT_ENABLE_REG);
>>
>> for (i = 0; i < PCIE_MSI_SET_NUM; i++) {
>> @@ -973,16 +971,12 @@ static void mtk_pcie_irq_save(struct mtk_gen3_pcie *pcie)
>> msi_set->saved_irq_state = readl_relaxed(msi_set->base +
>> PCIE_MSI_SET_ENABLE_OFFSET);
>> }
>> -
>> - raw_spin_unlock(&pcie->irq_lock);
>> }
>
> Jianjun added mtk_pcie_irq_save() and mtk_pcie_irq_restore() with
> d537dc125f07 ("PCI: mediatek-gen3: Add system PM support").
>
> I suggest looking at other drivers and structuring mediatek-gen3
> similarly, including using similar function names. No other drivers
> have a .*_pcie_irq_save() function. Several have .*_pcie_host_init(),
> and some of those do include some IRQ setup.
>
> Bjorn
Hello Bjorn,
thanks for the feedback!
Yes, I of course refer to the Gen3 driver... I'll check other drivers and
will try to improve the consistency of this one with the others as soon as
I have some bandwidth to perform the job.
Thanks again,
Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-06-09 7:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-04 11:35 [PATCH 0/2] MediaTek PCIe Gen3: Suspend fixes AngeloGioacchino Del Regno
2023-05-04 11:35 ` [PATCH 1/2] PCI: mediatek-gen3: Stop acquiring spinlocks in {suspend,resume}_noirq AngeloGioacchino Del Regno
[not found] ` <479acc85-2349-d0ac-851c-f57b1bf6aa9e@suse.com>
2023-06-08 10:17 ` AngeloGioacchino Del Regno
2023-06-08 17:15 ` Bjorn Helgaas
2023-06-09 7:29 ` AngeloGioacchino Del Regno [this message]
2023-05-04 11:35 ` [PATCH 2/2] PCI: mediatek-gen3: Assert MAC reset only if PHY reset also present AngeloGioacchino Del Regno
2023-12-28 15:00 ` Nícolas F. R. A. Prado
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=a5597b19-eca8-10b0-1fe4-8f6e1d1d1adf@collabora.com \
--to=angelogioacchino.delregno@collabora.com \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=jianjun.wang@mediatek.com \
--cc=kw@linux.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=matthias.bgg@gmail.com \
--cc=p.zabel@pengutronix.de \
--cc=robh@kernel.org \
--cc=ryder.lee@mediatek.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