Hi Bjorn, Thanks a lot for your comments! > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] > Sent: 2019年3月11日 21:33 > To: Z.q. Hou <zhiqiang.hou@xxxxxxx> > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; l.subrahmanya@xxxxxxxxxxxxxx; > shawnguo@xxxxxxxxxx; Leo Li <leoyang.li@xxxxxxx>; > lorenzo.pieralisi@xxxxxxx; catalin.marinas@xxxxxxx; > will.deacon@xxxxxxx; M.h. Lian <minghuan.lian@xxxxxxx>; Xiaowei Bao > <xiaowei.bao@xxxxxxx>; Mingkai Hu <mingkai.hu@xxxxxxx> > Subject: Re: [PATCHv4 00/28] PCI: refactor Mobiveil driver and add PCIe Gen4 > driver for NXP Layerscape SoCs > > Hi, > > On Mon, Mar 11, 2019 at 09:29:54AM +0000, Z.q. Hou wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx> > > > > This patch set is aim to refactor the Mobiveil driver and add PCIe > > support for NXP Layerscape series SoCs integrated Mobiveil's PCIe Gen4 > > controller. > > > > Hou Zhiqiang (28): > > PCI: mobiveil: uniform the register accessors > > "uniform" is not a verb. Maybe "Unify register accessors"? > > > PCI: mobiveil: format the code without function change > > PCI: mobiveil: correct the returned error number > > PCI: mobiveil: remove flag MSI_FLAG_MULTI_PCI_MSI > > PCI: mobiveil: correct PCI base address in MEM/IO outbound windows > > PCI: mobiveil: replace the resource list iteration function > > PCI: mobiveil: use WIN_NUM_0 explicitly for CFG outbound window > > PCI: mobiveil: use the 1st inbound window for MEM inbound > transactions > > PCI: mobiveil: correct inbound/outbound window setup routines > > PCI: mobiveil: fix the INTx process error > > PCI: mobiveil: only fix up the Class Code field > > PCI: mobiveil: move out the link up waiting from mobiveil_host_init > > Add parens for function names, e.g., "mobiveil_host_init()". This occurs > several more times, including both subject lines and changelogs. > > > PCI: mobiveil: move irq chained handler setup out of DT parse > > Capitalize acronyms in English text (subject lines, changelogs, comments), e.g., > s/irq/IRQ/ > > > PCI: mobiveil: initialize Primary/Secondary/Subordinate bus number > > dt-bindings: pci: mobiveil: change gpio_slave and apb_csr to optional > > PCI: mobiveil: refactor Mobiveil PCIe Host Bridge IP driver > > This should give a hint about the purpose of refactoring. Sounds like it's to > make it easier to support both host and endpoint mode? > > > PCI: mobiveil: fix the checking of valid device > > PCI: mobiveil: add link up condition check > > PCI: mobiveil: continue to initialize the host upon no PCIe link > > PCI: mobiveil: disabled IB and OB windows set by bootloader > > PCI: mobiveil: add Byte and Half-Word width register accessors > > "Byte" and "Half-Word" do not need to be capitalized. Also, the changelog > has a typo: "Half-Work" for "half-word". > > > PCI: mobiveil: make mobiveil_host_init can be used to re-init host > > Here's another of the places that need parens after the function name. > > > dt-bindings: pci: Add NXP Layerscape SoCs PCIe Gen4 controller > > PCI: mobiveil: add PCIe Gen4 RC driver for NXP Layerscape SoCs > > PCI: mobiveil: ls_pcie_g4: add Workaround for A-011577 > > PCI: mobiveil: ls_pcie_g4: add Workaround for A-011451 > > The reader of these changelogs likely doesn't know what internal identifiers > like "A-011577" mean, but *does* want a hint about what problem is being > fixed and what platforms are affected. So instead of the "ls_pcie_g4:" prefix, > use something like: > > PCI: mobiveil: Work around LX2160A r1.0 config access erratum > PCI: mobiveil: Work around LX2160A r1.0 split completion erratum > > and mention the erratum ID (A-011577) in the changelog. If you can include > the actual erratum text in the changelog, that would be even better. > > s/ERRATA/errata/ in the changelogs. > > > arm64: dts: freescale: lx2160a: add pcie DT nodes > > "PCIe" > > > arm64: defconfig: Enable CONFIG_PCI_LAYERSCAPE_GEN4 > > I already asked you once [1] to: > > please pay attention to the changelog conventions, e.g., capitalize the > first word of the sentence ("Remove flag ...", "Correct PCI base address > ...", etc), capitalize acronyms like "PCI" and "IRQ", use parentheses > after function names, etc. You can see the conventions by running "git > log --oneline drivers/pci/controller". > > For example, instead of this: > > PCI: mobiveil: add link up condition check > > it should be this: > > PCI: mobiveil: Add link up condition check > > Please wait at least a few days before posting a v5 in case there are other > comments. Thanks for your patience and will fix them in v5. > > Bjorn > > [1] > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore. > kernel.org%2Flinux-pci%2F20190130153447.GB229773%40google.com& > data=02%7C01%7Czhiqiang.hou%40nxp.com%7C591b7c129f0d42caf3b908d6 > a6261d39%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6368790 > 79968881729&sdata=1nwycpmT9p1VhdrZ1fdQXk0UQ4iiKVIEfDe3IGYgX > nI%3D&reserved=0 Thanks, Zhiqiang
