Re: [PATCH RESEND v2 3/7] PCI: Add new array for keeping track of ordering of reset methods
From: Krzysztof Wilczyński <hidden>
Date: 2021-05-20 15:26:52
Also in:
lkml
Hi Amey, [...]
+/* + * The ordering for functions in pci_reset_fn_methods + * is required for reset_methods byte array defined + * in struct pci_dev + */
A small nitpick: missing period at the end of the sentence in the comment above, and in other comments too. Might add for completeness and consistency. [...]
+typedef int (*pci_reset_fn_t)(struct pci_dev *, int);
+
+struct pci_reset_fn_method {
+ pci_reset_fn_t reset_fn;
+ char *name;
+};Question about the custom type definition above: would it be really needed? It there is only potentially a limited use for it, then perhaps it would not be useful to have one? Linus also has some preference on usage of custom types, as per: https://yarchive.net/comp/linux/typedefs.html But, in the end, this really boils down to a matter of style and/or preference. [...]
+#define PCI_RESET_FN_METHODS 5
Not sure if worth changing name of this constant, but what about the following: #define PCI_RESET_FN_METHODS_NUM 5 Or even perhaps: #define PCI_RESET_METHODS_NUM 5 So it's a little bit more self-explanatory. This would be in the similar notion, as per: https://elixir.bootlin.com/linux/v5.13-rc2/source/include/linux/pci.h#L115 [...]
+ u8 reset_methods[PCI_RESET_FN_METHODS]; /* Array for storing ordering of reset methods */
This comment reads somewhat awkward - we know that an array would be used, most likely, for storing things, thus what about the following: /* Reset methods ordered by priority */ Just a suggestion, though. Krzysztof