From patchwork Fri Mar 11 23:54:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 8570511 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id AEB00C0553 for ; Fri, 11 Mar 2016 23:54:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A93E02035D for ; Fri, 11 Mar 2016 23:54:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A975920260 for ; Fri, 11 Mar 2016 23:54:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751756AbcCKXyd (ORCPT ); Fri, 11 Mar 2016 18:54:33 -0500 Received: from mail.kernel.org ([198.145.29.136]:43959 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751407AbcCKXyc (ORCPT ); Fri, 11 Mar 2016 18:54:32 -0500 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1B74E2034B; Fri, 11 Mar 2016 23:54:31 +0000 (UTC) Received: from localhost (unknown [69.71.1.1]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BC03020260; Fri, 11 Mar 2016 23:54:29 +0000 (UTC) Date: Fri, 11 Mar 2016 17:54:27 -0600 From: Bjorn Helgaas To: Thierry Reding Cc: Bjorn Helgaas , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Stephen Warren , Alexandre Courbot , linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-tegra@vger.kernel.org Subject: Re: [PATCH v3 2/2] PCI: tegra: Support per-lane PHYs Message-ID: <20160311235427.GI4725@localhost> References: <1457452094-5409-1-git-send-email-thierry.reding@gmail.com> <1457452094-5409-2-git-send-email-thierry.reding@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1457452094-5409-2-git-send-email-thierry.reding@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Stephen, you had some comments on the previous version. What do you think about this one? On Tue, Mar 08, 2016 at 04:48:14PM +0100, Thierry Reding wrote: > From: Thierry Reding > > 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. > > Signed-off-by: Thierry Reding > ... > static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > { > const struct tegra_pcie_soc_data *soc = pcie->soc_data; > @@ -883,14 +905,24 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > afi_writel(pcie, value, AFI_FUSE); > } > > - if (!pcie->phy) > - err = tegra_pcie_phy_enable(pcie); > - else > - err = phy_power_on(pcie->phy); > + if (!pcie->legacy_phy) { > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_phy_power_on(port); > + if (err < 0) { > + dev_err(pcie->dev, > + "failed to power on PCIe port: %d\n", > + err); > + return err; > + } > + } > + } else { > + if (!pcie->phy) > + err = tegra_pcie_phy_enable(pcie); > + else > + err = phy_power_on(pcie->phy); > > - if (err < 0) { > - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); > - return err; > + if (err < 0) > + dev_err(pcie->dev, "failed to power on PHY: %d\n", err); In the legacy_phy case, we used to bail out if tegra_pcie_phy_enable() or phy_power_on() failed, but now we don't. I assume this is an unintentional change. I would personally write this as follows because I hate reading "if (!xxx) ... else ..." (this patch applies on top of the one you posted). Note that this restores the legacy_phy error path also. --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index 625db7d..139b9ca 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -882,6 +882,31 @@ static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port) return 0; } +static int tegra_pcie_port_phy_enable(struct tegra_pcie *pcie) +{ + if (pcie->legacy_phy) { + if (pcie->phy) + err = phy_power_on(pcie->phy); + else + err = tegra_pcie_phy_enable(pcie); + + if (err < 0) + dev_err(pcie->dev, "failed to power on PHY: %d\n", err); + + return err; + } + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_phy_power_on(port); + if (err < 0) { + dev_err(pcie->dev, "failed to power on PCIe port: %d\n", + err); + return err; + } + } + return 0; +} + static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) { const struct tegra_pcie_soc_data *soc = pcie->soc_data; @@ -921,25 +946,9 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) afi_writel(pcie, value, AFI_FUSE); } - if (!pcie->legacy_phy) { - list_for_each_entry(port, &pcie->ports, list) { - err = tegra_pcie_port_phy_power_on(port); - if (err < 0) { - dev_err(pcie->dev, - "failed to power on PCIe port: %d\n", - err); - return err; - } - } - } else { - if (!pcie->phy) - err = tegra_pcie_phy_enable(pcie); - else - err = phy_power_on(pcie->phy); - - if (err < 0) - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); - } + err = tegra_pcie_port_phy_enable(pcie); + if (err < 0) + return err; /* take the PCIe interface module out of reset */ reset_control_deassert(pcie->pcie_xrst);