[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240805132442.GA7274@thinkpad>
Date: Mon, 5 Aug 2024 18:54:42 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Lukas Wunner <lukas@...ner.de>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Len Brown <lenb@...nel.org>,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, mika.westerberg@...ux.intel.com,
Hsin-Yi Wang <hsinyi@...omium.org>
Subject: Re: [PATCH v5 1/4] PCI/portdrv: Make use of pci_dev::bridge_d3 for
checking the D3 possibility
On Fri, Aug 02, 2024 at 10:07:39PM +0200, Lukas Wunner wrote:
> On Fri, Aug 02, 2024 at 01:19:24PM +0200, Rafael J. Wysocki wrote:
> > On Fri, Aug 2, 2024 at 11:49AM Lukas Wunner <lukas@...ner.de> wrote:
> > >
> > > On Fri, Aug 02, 2024 at 11:25:00AM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > > PCI core is already caching the value of pci_bridge_d3_possible() in
> > > > pci_dev::bridge_d3 during pci_pm_init(). Since the value is not going to
> > > > change,
> >
> > Is that really the case?
> >
> > Have you seen pci_bridge_d3_update()?
>
> Okay the value may change at runtime, e.g. due to user space
> manipulating d3cold_allowed in sysfs.
>
The last part of the commit message is wrong, but pci_bridge_d3_update() is
already updating pci_dev::bridge_d3. And pci_bridge_d3_possible() is not making
use of any checks that could change dynamically IIUC. So what is wrong in using
pci_dev::bridge_d3?
Even more, if the client drivers have changed the state of pci_dev::bridge_d3
during runtime, then pci_bridge_d3_possible() won't catch that. Or is there a
reason to not do so purposefully?
> > > I don't know if there was a reason to call pci_bridge_d3_possible()
> > > (instead of using the cached value) on probe, remove and shutdown.
> > >
> > > The change is probably safe but it would still be good to get some
> > > positive test results with Thunderbolt laptops etc to raise the
> > > confidence.
> >
> > If I'm not mistaken, the change is not correct.
>
> You're right. Because the value may change, different code paths
> may be chosen on probe, remove and shutdown. Sorry for missing that.
>
Again, pci_bridge_d3_possible() is not making use of values that could change
dynamically.
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists