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
