From patchwork Fri Aug 25 21:18:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 9922859 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E8F4E6022E for ; Fri, 25 Aug 2017 21:19:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DAEA3223C6 for ; Fri, 25 Aug 2017 21:19:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CFAAE2844B; Fri, 25 Aug 2017 21:19:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_LOW autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3AE3B223C6 for ; Fri, 25 Aug 2017 21:19:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=qQQiZfwNwIEFklmQhvjxaAN4kpQC5Z50xHAAlPRpPIQ=; b=H+9yeajsLnaU5/ DwW9fru3bKn34qotn8nvSSl4M5LbMX3z5XPeXY0uL2eWcsAocc1LlshcLwginBtgLi27fOObIHgUu qPp3yLwjhtD4qAfYjiJBA3wTZllxpeZb3ZkLy+lM21z51/QhU9tezoc1HYaOpg2NPtm+Tcrn9HN+v XPjRsTlYwORgdY4zFdZVD+J3mtBZXGBQCSjpzd6nYqYxFT+bG1Fk8qCR2bIWwMmtUZwbpjRnnc2tY TgqNIuWF+OurLevVR/8eF0MH3q8zN9TeOdJ8BL2R6o5va15gyWDF2ftCH1A9NWzdqObcwUaA3sD5V mKDLbarN5KPVAcS4Yd2A==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dlM0N-0001jx-EL; Fri, 25 Aug 2017 21:19:03 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dlM0K-0001aH-D0 for linux-rockchip@lists.infradead.org; Fri, 25 Aug 2017 21:19:02 +0000 Received: from localhost (unknown [69.71.4.159]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BE2D1219AA; Fri, 25 Aug 2017 21:18:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BE2D1219AA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Fri, 25 Aug 2017 16:18:32 -0500 From: Bjorn Helgaas To: Shawn Lin Subject: Re: [PATCH v5 06/10] PCI: rockchip: fix missing phy manipulation for legacy phy Message-ID: <20170825211832.GA8154@bhelgaas-glaptop.roam.corp.google.com> References: <1503471673-69478-1-git-send-email-shawn.lin@rock-chips.com> <1503471777-73999-1-git-send-email-shawn.lin@rock-chips.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1503471777-73999-1-git-send-email-shawn.lin@rock-chips.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170825_141900_490621_90F30DA3 X-CRM114-Status: GOOD ( 22.47 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Bjorn Helgaas , linux-pci@vger.kernel.org, Jeffy Chen , Brian Norris , linux-rockchip@lists.infradead.org Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Aug 23, 2017 at 03:02:57PM +0800, Shawn Lin wrote: > For instance, if a EP connect to lane3 and work under lagecy > phy mode, so struct phy phys[0..2] are all NULL. In this case, > rockchip->lanes_map & BIT(i) will tell the driver that lane0 is > already inactive, but what we want actually is to power off > the phys[0] for legacy phy mode. Fix this by add checking of > rockchip->legacy_phy for rockchip_pcie_deinit_phys. This changelog is not quite correct. If "rockchip->legacy_phy", then rockchip->phys[0] is a valid PHY, but phys[1..3] are NULL (not 0..2). > Signed-off-by: Shawn Lin > --- > > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/pci/host/pcie-rockchip.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c > index 9cd51e0..933e3e9 100644 > --- a/drivers/pci/host/pcie-rockchip.c > +++ b/drivers/pci/host/pcie-rockchip.c > @@ -759,7 +759,7 @@ static void rockchip_pcie_deinit_phys(struct rockchip_pcie *rockchip) > > for (i = 0; i < MAX_LANE_NUM; i++) { > /* inactive lane is already powered off */ > - if (rockchip->lanes_map & BIT(i)) > + if (rockchip->legacy_phy || rockchip->lanes_map & BIT(i)) > phy_power_off(rockchip->phys[i]); > phy_exit(rockchip->phys[i]); > } I think this is harder to understand than necessary. If we're using legacy_phy, the pointer is in phys[0]. If we always set rockchip->lanes_map, even in the legacy_phy case (where it would show that only phys[0] is valid), this patch won't even be necessary. I'd propose the following patches, which could be squashed into the existing series on pci/host-rockchip. The first is purely cosmetic, as is some of the second. The important part is this: static u8 rockchip_pcie_lane_map(struct rockchip_pcie *rockchip) { - u32 val = rockchip_pcie_read(rockchip, PCIE_CORE_LANE_MAP); - u8 map = val & PCIE_CORE_LANE_MAP_MASK; + u32 val; + u8 map; + + if (rockchip->legacy_phy) + return BIT(0); commit d3d39c577edf63b9441d1a7614808e02721dd2b6 Author: Bjorn Helgaas Date: Fri Aug 25 16:00:25 2017 -0500 Possibly squash into "PCI: rockchip: Add per-lane PHY support" diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c index f8b88004e20f..5ccbdbfa97d0 100644 --- a/drivers/pci/host/pcie-rockchip.c +++ b/drivers/pci/host/pcie-rockchip.c @@ -867,24 +867,25 @@ static int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip) char *name; u32 i; - rockchip->phys[0] = devm_phy_get(dev, "pcie-phy"); - if (IS_ERR(rockchip->phys[0])) { - if (PTR_ERR(rockchip->phys[0]) == -EPROBE_DEFER) - return PTR_ERR(rockchip->phys[0]); - dev_dbg(dev, "missing legacy phy; search for per-lane PHY\n"); - } else { + phy = devm_phy_get(dev, "pcie-phy"); + if (!IS_ERR(phy)) { rockchip->legacy_phy = true; + rockchip->phys[0] = phy; dev_warn(dev, "legacy phy model is deprecated!\n"); return 0; } + if (PTR_ERR(phy) == -EPROBE_DEFER) + return PTR_ERR(phy); + + dev_dbg(dev, "missing legacy phy; search for per-lane PHY\n"); + for (i = 0; i < MAX_LANE_NUM; i++) { name = kasprintf(GFP_KERNEL, "pcie-phy-%u", i); if (!name) return -ENOMEM; - phy = devm_of_phy_get(rockchip->dev, - rockchip->dev->of_node, name); + phy = devm_of_phy_get(dev, dev->of_node, name); kfree(name); if (IS_ERR(phy)) { commit 6f8bcdfe4568809437e93e2d54e68b2cba3b4ac4 Author: Bjorn Helgaas Date: Fri Aug 25 15:39:10 2017 -0500 Possibly squash into "PCI: rockchip: Idle inactive PHY(s)" diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c index 60069acd9f86..29ebfc971896 100644 --- a/drivers/pci/host/pcie-rockchip.c +++ b/drivers/pci/host/pcie-rockchip.c @@ -309,8 +309,14 @@ static int rockchip_pcie_valid_device(struct rockchip_pcie *rockchip, static u8 rockchip_pcie_lane_map(struct rockchip_pcie *rockchip) { - u32 val = rockchip_pcie_read(rockchip, PCIE_CORE_LANE_MAP); - u8 map = val & PCIE_CORE_LANE_MAP_MASK; + u32 val; + u8 map; + + if (rockchip->legacy_phy) + return BIT(0); + + val = rockchip_pcie_read(rockchip, PCIE_CORE_LANE_MAP); + map = val & PCIE_CORE_LANE_MAP_MASK; /* The link may be using a reverse-indexed mapping. */ if (val & PCIE_CORE_LANE_MAP_REVERSE) @@ -715,13 +721,10 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) PCIE_CORE_PL_CONF_LANE_SHIFT); dev_dbg(dev, "current link width is x%d\n", status); - if (!rockchip->legacy_phy) { - /* power off unused lane(s) */ - rockchip->lanes_map = rockchip_pcie_lane_map(rockchip); - for (i = 0; i < MAX_LANE_NUM; i++) { - if (rockchip->lanes_map & BIT(i)) - continue; - + /* Power off unused lane(s) */ + rockchip->lanes_map = rockchip_pcie_lane_map(rockchip); + for (i = 0; i < MAX_LANE_NUM; i++) { + if (!(rockchip->lanes_map & BIT(i))) { dev_dbg(dev, "idling lane %d\n", i); phy_power_off(rockchip->phys[i]); } @@ -1378,7 +1381,7 @@ static int __maybe_unused rockchip_pcie_suspend_noirq(struct device *dev) } for (i = 0; i < MAX_LANE_NUM; i++) { - /* inactive lane is already powered off */ + /* inactive lanes are already powered off */ if (rockchip->lanes_map & BIT(i)) phy_power_off(rockchip->phys[i]); phy_exit(rockchip->phys[i]); @@ -1628,7 +1631,7 @@ static int rockchip_pcie_remove(struct platform_device *pdev) irq_domain_remove(rockchip->irq_domain); for (i = 0; i < MAX_LANE_NUM; i++) { - /* inactive lane is already powered off */ + /* inactive lanes are already powered off */ if (rockchip->lanes_map & BIT(i)) phy_power_off(rockchip->phys[i]); phy_exit(rockchip->phys[i]);