| Message ID | 20161013140746.GA14174@localhost |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Oct 13, 2016 at 7:07 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > Here are some more changes I'd like to have in v4.9. There's one small > Tegra bug fix in the PHY poweroff path, which is only used in failure > paths. The rest is all strictly cleanup that should make host bridge > drivers more readable, but shouldn't actually change any behavior. Ugh: > PCI changes for the v4.9 merge window: > > Altera host bridge driver > Add local struct device pointers (Bjorn Helgaas) Your "summary" is actually less legible than the shortlog, and looks entirely auto-generated. That's against the whole point of having a summary for a pull request. Please tell me what changed, don't auto-generate pointless unreadable crud. Ok? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 13, 2016 at 4:48 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Ugh: > > Your "summary" is actually less legible than the shortlog, and looks > entirely auto-generated. > > That's against the whole point of having a summary for a pull request. > > Please tell me what changed, don't auto-generate pointless unreadable crud. Ok? I wrote some kind of summary based on actually (hopefully) intelligent culling of relevant information instead of just bunching up together automated data. It may not be complete, but it's the kind of "what does this merge actually _merge_" information that is useful. Please do something like this in the future rather than the automated illegible mush: "Summary: - use local struct device pointers in many host bridge drivers for clarity - remove unused platform data - use generic DesignWare accessors - misc cleanups: remove redundant structure entries and re-order structure members to put comon generic fields first etc" See? Humans can read that and get an idea of what is going on. It would be even better if it had a few more lines about why those things matter, but as is, it's much better than that that tag contained. But the reason I want the maintainer to write this is that I'm lazy. No, wait, it's really because you are supposed to know the high-level detail and be able to really notice what is relevant. I just went by looking for patterns in the shortlog, doing silly things like git shortlog ..FETCH_HEAD --no-merges | cut -d: -f3- | sort | uniq -c | sort -n to get a feel for what you had done. But because I don't know the code, maybe I missed some really important change, or mis-characterized it somehow, or whatever. Which is why I want the pull requests to contain the kind of relevant information on what the hell is going on. Not just a "this is all that happened" munged shortlog in a slightly different format. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 13, 2016 at 05:21:40PM -0700, Linus Torvalds wrote: > On Thu, Oct 13, 2016 at 4:48 PM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > Ugh: > > > > Your "summary" is actually less legible than the shortlog, and looks > > entirely auto-generated. > > > > That's against the whole point of having a summary for a pull request. > > > > Please tell me what changed, don't auto-generate pointless unreadable crud. Ok? > > I wrote some kind of summary based on actually (hopefully) intelligent > culling of relevant information instead of just bunching up together > automated data. > > It may not be complete, but it's the kind of "what does this merge > actually _merge_" information that is useful. Please do something like > this in the future rather than the automated illegible mush: > > "Summary: > > - use local struct device pointers in many host bridge drivers for > clarity > > - remove unused platform data > > - use generic DesignWare accessors > > - misc cleanups: remove redundant structure entries and re-order > structure members to put comon generic fields first etc" That's accurate, thank you, and sorry for not providing it myself. I guess I've always been a little confused on what to put in the email vs. what should be in the tag message itself. I'm glad you pointed this out because it's just dawning on me that the tag message does not become part of your tree when you pull the tag, while the summary from the email pull request normally does. I've been focusing on the tag message, not the email summary, which was completely backwards. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 14, 2016 at 6:25 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > I guess I've always been a little confused on what to put in the email > vs. what should be in the tag message itself. So I actually end up using either or *both* for my merge messages. A lot of people put a "short and concise bulleted summary" in their tag message, and then put more of a free-form "wall of text" explanation in the email. Other people just put both in the tag, and let "git request-pull" take the tag-message as-is for the email. Yet other people don't really use the tag at all (Bottomley, for example, tends to have just something like "scsi-20161014" in his tag) and then the explanation and summary in the email. All of those options are _fine_. I don't really mind much one way or the other. But what I don't want in *either* the tag *or* the email message is just some automated list of all the commits. (Except for the shortlog itself, I mean. I obviously want *that* automated list of commits in the email, together with the expected diffstat) I actually think that from a first approximation, James Bottomley's approach is a good starting point. Why? Because if you think of the email as something that you send to me to explain why I should pull and what I get, your email summary text is likely all I really want. The people who put the message in the tag have often started out that way, and then they get to the point where they just automate the pull request thing by just writing a message _once_ (for the tag), and have it automatically show up in the email body. So I don't actually care much where the text is. I do want the email itself to have the summary explanation (so that I can see what you are talking about _before_ I even fetch the tag), but it's ok if the tag has some extra summary detail in it, or if the tag has just the same summary as the email, or if the tag has basically nothing at all and ends up being used only for signing purposes. What I *do* care about is that the explanation is an *explanation*. For *humans*. It's an explanation for me ("Why should I pull?") but it's also an explanation for the future ("What goes into the merge message?"). Btw, many people explain things like expected merge conflicts etc, and those I usually don't put in the merge message simply because they no longer make sense after the merge is already done. So the "for me" and "for the merge message" is not always the exact same thing, but to a close approximation the two are often very similar. And I do end up always editing the merge message. So in no case do I blindly take whatever you send me. Sometimes the editing is very minimal, sometimes I end up having to do major editing for typos or layout or something. But there is no automated "turn the email and tag into a merge message". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, Here are some more changes I'd like to have in v4.9. There's one small Tegra bug fix in the PHY poweroff path, which is only used in failure paths. The rest is all strictly cleanup that should make host bridge drivers more readable, but shouldn't actually change any behavior. Bjorn The following changes since commit bdf530984d10b6b88b10a6d03057409a3f1c6897: Merge branch 'pci/host-vmd' into next (2016-10-05 14:00:21 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git tags/pci-v4.9-changes-2 for you to fetch changes up to 217c6d21e9245e590971905fd4501fd53730c800: Merge branches 'pci/host-armada', 'pci/host-artpec', 'pci/host-dra7xx', 'pci/host-exynos', 'pci/host-hisi', 'pci/host-imx6', 'pci/host-keystone', 'pci/host-layerscape', 'pci/host-qcom' and 'pci/host-spear' into next (2016-10-12 11:15:06 -0500) ---------------------------------------------------------------- PCI changes for the v4.9 merge window: Altera host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Remove redundant platform_get_resource() return value check (Bjorn Helgaas) Rename altera_pcie_valid_config() to altera_pcie_valid_device() (Bjorn Helgaas) Simplify TLB_CFG_DW0 usage (Bjorn Helgaas) Simplify TLP_CFG_DW1 usage (Bjorn Helgaas) APM X-Gene host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Pass struct xgene_pcie_port to setup functions (Bjorn Helgaas) Add register accessors (Bjorn Helgaas) Axis ARTPEC-6 host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Add register accessors (Bjorn Helgaas) Use generic DesignWare accessors (Bjorn Helgaas) Remove unnecessary artpec6_pcie_link_up() (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Add resource name comments (Bjorn Helgaas) Broadcom iProc host bridge driver Add local struct device pointers (Bjorn Helgaas) Set drvdata at end of probe function (Bjorn Helgaas) Validate CSR base in BCMA setup code (Bjorn Helgaas) Remove redundant null pointer checking (Bjorn Helgaas) Hard-code PCIe capability offset instead of searching (Bjorn Helgaas) Freescale i.MX6 host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove redundant of_node pointer (Bjorn Helgaas) Removed unused struct imx6_pcie.mem_base (Bjorn Helgaas) Pass struct imx6_pcie to PHY accessors (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Use generic DesignWare accessors (Bjorn Helgaas) Reorder struct imx6_pcie (Bjorn Helgaas) Remove unused return values (Bjorn Helgaas) Freescale Layerscape host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Remove redundant struct ls_pcie.dbi (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Move struct pcie_port setup to probe function (Bjorn Helgaas) Remove unused ls_add_pcie_port() platform_device arg (Bjorn Helgaas) Reorder struct ls_pcie (Bjorn Helgaas) HiSilicon host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Name private struct pointer "hisi_pcie" consistently (Bjorn Helgaas) Remove redundant struct hisi_pcie.reg_base (Bjorn Helgaas) Use generic DesignWare accessors (Bjorn Helgaas) Include register block base in PCIE_SYS_STATE4 address (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Reorder struct hisi_pcie (Bjorn Helgaas) Marvell Aardvark host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) MAINTAINERS: Add DT binding to the Aardvark PCIe driver maintainer (Thomas Petazzoni) Marvell Armada host bridge driver Remove unused platform data (Bjorn Helgaas) Add local base pointer (Bjorn Helgaas) Remove redundant struct armada8k_pcie.base (Bjorn Helgaas) Use generic DesignWare accessors (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Reorder struct armada8k_pcie (Bjorn Helgaas) MAINTAINERS: Add maintainer for the PCIe Marvell Armada 8K driver (Thomas Petazzoni) Marvell MVEBU host bridge driver Add local struct device pointers (Bjorn Helgaas) Use existing of_node pointer (Bjorn Helgaas) NVIDIA Tegra host bridge driver Fix argument order in tegra_pcie_phy_disable() (Bjorn Helgaas) Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Qualcomm host bridge driver Remove unused platform data (Bjorn Helgaas) Remove redundant struct qcom_pcie.dbi (Bjorn Helgaas) Remove redundant struct qcom_pcie.dev (Bjorn Helgaas) Reorder struct qcom_pcie (Bjorn Helgaas) Renesas R-Car host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Remove unused rcar_pcie_get_resources() platform_device arg (Bjorn Helgaas) Remove DRV_NAME macro (Bjorn Helgaas) rcar-gen2: Add local struct device pointers (Bjorn Helgaas) Rockchip host bridge driver Remove unused platform data (Bjorn Helgaas) Indent "if" statement body (Dan Carpenter) Samsung Exynos host bridge driver Add local struct device pointers (Bjorn Helgaas) Uninline register accessors (Bjorn Helgaas) Name private struct pointer "exynos_pcie" consistently (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Reorder struct exynos_pcie (Bjorn Helgaas) ST Microelectronics SPEAr13xx host bridge driver Remove unused constants (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Reorder struct spear13xx_pcie (Bjorn Helgaas) Clean up struct device usage (Bjorn Helgaas) Synopsys DesignWare host bridge driver Rename dw_pcie_valid_config() to dw_pcie_valid_device() (Bjorn Helgaas) Simplify pcie_host_ops.readl_rc() and .writel_rc() interfaces (Bjorn Helgaas) Swap order of dw_pcie_writel_rc() reg/val arguments (Bjorn Helgaas) Export dw_pcie_readl_rc(), dw_pcie_writel_rc() (Bjorn Helgaas) Uninline register accessors (Bjorn Helgaas) Simplify dw_pcie_readl_unroll(), dw_pcie_writel_unroll() (Kishon Vijay Abraham I) Swap order of dw_pcie_writel_unroll() reg/val arguments (Bjorn Helgaas) designware-plat: Remove redundant dw_plat_pcie.mem_base (Bjorn Helgaas) designware-plat: Add local struct device pointers (Bjorn Helgaas) designware-plat: Remove unused platform data (Bjorn Helgaas) TI DRA7xx host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove redundant struct device pointer from dra7xx_pcie (Bjorn Helgaas) Set drvdata at end of probe function (Bjorn Helgaas) Use generic DesignWare accessors (Bjorn Helgaas) Pass device-specific struct to internal functions (Bjorn Helgaas) Move struct pcie_port setup to probe function (Bjorn Helgaas) Reorder struct dra7xx_pcie (Bjorn Helgaas) TI Keystone host bridge driver Add local struct device pointers (Bjorn Helgaas) Use generic DesignWare accessors (Bjorn Helgaas) Pass keystone_pcie, not address, to IRQ functions (Bjorn Helgaas) Pass keystone_pcie, not va_app_base, to DBI functions (Bjorn Helgaas) Add app register accessors (Bjorn Helgaas) Reorder struct keystone_pcie (Bjorn Helgaas) Xilinx AXI host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) Removed unused xilinx_pcie_assign_msi() argument (Bjorn Helgaas) Xilinx NWL host bridge driver Add local struct device pointers (Bjorn Helgaas) Remove unused platform data (Bjorn Helgaas) ---------------------------------------------------------------- Bjorn Helgaas (107): PCI: designware: Rename dw_pcie_valid_config() to dw_pcie_valid_device() PCI: rcar: Add local struct device pointers PCI: xgene: Add local struct device pointers PCI: designware: Simplify pcie_host_ops.readl_rc() and .writel_rc() interfaces PCI: designware: Swap order of dw_pcie_writel_rc() reg/val arguments PCI: designware: Export dw_pcie_readl_rc(), dw_pcie_writel_rc() PCI: designware: Uninline register accessors PCI: designware: Swap order of dw_pcie_writel_unroll() reg/val arguments PCI: designware-plat: Remove redundant dw_plat_pcie.mem_base PCI: designware-plat: Add local struct device pointers PCI: designware-plat: Remove unused platform data PCI: spear: Remove unused constants PCI: spear: Pass device-specific struct to internal functions PCI: spear: Reorder struct spear13xx_pcie PCI: spear: Clean up struct device usage PCI: aardvark: Add local struct device pointers PCI: aardvark: Remove unused platform data PCI: altera: Add local struct device pointers PCI: altera: Remove unused platform data PCI: altera: Remove redundant platform_get_resource() return value check PCI: altera: Rename altera_pcie_valid_config() to altera_pcie_valid_device() PCI: altera: Simplify TLB_CFG_DW0 usage PCI: altera: Simplify TLP_CFG_DW1 usage PCI: armada: Remove unused platform data PCI: armada: Add local base pointer PCI: armada: Remove redundant struct armada8k_pcie.base PCI: armada: Use generic DesignWare accessors PCI: armada: Pass device-specific struct to internal functions PCI: armada: Reorder struct armada8k_pcie PCI: artpec6: Add local struct device pointers PCI: artpec6: Remove unused platform data PCI: artpec6: Add register accessors PCI: artpec6: Use generic DesignWare accessors PCI: artpec6: Remove unnecessary artpec6_pcie_link_up() PCI: artpec6: Pass device-specific struct to internal functions PCI: artpec6: Add resource name comments PCI: dra7xx: Add local struct device pointers PCI: dra7xx: Remove redundant struct device pointer from dra7xx_pcie PCI: dra7xx: Set drvdata at end of probe function PCI: dra7xx: Use generic DesignWare accessors PCI: dra7xx: Pass device-specific struct to internal functions PCI: dra7xx: Move struct pcie_port setup to probe function PCI: exynos: Add local struct device pointers PCI: exynos: Uninline register accessors PCI: exynos: Name private struct pointer "exynos_pcie" consistently PCI: exynos: Pass device-specific struct to internal functions PCI: exynos: Reorder struct exynos_pcie PCI: hisi: Add local struct device pointers PCI: hisi: Remove unused platform data PCI: hisi: Name private struct pointer "hisi_pcie" consistently PCI: hisi: Remove redundant struct hisi_pcie.reg_base PCI: hisi: Use generic DesignWare accessors PCI: imx6: Add local struct device pointers PCI: imx6: Remove redundant of_node pointer PCI: imx6: Removed unused struct imx6_pcie.mem_base PCI: imx6: Pass struct imx6_pcie to PHY accessors PCI: imx6: Pass device-specific struct to internal functions PCI: imx6: Use generic DesignWare accessors PCI: imx6: Reorder struct imx6_pcie PCI: imx6: Remove unused return values PCI: iproc: Add local struct device pointers PCI: iproc: Set drvdata at end of probe function PCI: iproc: Validate CSR base in BCMA setup code PCI: iproc: Remove redundant null pointer checking PCI: iproc: Hard-code PCIe capability offset instead of searching PCI: keystone: Add local struct device pointers PCI: keystone: Use generic DesignWare accessors PCI: keystone: Pass keystone_pcie, not address, to IRQ functions PCI: keystone: Pass keystone_pcie, not va_app_base, to DBI functions PCI: keystone: Add app register accessors PCI: keystone: Reorder struct keystone_pcie PCI: layerscape: Add local struct device pointers PCI: layerscape: Remove unused platform data PCI: layerscape: Remove redundant struct ls_pcie.dbi PCI: layerscape: Pass device-specific struct to internal functions PCI: layerscape: Move struct pcie_port setup to probe function PCI: layerscape: Remove unused ls_add_pcie_port() platform_device arg PCI: layerscape: Reorder struct ls_pcie PCI: mvebu: Add local struct device pointers PCI: mvebu: Use existing of_node pointer PCI: qcom: Remove unused platform data PCI: qcom: Remove redundant struct qcom_pcie.dbi PCI: qcom: Remove redundant struct qcom_pcie.dev PCI: qcom: Reorder struct qcom_pcie PCI: rcar: Remove unused platform data PCI: rcar: Remove unused rcar_pcie_get_resources() platform_device arg PCI: rcar: Remove DRV_NAME macro PCI: rcar-gen2: Add local struct device pointers PCI: rockchip: Remove unused platform data PCI: tegra: Fix argument order in tegra_pcie_phy_disable() PCI: tegra: Add local struct device pointers PCI: tegra: Remove unused platform data PCI: xgene: Remove unused platform data PCI: xgene: Pass struct xgene_pcie_port to setup functions PCI: xgene: Add register accessors PCI: xilinx: Add local struct device pointers PCI: xilinx: Remove unused platform data PCI: xilinx: Removed unused xilinx_pcie_assign_msi() argument PCI: xilinx-nwl: Add local struct device pointers PCI: xilinx-nwl: Remove unused platform data PCI: dra7xx: Reorder struct dra7xx_pcie PCI: hisi: Include register block base in PCIE_SYS_STATE4 address PCI: hisi: Pass device-specific struct to internal functions PCI: hisi: Reorder struct hisi_pcie Merge branches 'pci/host-aardvark', 'pci/host-altera', 'pci/host-iproc', 'pci/host-mvebu', 'pci/host-rcar', 'pci/host-rockchip', 'pci/host-tegra', 'pci/host-xgene' and 'pci/host-xilinx' into next Merge branch 'pci/host-designware' into next Merge branches 'pci/host-armada', 'pci/host-artpec', 'pci/host-dra7xx', 'pci/host-exynos', 'pci/host-hisi', 'pci/host-imx6', 'pci/host-keystone', 'pci/host-layerscape', 'pci/host-qcom' and 'pci/host-spear' into next Dan Carpenter (1): PCI: rockchip: Indent "if" statement body Kishon Vijay Abraham I (1): PCI: designware: Simplify dw_pcie_readl_unroll(), dw_pcie_writel_unroll() Thomas Petazzoni (2): MAINTAINERS: Add DT binding to the Aardvark PCIe driver maintainer MAINTAINERS: Add maintainer for the PCIe Marvell Armada 8K driver MAINTAINERS | 9 ++ drivers/pci/host/pci-aardvark.c | 39 ++--- drivers/pci/host/pci-dra7xx.c | 103 ++++++------- drivers/pci/host/pci-exynos.c | 220 +++++++++++++--------------- drivers/pci/host/pci-imx6.c | 250 ++++++++++++++++---------------- drivers/pci/host/pci-keystone-dw.c | 123 +++++++++------- drivers/pci/host/pci-keystone.c | 28 ++-- drivers/pci/host/pci-keystone.h | 9 +- drivers/pci/host/pci-layerscape.c | 65 +++++---- drivers/pci/host/pci-mvebu.c | 21 ++- drivers/pci/host/pci-rcar-gen2.c | 46 +++--- drivers/pci/host/pci-tegra.c | 237 +++++++++++++++--------------- drivers/pci/host/pci-xgene.c | 151 ++++++++++--------- drivers/pci/host/pcie-altera.c | 77 +++++----- drivers/pci/host/pcie-armada8k.c | 78 +++++----- drivers/pci/host/pcie-artpec6.c | 115 ++++++++------- drivers/pci/host/pcie-designware-plat.c | 25 ++-- drivers/pci/host/pcie-designware.c | 102 ++++++------- drivers/pci/host/pcie-designware.h | 7 +- drivers/pci/host/pcie-hisi.c | 86 +++++------ drivers/pci/host/pcie-iproc-bcma.c | 14 +- drivers/pci/host/pcie-iproc-platform.c | 27 ++-- drivers/pci/host/pcie-iproc.c | 52 +++---- drivers/pci/host/pcie-qcom.c | 30 ++-- drivers/pci/host/pcie-rcar.c | 98 +++++++------ drivers/pci/host/pcie-rockchip.c | 4 +- drivers/pci/host/pcie-spear13xx.c | 108 +++----------- drivers/pci/host/pcie-xilinx-nwl.c | 109 +++++++------- drivers/pci/host/pcie-xilinx.c | 62 ++++---- 29 files changed, 1108 insertions(+), 1187 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html