On Thu, May 11, 2023 at 10:55:06AM -0500, Bjorn Helgaas wrote:
> On Thu, May 11, 2023 at 04:14:25PM +0300, Ilpo Järvinen wrote:
> > +int pcie_capability_clear_and_set_word_locked(struct pci_dev *dev, int pos,
> > + u16 clear, u16 set)
> > +{
> > + unsigned long flags;
> > + int ret;
> > +
> > + spin_lock_irqsave(&dev->cap_lock, flags);
> > + ret = pcie_capability_clear_and_set_word(dev, pos, clear, set);
> > + spin_unlock_irqrestore(&dev->cap_lock, flags);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(pcie_capability_clear_and_set_word_locked);
>
> I didn't see the prior discussion with Lukas, so maybe this was
> answered there, but is there any reason not to add locking to
> pcie_capability_clear_and_set_word() and friends directly?
>
> It would be nice to avoid having to decide whether to use the locked
> or unlocked versions.
I think we definitely want to also offer lockless accessors which
can be used in hotpaths such as interrupt handlers if the accessed
registers don't need any locking (e.g. because there are no concurrent
accesses).
So the relatively lean approach chosen here which limits locking to
Link Control and Link Control 2, but allows future expansion to other
registers as well, seemed reasonable to me.
Thanks,
Lukas
