---- On Mon, 07 Feb 2022 12:56:13 -0500 Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote ---- > On Mon, Feb 07, 2022 at 04:09:05AM -0500, Li Chen wrote: > > From: Li Chen <lchen@xxxxxxxxxxxxx> > > > > Introduce pci_endpoint_epf_transfer_data to simplify > > read and write operations. > > > > Also tabify this file. > > Thanks for the patch. > > This doesn't apply cleanly on v5.17-rc1. Please make it apply cleanly > there or at least mention where it *does* apply. Ok, will send v7 which will be based on v5.17-rc3. > > Please separate the whitespace tabification changes and the > pci_endpoint_epf_transfer_data() changes into two separate patches. > When they're mixed together, it's harder to review the patch. > > > #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \ > > - miscdev) > > + miscdev) > > Always indent with tabs when possible: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=v5.16#n18 > > Hmm, coding-style.rst is unfortunately not very explicit about that. > > But it's obvious from the existing code in this file that things > should not be indented four spaces, as you did in > pci_endpoint_test_transfer_data(). So, can I say space is not allowed in indentation? If so, I should not use emacs's tabify, because it will not convert 4 space to 8-width tab. I'm also not sure is scripts/Lindent or clang-format is a good choice here, they do too much changes. > > Your patch should match the style of the existing code. > > > +static bool pci_endpoint_test_transfer_data(struct pci_endpoint_test *test, > > + unsigned long arg, const int operation) > > +{ > > + struct pci_endpoint_test_xfer_param param; > > + bool ret = false; > > + u32 flags = 0; > > > > + // if we ask rc to write to ep, then ep should do read operation, and vice versa. > > Please use /* */ comments to match the prevailing kernel comment > style: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=v5.16#n598 > > And spell out or at least capitalize "RC" and "EP" since they're not > real words. > Thanks, will do it in v7. > Bjorn > Regards, Li