Hi Rick,
Thanks very much for your work.
On Thu, Jan 26, 2023 at 02:50:40PM +0100, Rick Wertenbroek wrote:
> This is a series of patches that fixes the PCIe endpoint controller driver
> for the Rockchip RK3399 SoC. It is based on Linux kernel 6.0.19
>
> The original driver in mainline had issues and would not allow for the
> RK3399 to operate in PCIe endpoint mode. This patch series fixes that so
> that the PCIe core controller of the RK3399 SoC can now act as a PCIe
> endpoint.
So we merged cf590b078391 ("PCI: rockchip: Add EP driver for Rockchip
PCIe controller") when it actually didn't work? Ouch. Thanks for
fixing it and testing it.
> Rick Wertenbroek (8):
> PCI: rockchip: Removed writes to unused registers
> PCI: rockchip: Fixed setup of Device ID
> PCI: rockchip: Fixed endpoint controller Configuration Request Retry
> Status
> PCI: rockchip: Added poll and timeout to wait for PHY PLLs to be
> locked
> PCI: rockchip: Added dtsi entry for PCIe endpoint controller
> PCI: rockchip: Fixed window mapping and address translation for
> endpoint
> PCI: rockchip: Fixed legacy IRQ generation for endpoint
> PCI: rockchip: Fixed MSI generation from PCIe endpoint core
For the next iteration, can you please update these subject lines and
commit logs to:
- Use imperative mood, i.e., read like a command, instead of a past
tense description of what was done. For example, say "Remove
writes to unused registers" instead of "Removed writes ..."
- Be more specific when possible. "Fix" conveys no information
about the actual code change. For example, "Fixed endpoint
controller Configuration Request Retry Status" gives a general
idea, but it would be more useful if it said something about
clearing config mode after probe.
- Say what the patch does in the commit log. The current ones often
describe a *problem*, but do not explicitly say what the patch
does. The commit log should be complete in itself even without
the subject line, so it usually contains a slightly expanded
version of the subject line.
- Split patches that do more than one logical thing. The commit log
for "Fixed MSI generation ..." talks about a u16/u32 shift issue,
but the patch also adds an unrelated check for multi-function
devices.
- If a patch is a fix for an existing issue and may need to be
backported, identify the commit that introduced the issue and add
"Fixes: " lines. This helps distros figure out whether and how
far to backport patches.
- Refer to the device consistently. I see:
RK3399 PCI EP core
RK3399 SoC PCIe EP core
RK3399 PCIe endpoint core
I vote for "RK3399 PCIe Endpoint core".
Notes about imperative mood:
https://chris.beams.io/posts/git-commit/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.0#n94
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++
> drivers/pci/controller/pcie-rockchip-ep.c | 149 +++++++++++-----------
> drivers/pci/controller/pcie-rockchip.c | 16 +++
> drivers/pci/controller/pcie-rockchip.h | 36 ++++--
> 4 files changed, 137 insertions(+), 89 deletions(-)
>
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
