# Re: \[PATCH v5 10/11\] PCI: Replace pci\_dev::driver usage by pci\_dev::dev.driver

- From: Andrew Donnellan \<hidden\>
- Date: 2021-09-29 15:46:03 
- List: linuxppc-dev
- Page: https://ratatoskr.run/linuxppc-dev/2021/09/8878930
- Thread: https://ratatoskr.run/linux-pci/2021/09/1525586/t.md

```text
On 29/9/21 11:43 pm, Uwe Kleine-König wrote:> I'm not a huge fan either, I used it to keep the control flow as is and
> without introducing several calls to to_pci_driver.
>
> The whole code looks as follows:
>
> 	list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
> 		struct pci_driver *afu_drv;
> 		if (afu_dev->dev.driver &&
> 		    (afu_drv = to_pci_driver(afu_dev->dev.driver))->err_handler &&
> 		    afu_drv->err_handler->resume)
> 			afu_drv->err_handler->resume(afu_dev);
> 	}
>
> Without assignment in the if it could look as follows:
>
> 	list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
> 		struct pci_driver *afu_drv;
>
> 		if (!afu_dev->dev.driver)
> 			continue;
>
> 		afu_drv = to_pci_driver(afu_dev->dev.driver));
>
> 		if (afu_drv->err_handler && afu_drv->err_handler->resume)
> 			afu_drv->err_handler->resume(afu_dev);
> 	}
>
> Fine for me.

This looks fine.

As an aside while writing my email I discovered the existence of container_of_safe(), a version of container_of() that handles the null and err ptr cases... if to_pci_driver() used that, the null check in the caller could be moved until after the to_pci_driver() call which would be neater.

But then, grep tells me that container_of_safe() is used precisely zero times in the entire tree. Interesting.

> (Sidenote: What happens if the device is unbound directly after the
> check for afu_dev->dev.driver? This is a problem the old code had, too
> (assuming it is a real problem, didn't check deeply).)

Looking at any of the cxl PCI error handling paths brings back nightmares from a few years ago... Fred: I wonder if we need to add a lock here?

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited
```