Messages in this thread | | | Date | Thu, 20 May 2021 16:54:50 +0200 | From | Krzysztof Wilczyński <> | Subject | Re: [PATCH RESEND v2 1/7] PCI: merge slot and bus reset implementations |
| |
Hi Amey,
Thank you for working on this! Few comments and suggestions below.
[...] > Link: https://lkml.org/lkml/2021/3/23/911
Linking to lkml.org is fine, however it became a canon now to link to lore, so this would be:
https://lore.kernel.org/lkml/20210323100625.0021a943@omen.home.shazbot.org/
I personally find it a bit easier to read on lore compared to lkml.org when it goes to a large and long running threads.
[...] > +int pci_reset_bus_function(struct pci_dev *dev, int probe) > +{ > + int rc = pci_dev_reset_slot_function(dev, probe); > + > + if (rc != -ENOTTY) > + return rc; > + return pci_parent_bus_reset(dev, probe); > +}
Depends on the style, but I would suggest using a boolean type for the probe argument here and in the other functions that enable or disable something. I makes the intent clear, and this is also a popular pattern you can see throughout the PCI tree.
Also, I would suggest adding a newline to separate final return, so that it's easier to read the code, and to keep things consistent.
[...] > - rc = pci_dev_reset_slot_function(dev, 0); > - if (rc != -ENOTTY) > - return rc; > - return pci_parent_bus_reset(dev, 0); > + return pci_reset_bus_function(dev, 0);
See above about using boolean type here.
[...] > - rc = pci_dev_reset_slot_function(dev, 1); > if (rc != -ENOTTY) > return rc; > > - return pci_parent_bus_reset(dev, 1); > + return pci_reset_bus_function(dev, 1);
Same as above.
Krzysztof
| |