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 linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
