RE: [PATCH v2 06/10] PCI: layerscape: Modify the way of getting capability... | linux-arm-kernel

quoted

-----Original Message-----
From: Andrew Murray <redacted>
Sent: 2019年9月2日 21:37
To: Xiaowei Bao <redacted>
Cc: Kishon Vijay Abraham I <redacted>; bhelgaas@google.com;
robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
[off-list ref]; lorenzo.pieralisi@arm.co
[off-list ref]; arnd@arndb.de; gregkh@linuxfoundation.org;
M.h. Lian [off-list ref]; Mingkai Hu [off-list ref];
Roy Zang [off-list ref]; jingoohan1@gmail.com;
gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 06/10] PCI: layerscape: Modify the way of getting
capability with different PEX

On Fri, Aug 23, 2019 at 04:13:30AM +0000, Xiaowei Bao wrote:

-----Original Message-----
From: Kishon Vijay Abraham I <redacted>
Sent: 2019年8月23日 11:40
To: Xiaowei Bao <redacted>; bhelgaas@google.com;
robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org; Leo
robh+Li
[off-list ref]; lorenzo.pieralisi@arm.co
[off-list ref]; arnd@arndb.de;
gregkh@linuxfoundation.org; M.h. Lian [off-list ref];
Mingkai Hu [off-list ref]; Roy Zang [off-list ref];
jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linuxppc-dev@lists.ozlabs.org; andrew.murray@arm.com
Subject: Re: [PATCH v2 06/10] PCI: layerscape: Modify the way of
getting capability with different PEX

Hi,

(Fixed Lorenzo's email address. All the patches in the series have
wrong email
id)

On 23/08/19 8:09 AM, Xiaowei Bao wrote:

-----Original Message-----
From: Kishon Vijay Abraham I <redacted>
Sent: 2019年8月22日 19:44
To: Xiaowei Bao <redacted>; bhelgaas@google.com;
robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
robh+Leo
Li
[off-list ref]; lorenzo.pieralisi@arm.co; arnd@arndb.de;
gregkh@linuxfoundation.org; M.h. Lian [off-list ref];
Mingkai Hu [off-list ref]; Roy Zang [off-list ref];
jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org;
linux-arm-kernel@lists.infradead.org;
linuxppc-dev@lists.ozlabs.org; andrew.murray@arm.com
Subject: Re: [PATCH v2 06/10] PCI: layerscape: Modify the way of
getting capability with different PEX

Hi,

On 22/08/19 4:52 PM, Xiaowei Bao wrote:
The different PCIe controller in one board may be have different
capability of MSI or MSIX, so change the way of getting the MSI
capability, make it more flexible.
please use different pci_epc_features table for different boards.
Thanks, I think that it will be more flexible to dynamically get
MSI or MSIX capability, Thus, we will not need to define the
pci_epc_feature for
different boards.

Is the restriction because you cannot have different compatible for
different boards?
Sorry, I am not very clear what your mean, I think even if I use the
same compatible with different boards, each boards will enter the
probe function, in there I will get the MSI or MSIX PCIe capability of
the current controller in this board. Why do I need to define the
pci_epc_feature for different boards?

At present you determine how to set the [msi,msix]_capable flags of
pci_epc_features based on reading the function capabilities at probe time.
Instead of doing this, is it possible that you can determine the flags based on
the compatible type alone? For example, is the MSI/MSIX capability the same
for all fsl,ls2088a-pcie-ep devices?

If it isn't *necessary* to probe for this information at probe time, then you
could instead create a static pci_epc_features structure and assign it to
something in your drvdata. This may provide some benefits.

The dw_pcie_ep_get_features function would then look like:

static const struct pci_epc_features*
ls_pcie_ep_get_features(struct dw_pcie_ep *ep) {
	struct dw_pcie *pci = to_dw_pcie_from_pp(ep);
	struct ls_pcie_ep *pcie = dev_get_drvdata(pci->dev);
	return pcie->epc_features;
}

This also means you can revert "[v3,03/11] PCI: designware-ep: Move the".

Is this what you had in mind Kishon?
Yes, I consider this scheme, but there is a issue with my board, e.g. my board have
three PCIE controllers, but only two controllers support MSI, I can't said that the 
board support the MSI feature, so I only set the msi_capabitily by reading the MSI
capability struct the current PCIE controller, I am also very entangled in this issue.
so, do you have better advice? Thanks a lot.

Thanks 
Xiaowei

Thanks,

Andrew Murray

Thanks
Kishon

Thanks
Kishon
Signed-off-by: Xiaowei Bao <redacted>
---
v2:
 - Remove the repeated assignment code.

 drivers/pci/controller/dwc/pci-layerscape-ep.c | 26
+++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 4e92a95..8461f62 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -22,6 +22,7 @@

 struct ls_pcie_ep {
 	struct dw_pcie		*pci;
+	struct pci_epc_features	*ls_epc;
 };

 #define to_ls_pcie_ep(x)	dev_get_drvdata((x)->dev)
@@ -40,25 +41,26 @@ static const struct of_device_id
ls_pcie_ep_of_match[] = {
 	{ },
 };

-static const struct pci_epc_features ls_pcie_epc_features = {
-	.linkup_notifier = false,
-	.msi_capable = true,
-	.msix_capable = false,
-};
-
 static const struct pci_epc_features*
ls_pcie_ep_get_features(struct dw_pcie_ep *ep)  {
-	return &ls_pcie_epc_features;
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
+
+	return pcie->ls_epc;
 }

 static void ls_pcie_ep_init(struct dw_pcie_ep *ep)  {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
 	enum pci_barno bar;

 	for (bar = BAR_0; bar <= BAR_5; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
+
+	pcie->ls_epc->msi_capable = ep->msi_cap ? true : false;
+	pcie->ls_epc->msix_capable = ep->msix_cap ? true : false;
 }

 static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8
func_no, @@
-118,6 +120,7 @@ static int __init ls_pcie_ep_probe(struct
platform_device
*pdev)
 	struct device *dev = &pdev->dev;
 	struct dw_pcie *pci;
 	struct ls_pcie_ep *pcie;
+	struct pci_epc_features *ls_epc;
 	struct resource *dbi_base;
 	int ret;

@@ -129,6 +132,10 @@ static int __init ls_pcie_ep_probe(struct
platform_device *pdev)
 	if (!pci)
 		return -ENOMEM;

+	ls_epc = devm_kzalloc(dev, sizeof(*ls_epc), GFP_KERNEL);
+	if (!ls_epc)
+		return -ENOMEM;
+
 	dbi_base = platform_get_resource_byname(pdev,
IORESOURCE_MEM,
"regs");
 	pci->dbi_base = devm_pci_remap_cfg_resource(dev,
dbi_base);
 	if (IS_ERR(pci->dbi_base))
@@ -139,6 +146,11 @@ static int __init ls_pcie_ep_probe(struct
platform_device *pdev)
 	pci->ops = &ls_pcie_ep_ops;
 	pcie->pci = pci;

+	ls_epc->linkup_notifier = false,
+	ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
+
+	pcie->ls_epc = ls_epc;
+
 	platform_set_drvdata(pdev, pcie);

 	ret = ls_add_pcie_ep(pcie, pdev);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help