On 8/14/2018 9:27 PM, Laszlo Ersek wrote:
[...]
+cap_error:
+ msi_uninit(dev);
(4) This error handler doesn't look entirely correct; we can reach it
without having initialized MSI. (MSI setup is conditional; and even if
we attempt it, it is permitted to fail with "msi=auto".) Therefore,
msi_uninit() should be wrapped into "if (msi_present())", same as you
see in pci_bridge_dev_exitfn().
sure, can add a pre-check. But I don't understand why we need that,
msi_uninit() will check msi_present()?
Thanks for pointing that out.
So, I grepped the source code for msi_uninit(). It seems that only one
location gates it with msi_present() -- in pci_bridge_dev_exitfn() --,
while 13 other locations just call it unconditionally.
I don't know what the consistent approach is here. I'll have to defer to
the PCI maintainers; their taste will decide. Technically your code
looks correct, then.
Yes, I also grep that. I guess if we directly use msi_uninit, this is
because msi_init is non-conditional too.
Anyway, I added check in my version two. BTW, other comments are
included in v2.
Thanks,
Jing
Thanks
Laszlo
