Re: [PATCH v10 5/6] PCI: endpoint: makeup pci-epf-vntb.c

Bjorn Helgaas
2022-09-13T22:19:26Z

In subject, capitalize and change "makeup" to "Clean up":

PCI: endpoint: pci-epf-vntb: Clean up

On Tue, Sep 13, 2022 at 04:09:56PM -0500, Frank Li wrote:
> Remove unused field: epf_db_phy.
> Remove __iomem before epf_db.
> Remove dupicate check if (readl(ntb->epf_db + i * 4)).
> Using readl_relaxed instead of readl.
> Using marco ENTRY_SIZE instead of number 4 at all place.

Add "()" after function names.

s/marco/macro/

It would be nice if "ENTRY_SIZE" had a hint about what kind of entry
we're talking about.

Since this is a collection of random cleanups, I noticed a typo in
epf_ntb_configure_interrupt() kernel-doc: s/capaiblity/capability/

The struct epf_ntb_ctrl definition is also whitespace-damaged. The
members of struct epf_ntb_ctrl and struct epf_ntb should follow the
same indentation style. Some members of struct epf_ntb_ctrl are
indented with a tab, others with space. Either make them all tabs and
indent struct epf_ntb similarly, or indent the struct epf_ntb_ctrl
members with a single space.

The comments in the file have a whole collection of ways to spell
vhost: Virtual Host, VHOST, VHost, vHOST, vhost. Make them all the
same, please. You can use "Virtual Host (VHOST)" or whatever the
first time if you want to use the short version later.

Same for host/HOST/etc. I don't want to read things like this:

@ntb: NTB device that facilitates communication between HOST and vHOST2

Wrapper to initialize a particular EPC interface and start the
workqueue to check for commands from host.

and wonder whether "host" is supposed to be the same as "HOST". Also,
why does that say "vHOST*2*"?

There are several instances of "HOST1" and "HOST2" (and "vHOST2").
Should those appear somewhere in the diagram at the top of the file?

The diagram starts with "/**" which means it's kernel-doc, but the
diagram is not kernel-doc. Please run this:

scripts/kernel-doc -v -none drivers/pci/endpoint/functions/pci-epf-vntb.c

and fix all the warnings.

Bjorn