Messages in this thread | | | Date | Tue, 27 Jun 2023 10:38:54 -0700 | Subject | Re: [PATCH v3 2/2] PCI: pciehp: Clear the optional capabilities in DEVCTL2 on a hot-plug | From | Smita Koralahalli <> |
| |
On 6/22/2023 2:42 PM, Lukas Wunner wrote: > [cc += Jay, Felix] > > On Thu, Jun 22, 2023 at 02:02:12PM -0700, Smita Koralahalli wrote: >> Would it be fair to just reuse pci_enable_atomic_ops_to_root() for >> Atomic_Ops configuration? > > Hm, that's a good question. I'm not an expert on that corner of > the PCI core. > > But indeed what you could try is amend that function to not only > *set* PCI_EXP_DEVCTL2_ATOMIC_REQ if it's supported, but to also > *clear* it if it's not supported. > > And you'd have to call pci_enable_atomic_ops_to_root() on enumeration, > e.g. from pci_init_capabilities(). > > That should obviate the need to call pci_enable_atomic_ops_to_root() > from drivers, so you could probably remove the call from all the > drivers which currently call it (amdgpu, infiniband, mellanox), > in one separate patch per driver. > > An then you could drop the EXPORT clause for pci_enable_atomic_ops_to_root() > and make it private to the PCI core. > > So that would be 5 patches (enablement/disablement on enumeration, > amendmend of the 3 drivers, making the call private). > > I'm not sure if anyone will cry foul if you do that but if you want > to give it a try, go for it. :)
Okay, I see there are no objections except for Bjorn/Jay's comments on
"But there could be devices where AtomicOps are nominally supported but untested or broken.."
Would this be an issue?
If not, I will start working on those 5 patches.
Thanks, Smita > > I don't now why commit 430a23689dea, which introduced > pci_enable_atomic_ops_to_root(), chose to add it as a library function > which is only called from specific drivers, instead of universally > enabling the feature for all devices. Adding the commit authors to cc > so they can chime in. > > Thanks, > > Lukas
| |