From: Bjorn Helgaas <helgaas@kernel.org>
To: Songyang Li <leesongyang@outlook.com>
Cc: bhelgaas@google.com, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: Cancel compilation restrictions on function pcie_clear_device_status
Date: Mon, 17 Jun 2024 11:31:06 -0500 [thread overview]
Message-ID: <20240617163106.GA1217016@bhelgaas> (raw)
In-Reply-To: <TY3P286MB2754F489000B7FA6F9CF19D8B4CD2@TY3P286MB2754.JPNP286.PROD.OUTLOOK.COM>
On Mon, Jun 17, 2024 at 09:35:20PM +0800, Songyang Li wrote:
> On Sat, 15 Jun 2024 16:26:03 -0500, Bjorn Helgaas wrote:
> > > On Wed, 12 Jun 2024 15:14:32 -0500, Bjorn Helgaas wrote:
> > > > I think all current any callers of pcie_clear_device_status() are also
> > > > under CONFIG_PCIEAER, so I don't think this fixes a current problem.
> > > >
> > > > As you point out, it might make sense to use
> > > > pcie_clear_device_status() even without AER, but I think we should
> > > > include this change at the time when we add such a use.
> > > >
> > > > If I'm missing a use with the current kernel, let me know.
> > >
> > > As far as I know, some PCIe device drivers, for example,
> > > [net/ethernet/broadcom/tg3.c],[net/ethernet/atheros/atl1c/atl1c_main.c],
> > > which use the following code to clear the device status register,
> > > pcie_capability_write_word(tp->pdev, PCI_EXP_DEVSTA,
> > > PCI_EXP_DEVSTA_CED |
> > > PCI_EXP_DEVSTA_NFED |
> > > PCI_EXP_DEVSTA_FED |
> > > PCI_EXP_DEVSTA_URD);
> > > I think it may be more suitable to export the pcie_clear_device_status()
> > > for use in the driver code.
> >
> > If we want to use this from drivers, it would make sense to do
> > something like this patch, and this patch could be part of a series to
> > call it from the drivers.
> >
> > But at the same time, we should ask whether drivers should be clearing
> > this status themselves, or whether it should be done by the PCI core.
>
> After careful consideration, I agree with your point of view.
> I hold a viewpoint that it should be done by the PCI core,
> rather than pcie drivers. I give up this patch, and then I have
> gained a profound understanding of PCIe Core from this communication.
I tend to think this should be done by the PCI core, but I haven't
looked at it enough to know how or where. If you pursue it, I'd love
to see your ideas!
Thanks,
Bjorn
next prev parent reply other threads:[~2024-06-17 16:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-11 15:15 [PATCH] PCI: Cancel compilation restrictions on function pcie_clear_device_status Songyang Li
2024-06-11 22:34 ` Bjorn Helgaas
2024-06-12 15:15 ` Songyang Li
2024-06-12 20:14 ` Bjorn Helgaas
2024-06-15 3:13 ` Songyang Li
2024-06-15 21:26 ` Bjorn Helgaas
2024-06-17 13:35 ` Songyang Li
2024-06-17 16:31 ` Bjorn Helgaas [this message]
2024-06-23 3:28 ` Songyang Li
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=20240617163106.GA1217016@bhelgaas \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=leesongyang@outlook.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.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