On 14-Mar-19 17:35, Jason Gunthorpe wrote:
> On Thu, Mar 14, 2019 at 03:14:37PM +0200, Gal Pressman wrote:
>
>>>> +static void efa_remove_device(struct pci_dev *pdev)
>>>> +{
>>>> + struct efa_dev *dev = pci_get_drvdata(pdev);
>>>> + struct efa_com_dev *edev;
>>>> +
>>>> + edev = dev->edev;
>>>> + efa_com_admin_destroy(edev);
>>>> + efa_free_mgmnt_irq(dev);
>>>> + efa_disable_msix(dev);
>>>> + efa_com_mmio_reg_read_destroy(edev);
>>>> + devm_iounmap(&pdev->dev, edev->reg_bar);
>>>> + efa_release_bars(dev, EFA_BASE_BAR_MASK);
>>>> + kfree(edev);
>>>> + ib_dealloc_device(&dev->ibdev);
>>>> + pci_disable_device(pdev);
>>>
>>> This should probably be using the new dealloc_driver flow, it is
>>> somewhat easier to see how the lifetimes work..
>>
>> Didn't see this in for-next yet? The core API changes very frequently, these
>> changes could block the driver forever :).
>
> struct ib_device_ops {
> [..]
>
> /* Device lifecycle callbacks */
> /*
> * Called after the device becomes registered, before clients are
> * attached
> */
> int (*enable_driver)(struct ib_device *dev);
> /*
> * This is called as part of ib_dealloc_device().
> */
> void (*dealloc_driver)(struct ib_device *dev);
>
> Feel free to add more callbacks if it makes the driver clearer (ala
> netdev).
>
> dealloc_driver is called as part of ib_unregister_device()
>
> This driver doesn't strictly require this (siw does) but using it does seem
> like a clearer driver design..
Thanks Jason, will take a look.
