On Mon, Oct 16, 2023 at 12:27:42PM -0500, Bjorn Helgaas wrote: > "git am" complained about a couple whitespace errors elsewhere in this > series: > > Applying: arch/sh/boot/compressed/head_32.S: passing FDT address to initialize function. > .git/rebase-apply/patch:25: trailing whitespace. > Applying: drivers/irqchip: SH7751 IRL external encoder with enable gate. > .git/rebase-apply/patch:33: new blank line at EOF. BTW, I mentioned all these things and more a month or so ago: https://lore.kernel.org/r/20230918191602.GA201859@bhelgaas https://lore.kernel.org/r/20230918193337.GA203483@bhelgaas https://lore.kernel.org/r/20230918193036.GA203163@bhelgaas I *thought* this seemed familiar ;) Bjorn > On Sat, Oct 14, 2023 at 11:53:44PM +0900, Yoshinori Sato wrote: > > pci-sh7751.h move from "arch/sh/drivers/pci/pci-sh7751.h" > > pci-sh7751.c convert from "arch/sh/drivers/pci/pci-sh7751.c" > > Note the subject line conventions in drivers/pci (use "git log > --oneline" to see them): use something like this: > > PCI: sh7751: Add SH7751 PCI host bridge driver > > with no period at the end. > > arch/sh/drivers/pci/pci-sh7751.h and arch/sh/drivers/pci/pci-sh7751.c > still exist after applying this series. Better to have a single patch > that moves the content from arch/sh/drivers/pci/ to > drivers/pci/controller/. > > Neither file looks like a simple move; there's a lot of reorganization > going on at the same time. It's really difficult to review a patch > like that because we can't compare the content from before and after. > > If you make a patch that does the cleanup/reorganization, and a > separate patch that is just a simple move, and you use "git mv" for > the move, git should notice that this is just a rename, and that diff > will be tiny. > > One of the reorganization patches should be to incorporate the > pci-sh7751.h content directly into pci-sh7751.c. Since it's only used > in pci-sh7751.c, there's really no advantage to having it in a > separate file. > > Bjorn > > > Signed-off-by: Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx> > > --- > > drivers/pci/controller/Kconfig | 9 + > > drivers/pci/controller/Makefile | 1 + > > drivers/pci/controller/pci-sh7751.c | 285 ++++++++++++++++++++++++++++ > > drivers/pci/controller/pci-sh7751.h | 267 ++++++++++++++++++++++++++ > > 4 files changed, 562 insertions(+) > > create mode 100644 drivers/pci/controller/pci-sh7751.c > > create mode 100644 drivers/pci/controller/pci-sh7751.h > > > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > > index c0c3f2824990..037ff44bd1e8 100644 > > --- a/drivers/pci/controller/Kconfig > > +++ b/drivers/pci/controller/Kconfig > > @@ -342,6 +342,15 @@ config PCIE_XILINX_CPM > > Say 'Y' here if you want kernel support for the > > Xilinx Versal CPM host bridge. > > > > +config PCI_SH7751 > > + bool "Renesas SH7751 PCI controller" > > + depends on OF > > + depends on CPU_SUBTYPE_SH7751 || CPU_SUBTYPE_SH7751R || COMPILE_TEST > > + select PCI_HOST_COMMON > > + help > > + Say 'Y' here if you want kernel to support the Renesas SH7751 PCI > > + Host Bridge driver. > > Move this so the menu entries stay sorted by vendor name. > > Bjorn
