On Thu, Jun 21, 2018 at 03:06:00PM +0300, Mika Westerberg wrote:
> On Sat, Jun 16, 2018 at 09:25:00PM +0200, Lukas Wunner wrote:
> > struct controller {
> > struct mutex ctrl_lock;
> > struct pcie_device *pcie;
> > + struct rw_semaphore reset_lock;
>
> Is there any particular reason why you can't use a regular mutex here?
It would unnecessarily serialize pciehp_probe() against pciehp_ist()
even though it's legal for the two to run in parallel.
I only want to serialize pciehp_reset_slot() with the rest of the driver,
more specifically those portions accessing the Presence Detect State bit
in the Slot Status register and the Data Link Layer Link Active bit in
the Link Status register. And the only functions remaining that access
those bits are pciehp_probe() and pciehp_ist(). The tool to achieve that
kind of serialization is an rw_semaphore.
That said, I don't know for sure that the two above-mentioned bits flap
on performing a slot reset. But I highly suspect it, so the code looked
unsafe. It would be good if Rajat Jain and Alex Williamson, who
introduced pciehp_reset_slot(), could voice their opinion as to the
patch's necessity.
Thanks,
Lukas
