Re: [PATCH v3 2/2] PCI: pciehp: Clear the optional capabilities in DEVCTL2 on a hot-plug

From: Smita Koralahalli
Date: Tue Jun 27 2023 - 13:39:04 EST


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