Re: [PATCH v3 2/2] PCI: tegra: Support per-lane PHYs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/08/2016 08:48 AM, Thierry Reding wrote:
From: Thierry Reding <treding@xxxxxxxxxx>

The current XUSB pad controller bindings are insufficient to describe
PHY devices attached to USB controllers. New bindings have been created
to overcome these restrictions. As a side-effect each root port now is
assigned a set of PHY devices, one for each lane associated with the
root port. This has the benefit of allowing fine-grained control of the
power management for each lane.

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c

+static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port)
+{
+	struct device *dev = port->pcie->dev;
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < port->lanes; i++) {
+		err = phy_power_on(port->phys[i]);

This assume the number of array entries is precisely the number of lanes. That seems to contradict the binding update which said the number might not match. Perhaps there's an expectation that phy_power_on() is a no-op for some "invalid" values like NULL or an error-pointer value? But...

+static struct phy *devm_of_phy_optional_get_index(struct device *dev,
+						  struct device_node *np,
+						  const char *consumer,
+						  unsigned int index)
+{
+	struct phy *phy;
+	char *name;
+
+	name = kasprintf(GFP_KERNEL, "%s-%u", consumer, index);
+	if (!name)
+		return ERR_PTR(-ENOMEM);
+
+	phy = devm_of_phy_get(dev, np, name);
+	kfree(name);
+
+	if (IS_ERR(phy) && PTR_ERR(phy) == -ENODEV)
+		phy = NULL;
+
+	return phy;
+}

The error-handling there looks wrong. The function generally returns either a valid PHY or an error pointer. However, in the case of -ENODEV, NULL is returned. Subsystems are supposed to encode their handles as, and functions are supposed to return, either NULL or an error pointer for error cases, not both/either. Is the PHY API broken in this regard? If so, then this code is fine, but if not it might need a fix.

Aside from that, this patch looks fine.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux