Re: [PATCH v3 5/5] PCI: add PCI controller for Keystone PCIe h/w

On Tuesday, July 01, 2014 6:45 AM, Murali Karicheri wrote:

Keystone PCIe controller is based on v3.65 version of the
designware h/w. Main differences are
	1. No ATU support
	2. Legacy and MSI irq functions are implemented in
	   application register space
	3. MSI interrupts are multiplexed over 8 IRQ lines to the Host
	   side.
All of the Application register space handing code are organized into
pci-keystone-dw.c and the functions are called from pci-keystone.c
to implement PCI controller driver. Also add necessary DT documentation
for the driver.

Signed-off-by: Murali Karicheri <redacted>

CC: Santosh Shilimkar <redacted>
CC: Russell King <redacted>
CC: Grant Likely <redacted>
CC: Rob Herring <robh+dt@kernel.org>
CC: Mohit Kumar <redacted>
CC: Jingoo Han <redacted>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Pratyush Anand <redacted>
CC: Richard Zhu <redacted>
CC: Kishon Vijay Abraham I <redacted>
CC: Marek Vasut <marex@denx.de>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Pawel Moll <redacted>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Ian Campbell <redacted>
CC: Kumar Gala <redacted>
CC: Randy Dunlap <redacted>
CC: Grant Likely <redacted>
---
 .../devicetree/bindings/pci/pci-keystone.txt       |   69 +++
 drivers/pci/host/Kconfig                           |    5 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pci-keystone-dw.c                 |  523 ++++++++++++++++++++
 drivers/pci/host/pci-keystone.c                    |  381 ++++++++++++++
 drivers/pci/host/pci-keystone.h                    |   56 +++
 6 files changed, 1035 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
 create mode 100644 drivers/pci/host/pci-keystone-dw.c
 create mode 100644 drivers/pci/host/pci-keystone.c
 create mode 100644 drivers/pci/host/pci-keystone.h

[.....]

+
+static int __init ks_pcie_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct keystone_pcie *ks_pcie;
+	struct pcie_port *pp;
+	struct resource *res;
+	void __iomem *reg_p;
+	struct phy *phy;
+	int ret = 0;
+	u32 val;
+
+	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
+				GFP_KERNEL);
+	if (!ks_pcie) {
+		dev_err(dev, "no memory for keystone pcie\n");
+		return -ENOMEM;
+	}
+	pp = &ks_pcie->pp;
+
+	/* initialize SerDes Phy if present */
+	phy = devm_phy_get(dev, "pcie-phy");
+	if (phy)
+		ret = phy_init(phy);

Why don't you check 'ret' value?
How about the following?

+	/* initialize SerDes Phy if present */
+	phy = devm_phy_get(dev, "pcie-phy");
+	if (!IS_ERR_OR_NULL(phy) {
+		ret = phy_init(phy);
+		if (ret <0)
+			return ret;
+	}


Best regards,
Jingoo Han

[....]