Re: [PATCH V1 RESEND 1/4] pci: add interface to create pci-ep device tree node

From: Bjorn Helgaas <helgaas@kernel.org>
To: Lizhi Hou <lizhi.hou@xilinx.com>
Cc: linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	robh@kernel.org, yilun.xu@intel.com, maxz@xilinx.com,
	sonal.santan@xilinx.com, yliu@xilinx.com,
	michal.simek@xilinx.com, stefanos@xilinx.com, trix@redhat.com,
	mdf@kernel.org, dwmw2@infradead.org,
	linux-kernel@vger.kernel.org, Max Zhen <max.zhen@xilinx.com>
Subject: Re: [PATCH V1 RESEND 1/4] pci: add interface to create pci-ep device tree node
Date: Thu, 10 Mar 2022 13:34:58 -0600	[thread overview]
Message-ID: <20220310193458.GA167650@bhelgaas> (raw)
In-Reply-To: <20220305052304.726050-2-lizhi.hou@xilinx.com>

Run "git log --oneline drivers/pci/of.c" and follow the convention,
e.g., something like:

  PCI: Add DT Endpoint description interfaces

On Fri, Mar 04, 2022 at 09:23:01PM -0800, Lizhi Hou wrote:
> This patch enables PCIe device to uses flattened device tree to describe
> apertures in its PCIe BARs. The aperture address consists of PCIe BAR index
> and offset.
> 
> For this kind of device, the driver probe routine calls the new added
> interface to create a device tree node. This device tree node is attached
> under system device tree root. Then the driver may load the flatten device
> tree overlay and attach it under this node. And the node also contains
> 'ranges' property which is used to translate aperture address(BAR index
> and offset) to CPU address.

In the commit log, please say *what* this patch does and why we need
it.  The current text talks about how some interface might be used,
but doesn't specifically say what interface that is or that this patch
adds it.

It should also have a specific pointer to the relevant DT binding.

> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>  drivers/pci/of.c       | 180 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_pci.h |  15 ++++
>  2 files changed, 195 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index cb2e8351c2cc..198f08351070 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -605,6 +605,186 @@ int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
>  	return pci_parse_request_of_pci_ranges(dev, bridge);
>  }
>  
> +#if IS_ENABLED(CONFIG_OF_DYNAMIC)
> +
> +static void devm_of_pci_destroy_bus_endpoint(struct device *dev, void *res)
> +{
> +	struct device_node *node = res;
> +
> +	of_detach_node(node);
> +}
> +
> +static int of_ep_add_property(struct device *dev, struct property **proplist, const char *name,

Please rewrap code and comments to fit in 80 columns like the rest of
the file.  There's a lot more below that I snipped out.

Bjorn