[{"id":1786441,"web_url":"http://patchwork.ozlabs.org/comment/1786441/","msgid":"<20171013163808.GB8761@ulmo>","list_archive_url":null,"date":"2017-10-13T16:38:08","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n> UPHY programming is performed by BPMP, PHY enable calls are\n> not required for Tegra186 PCIe. Power partition ungate is\n> done by BPMP powergate driver.\n> \n> Signed-off-by: Manikanta Maddireddy \n> Reviewed-by: Mikko Perttunen \n> Tested-by: Mikko Perttunen \n> ---\n> V2: Add soc->program_uphy check for phy_exit call\n> drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n> 1 file changed, 108 insertions(+), 24 deletions(-)\n\nThis works, excellent!\n\nTested-by: Thierry Reding \nAcked-by: Thierry Reding ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"iudcAxPs\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yDD1362pRz9sRm\n\tfor ;\n\tSat, 14 Oct 2017 03:38:15 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751502AbdJMQiN (ORCPT );\n\tFri, 13 Oct 2017 12:38:13 -0400","from mail-wm0-f67.google.com ([74.125.82.67]:50797 \"EHLO\n\tmail-wm0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752333AbdJMQiL (ORCPT\n\t); Fri, 13 Oct 2017 12:38:11 -0400","by mail-wm0-f67.google.com with SMTP id u138so22569202wmu.5;\n\tFri, 13 Oct 2017 09:38:11 -0700 (PDT)","from localhost\n\t(p200300E41BE4FD00CEAD5B94E1CFD280.dip0.t-ipconnect.de.\n\t[2003:e4:1be4:fd00:cead:5b94:e1cf:d280])\n\tby smtp.gmail.com with ESMTPSA id\n\tv5sm1186398wrf.29.2017.10.13.09.38.09\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 13 Oct 2017 09:38:09 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=/lwrWCuElFiRPJ7jLpEZT0XN9vHFHyKfo9xJSs+QuWA=;\n\tb=iudcAxPsrizO3gzR6jY84r0CjN2QZev20ti3KlB8W4FFT6bWOzmjTxtVAwXPfUejae\n\tQnsmqzaCyyCj7QTWGQCC+Jr4v+1cw4MvSW2MAU34NB7tZEiUGvRHWOp0XmLc3IEq6CBI\n\tN7wN+moSzPFf6hC3vkoW7gBUYvaOyWy+WOm/pHicE+rwJCu149nz69YyriRKKcgFRgea\n\t3g23MvNIhSRG+zctkCALVN478VK/nNwCaM3bDfMOTxKlHnZQnok0xuHHiaOuhmkNkv1A\n\tpIHR9RIKSe5jTj0nIGGOUPFNfGMVLVfs0N/+cpuQePIZy10+7i4eIMqB7NMbOZeQ1F0Y\n\tSwJA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=/lwrWCuElFiRPJ7jLpEZT0XN9vHFHyKfo9xJSs+QuWA=;\n\tb=rgigbtRsJZYwUCNkHPnuNGOzoiE1i1wVahatL2veLAwCL3M5OMCjgSRv0JODg32tXR\n\tbcA04NVvOPOVWRGDHAB87kM/7TTDpz30GS7Rj+Lb/uxHvP4yjPazl7bS49jwzgSopdyL\n\tNb4mbSPZfBUtJneg1Y7TiC66/rrjcwSsUcPCD/ZntUzdv5dnnRfkj/mfvv9D0NGNraF2\n\tPSCx1pdCTgt68Ct92StAuxLpytFNEiL2wlsKKgax/b53mg7whZsssPs1T1PGsN4wHmoa\n\t+8x4ubCaMtGWqmk1wry5zJnvlcmDFy3xxybTYBRRb0K2Pk/76ixrMkE7Ygui0aWDQT3I\n\tp+ug==","X-Gm-Message-State":"AMCzsaWSxCMPPcpB4Z/JxDo4hUlWfSfXC7TfbD03tw+Z6GLPMpkezTxo\n\tJWvK7U35XMC2m24f35FiJz0=","X-Google-Smtp-Source":"ABhQp+T2UXYjPd5RhKunAtSZWzniFLV6Epnd8e7L2aiq3MB1XX2eHr1ri9txhltN03lUMAFhgqBaQA==","X-Received":"by 10.28.69.8 with SMTP id s8mr1721241wma.65.1507912690102;\n\tFri, 13 Oct 2017 09:38:10 -0700 (PDT)","Date":"Fri, 13 Oct 2017 18:38:08 +0200","From":"Thierry Reding ","To":"Manikanta Maddireddy ","Cc":"bhelgaas@google.com, jonathanh@nvidia.com,\n\tlinux-tegra@vger.kernel.org, linux-pci@vger.kernel.org,\n\tmperttunen@nvidia.com","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","Message-ID":"<20171013163808.GB8761@ulmo>","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"kXdP64Ggrk/fb43R\"","Content-Disposition":"inline","In-Reply-To":"<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1788698,"web_url":"http://patchwork.ozlabs.org/comment/1788698/","msgid":"<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-10-17T17:34:28","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n> UPHY programming is performed by BPMP, PHY enable calls are\n> not required for Tegra186 PCIe. Power partition ungate is\n> done by BPMP powergate driver.\n> \n> Signed-off-by: Manikanta Maddireddy \n> Reviewed-by: Mikko Perttunen \n> Tested-by: Mikko Perttunen \n> ---\n> V2: Add soc->program_uphy check for phy_exit call\n> drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n> 1 file changed, 108 insertions(+), 24 deletions(-)\n\n> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> \t\tafi_writel(pcie, value, AFI_FUSE);\n> \t}\n> \n> -\terr = tegra_pcie_phy_power_on(pcie);\n> -\tif (err < 0) {\n> -\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> -\t\treturn err;\n> +\tif (soc->program_uphy) {\n> +\t\terr = tegra_pcie_phy_power_on(pcie);\n> +\t\tif (err < 0) {\n> +\t\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> +\t\t\treturn err;\n> +\t\t}\n\nThis looks good: it's obvious that the previously-supported devices\ncontinue to work the same way because you set \".program_uphy = true\"\nfor them. (It would be even more obvious if you changed the sense so\nthat only the new device had to add this initializaer, but in general\nI prefer the positive logic as you have here, and I did verify that\nyou added the initializer for all tegra_pcie_soc variants.)\n\n> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> static void tegra_pcie_power_off(struct tegra_pcie *pcie)\n> {\n> \tstruct device *dev = pcie->dev;\n> +\tconst struct tegra_pcie_soc *soc = pcie->soc;\n> \tint err;\n> \n> \t/* TODO: disable and unprepare clocks? */\n> \n> -\terr = tegra_pcie_phy_power_off(pcie);\n> -\tif (err < 0)\n> -\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> +\tif (soc->program_uphy) {\n> +\t\terr = tegra_pcie_phy_power_off(pcie);\n> +\t\tif (err < 0)\n> +\t\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> +\t}\n> \n> \treset_control_assert(pcie->pcie_xrst);\n> \treset_control_assert(pcie->afi_rst);\n> \treset_control_assert(pcie->pex_rst);\n> \n> -\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> +\tif (!dev->pm_domain)\n> +\t\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n\nBut this one isn't obvious because nothing in your patch obviously\naffects dev->pm_domain, so I can't tell whether this is safe. It's\nworth a changelog comment to help justify this.\n\nBjorn","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=helgaas@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yGj4C09Yzz9t39\n\tfor ;\n\tWed, 18 Oct 2017 04:34:35 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1759515AbdJQRec (ORCPT );\n\tTue, 17 Oct 2017 13:34:32 -0400","from mail.kernel.org ([198.145.29.99]:33972 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753594AbdJQRea (ORCPT );\n\tTue, 17 Oct 2017 13:34:30 -0400","from localhost (unknown [69.55.156.165])\n\t(using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits))\n\t(No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 4D5E0218CA;\n\tTue, 17 Oct 2017 17:34:29 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 4D5E0218CA","Date":"Tue, 17 Oct 2017 12:34:28 -0500","From":"Bjorn Helgaas ","To":"Manikanta Maddireddy ","Cc":"bhelgaas@google.com, thierry.reding@gmail.com,\n\tjonathanh@nvidia.com, linux-tegra@vger.kernel.org,\n\tlinux-pci@vger.kernel.org, mperttunen@nvidia.com","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","Message-ID":"<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1788842,"web_url":"http://patchwork.ozlabs.org/comment/1788842/","msgid":"<20171017202749.GB10482@ulmo>","list_archive_url":null,"date":"2017-10-17T20:27:49","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:\n> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n> > UPHY programming is performed by BPMP, PHY enable calls are\n> > not required for Tegra186 PCIe. Power partition ungate is\n> > done by BPMP powergate driver.\n> > \n> > Signed-off-by: Manikanta Maddireddy \n> > Reviewed-by: Mikko Perttunen \n> > Tested-by: Mikko Perttunen \n> > ---\n> > V2: Add soc->program_uphy check for phy_exit call\n> > drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n> > 1 file changed, 108 insertions(+), 24 deletions(-)\n> \n> > @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> > \t\tafi_writel(pcie, value, AFI_FUSE);\n> > \t}\n> > \n> > -\terr = tegra_pcie_phy_power_on(pcie);\n> > -\tif (err < 0) {\n> > -\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> > -\t\treturn err;\n> > +\tif (soc->program_uphy) {\n> > +\t\terr = tegra_pcie_phy_power_on(pcie);\n> > +\t\tif (err < 0) {\n> > +\t\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> > +\t\t\treturn err;\n> > +\t\t}\n> \n> This looks good: it's obvious that the previously-supported devices\n> continue to work the same way because you set \".program_uphy = true\"\n> for them. (It would be even more obvious if you changed the sense so\n> that only the new device had to add this initializaer, but in general\n> I prefer the positive logic as you have here, and I did verify that\n> you added the initializer for all tegra_pcie_soc variants.)\n> \n> > @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> > static void tegra_pcie_power_off(struct tegra_pcie *pcie)\n> > {\n> > \tstruct device *dev = pcie->dev;\n> > +\tconst struct tegra_pcie_soc *soc = pcie->soc;\n> > \tint err;\n> > \n> > \t/* TODO: disable and unprepare clocks? */\n> > \n> > -\terr = tegra_pcie_phy_power_off(pcie);\n> > -\tif (err < 0)\n> > -\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> > +\tif (soc->program_uphy) {\n> > +\t\terr = tegra_pcie_phy_power_off(pcie);\n> > +\t\tif (err < 0)\n> > +\t\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> > +\t}\n> > \n> > \treset_control_assert(pcie->pcie_xrst);\n> > \treset_control_assert(pcie->afi_rst);\n> > \treset_control_assert(pcie->pex_rst);\n> > \n> > -\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> > +\tif (!dev->pm_domain)\n> > +\t\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> \n> But this one isn't obvious because nothing in your patch obviously\n> affects dev->pm_domain, so I can't tell whether this is safe. It's\n> worth a changelog comment to help justify this.\n\nThe last sentence in the commit message is what this is about. Maybe it\nshould be more verbose:\n\n\tSince the BPMP controls the powergates on Tegra186, there is no\n\tneed to manually power on and off the PCIe power partition. The\n\tBPMP generic power domain driver takes care of it instead.\n\n?\n\nThierry","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"dOzXBJF5\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yGmwC3fshz9t2m\n\tfor ;\n\tWed, 18 Oct 2017 07:27:55 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1756797AbdJQU1x (ORCPT );\n\tTue, 17 Oct 2017 16:27:53 -0400","from mail-qk0-f196.google.com ([209.85.220.196]:52964 \"EHLO\n\tmail-qk0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1755353AbdJQU1w (ORCPT\n\t); Tue, 17 Oct 2017 16:27:52 -0400","by mail-qk0-f196.google.com with SMTP id b15so3688570qkg.9;\n\tTue, 17 Oct 2017 13:27:52 -0700 (PDT)","from localhost\n\t(p200300E41BE4FD00CEAD5B94E1CFD280.dip0.t-ipconnect.de.\n\t[2003:e4:1be4:fd00:cead:5b94:e1cf:d280])\n\tby smtp.gmail.com with ESMTPSA id\n\tp9sm6506368qkl.7.2017.10.17.13.27.50\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 17 Oct 2017 13:27:50 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=oos8J7GXqqnI+ly1DxxbX6dLYSPAo2pP3m3t5Y9qjJ4=;\n\tb=dOzXBJF54KiZDEAWy3ETsaEnQoqLud/MMOD0R6UVrJE2sw+80QmYTbvFnoaqqRuHUw\n\tAjtvaCaD05pbjG0BAkQxiErUt0JopWk/FMS5FJVBC2wvDFUqq91xaUJnnsDIG/MlE5QR\n\tyeEGIbeuLTpMQkxeJfiFPgycdDWrSC2LDW0lRsQlnTVGnU4X/6EdUskZR815TSqnJxU9\n\tAhSE/AqB1+DhK8oFbdWs49+svnomjxvERjClc5mNJtoZ02inrspE8gAVkiPj+I7UKl26\n\tYTv/8morusfsNnUz0wyIGIeVkk1OlQH8bjAWLKWdPg8k5VWtNDxazaQqVevb5PAAJuVN\n\t8Y4g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=oos8J7GXqqnI+ly1DxxbX6dLYSPAo2pP3m3t5Y9qjJ4=;\n\tb=floROV0sgeBPS91DIT52C1sy/2VhjnH7aHqTG1hKQ7GqMrJRSzq1fG26X1cgvqym+M\n\tdNCrFihnmWi6Fw8GP/6VNE12v0C0IIcchcu2xwScNphxgbJUqTe6XjhSU4rHZkPEizlX\n\tSN34qBxWSXA2w6sWdnAVn+leCszj1kyalOwPgSad7sNSPVpJYljjvmEmmPEotTtAA2nV\n\typrd4pgVDPFLdrtQB40G7ZZIr7g1X5EjGUUX7HWXg1bSAWqxBF++BFK6PJFgK/6F2qjA\n\tEx0TD2tUEOtCu3IkO0RvqJvsdXwSM/ndDgV3mmbxrjkNcT5Qkg1ygHxBbkfNJaKe28VZ\n\tJzaw==","X-Gm-Message-State":"AMCzsaVX+lHSF0Bjuyoclc2ZmBUn/MO+WRETF+DSv9XMdOo5FfInh/aO\n\t6KZu7cM4PUby1zQP+4phbSw=","X-Google-Smtp-Source":"ABhQp+SMGiwqCtUaiCxMQ+eou9H6XHP1CdTQVN+WgJAzaggmCY192W5+V/pM8YPWGf5usqRnTGPgvg==","X-Received":"by 10.55.152.199 with SMTP id\n\ta190mr16484652qke.190.1508272071519; \n\tTue, 17 Oct 2017 13:27:51 -0700 (PDT)","Date":"Tue, 17 Oct 2017 22:27:49 +0200","From":"Thierry Reding ","To":"Bjorn Helgaas ","Cc":"Manikanta Maddireddy , bhelgaas@google.com,\n\tjonathanh@nvidia.com, linux-tegra@vger.kernel.org,\n\tlinux-pci@vger.kernel.org, mperttunen@nvidia.com","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","Message-ID":"<20171017202749.GB10482@ulmo>","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>\n\t<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"H1spWtNR+x+ondvy\"","Content-Disposition":"inline","In-Reply-To":"<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1789571,"web_url":"http://patchwork.ozlabs.org/comment/1789571/","msgid":"<20171018132752.GO5641@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-10-18T13:27:53","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:\n> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:\n> > On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n> > > UPHY programming is performed by BPMP, PHY enable calls are\n> > > not required for Tegra186 PCIe. Power partition ungate is\n> > > done by BPMP powergate driver.\n> > > \n> > > Signed-off-by: Manikanta Maddireddy \n> > > Reviewed-by: Mikko Perttunen \n> > > Tested-by: Mikko Perttunen \n> > > ---\n> > > V2: Add soc->program_uphy check for phy_exit call\n> > > drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n> > > 1 file changed, 108 insertions(+), 24 deletions(-)\n> > \n> > > @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> > > \t\tafi_writel(pcie, value, AFI_FUSE);\n> > > \t}\n> > > \n> > > -\terr = tegra_pcie_phy_power_on(pcie);\n> > > -\tif (err < 0) {\n> > > -\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> > > -\t\treturn err;\n> > > +\tif (soc->program_uphy) {\n> > > +\t\terr = tegra_pcie_phy_power_on(pcie);\n> > > +\t\tif (err < 0) {\n> > > +\t\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> > > +\t\t\treturn err;\n> > > +\t\t}\n> > \n> > This looks good: it's obvious that the previously-supported devices\n> > continue to work the same way because you set \".program_uphy = true\"\n> > for them. (It would be even more obvious if you changed the sense so\n> > that only the new device had to add this initializaer, but in general\n> > I prefer the positive logic as you have here, and I did verify that\n> > you added the initializer for all tegra_pcie_soc variants.)\n> > \n> > > @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> > > static void tegra_pcie_power_off(struct tegra_pcie *pcie)\n> > > {\n> > > \tstruct device *dev = pcie->dev;\n> > > +\tconst struct tegra_pcie_soc *soc = pcie->soc;\n> > > \tint err;\n> > > \n> > > \t/* TODO: disable and unprepare clocks? */\n> > > \n> > > -\terr = tegra_pcie_phy_power_off(pcie);\n> > > -\tif (err < 0)\n> > > -\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> > > +\tif (soc->program_uphy) {\n> > > +\t\terr = tegra_pcie_phy_power_off(pcie);\n> > > +\t\tif (err < 0)\n> > > +\t\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> > > +\t}\n> > > \n> > > \treset_control_assert(pcie->pcie_xrst);\n> > > \treset_control_assert(pcie->afi_rst);\n> > > \treset_control_assert(pcie->pex_rst);\n> > > \n> > > -\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> > > +\tif (!dev->pm_domain)\n> > > +\t\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> > \n> > But this one isn't obvious because nothing in your patch obviously\n> > affects dev->pm_domain, so I can't tell whether this is safe. It's\n> > worth a changelog comment to help justify this.\n> \n> The last sentence in the commit message is what this is about. Maybe it\n> should be more verbose:\n> \n> \tSince the BPMP controls the powergates on Tegra186, there is no\n> \tneed to manually power on and off the PCIe power partition. The\n> \tBPMP generic power domain driver takes care of it instead.\n\nIf you know Tegra intimately, maybe that clarifies it. I don't, so my\nproblem is that there's nothing in the patch that helps me verify\nthis. I infer that maybe there's something different in the DT that\nmeans dev->pm_domain will be set for Tegra186, but not for other\nvariants?\n\nThere should be something that helps the reader connect the dots.\n\nBjorn","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=helgaas@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yHCZB1V2Xz9t4c\n\tfor ;\n\tThu, 19 Oct 2017 00:28:50 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753676AbdJRN2r (ORCPT );\n\tWed, 18 Oct 2017 09:28:47 -0400","from mail.kernel.org ([198.145.29.99]:56314 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753889AbdJRN1z (ORCPT );\n\tWed, 18 Oct 2017 09:27:55 -0400","from localhost (173-17-104-160.client.mchsi.com [173.17.104.160])\n\t(using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits))\n\t(No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 9D2852133D;\n\tWed, 18 Oct 2017 13:27:54 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 9D2852133D","Date":"Wed, 18 Oct 2017 08:27:53 -0500","From":"Bjorn Helgaas ","To":"Thierry Reding ","Cc":"Manikanta Maddireddy , bhelgaas@google.com,\n\tjonathanh@nvidia.com, linux-tegra@vger.kernel.org,\n\tlinux-pci@vger.kernel.org, mperttunen@nvidia.com","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","Message-ID":"<20171018132752.GO5641@bhelgaas-glaptop.roam.corp.google.com>","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>\n\t<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>\n\t<20171017202749.GB10482@ulmo>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20171017202749.GB10482@ulmo>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1789622,"web_url":"http://patchwork.ozlabs.org/comment/1789622/","msgid":"<9ceb5f5c-7fcc-d729-e5d8-d13142bee8ed@nvidia.com>","list_archive_url":null,"date":"2017-10-18T14:14:12","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":72399,"url":"http://patchwork.ozlabs.org/api/people/72399/","name":"Manikanta Maddireddy","email":"mmaddireddy@nvidia.com"},"content":"On 18-Oct-17 6:57 PM, Bjorn Helgaas wrote:\n> On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:\n>> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:\n>>> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n>>>> UPHY programming is performed by BPMP, PHY enable calls are\n>>>> not required for Tegra186 PCIe. Power partition ungate is\n>>>> done by BPMP powergate driver.\n>>>>\n>>>> Signed-off-by: Manikanta Maddireddy \n>>>> Reviewed-by: Mikko Perttunen \n>>>> Tested-by: Mikko Perttunen \n>>>> ---\n>>>> V2: Add soc->program_uphy check for phy_exit call\n>>>> drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n>>>> 1 file changed, 108 insertions(+), 24 deletions(-)\n>>>\n>>>> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n>>>> \t\tafi_writel(pcie, value, AFI_FUSE);\n>>>> \t}\n>>>> \n>>>> -\terr = tegra_pcie_phy_power_on(pcie);\n>>>> -\tif (err < 0) {\n>>>> -\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n>>>> -\t\treturn err;\n>>>> +\tif (soc->program_uphy) {\n>>>> +\t\terr = tegra_pcie_phy_power_on(pcie);\n>>>> +\t\tif (err < 0) {\n>>>> +\t\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n>>>> +\t\t\treturn err;\n>>>> +\t\t}\n>>>\n>>> This looks good: it's obvious that the previously-supported devices\n>>> continue to work the same way because you set \".program_uphy = true\"\n>>> for them. (It would be even more obvious if you changed the sense so\n>>> that only the new device had to add this initializaer, but in general\n>>> I prefer the positive logic as you have here, and I did verify that\n>>> you added the initializer for all tegra_pcie_soc variants.)\n>>>\n>>>> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n>>>> static void tegra_pcie_power_off(struct tegra_pcie *pcie)\n>>>> {\n>>>> \tstruct device *dev = pcie->dev;\n>>>> +\tconst struct tegra_pcie_soc *soc = pcie->soc;\n>>>> \tint err;\n>>>> \n>>>> \t/* TODO: disable and unprepare clocks? */\n>>>> \n>>>> -\terr = tegra_pcie_phy_power_off(pcie);\n>>>> -\tif (err < 0)\n>>>> -\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n>>>> +\tif (soc->program_uphy) {\n>>>> +\t\terr = tegra_pcie_phy_power_off(pcie);\n>>>> +\t\tif (err < 0)\n>>>> +\t\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n>>>> +\t}\n>>>> \n>>>> \treset_control_assert(pcie->pcie_xrst);\n>>>> \treset_control_assert(pcie->afi_rst);\n>>>> \treset_control_assert(pcie->pex_rst);\n>>>> \n>>>> -\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n>>>> +\tif (!dev->pm_domain)\n>>>> +\t\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n>>>\n>>> But this one isn't obvious because nothing in your patch obviously\n>>> affects dev->pm_domain, so I can't tell whether this is safe. It's\n>>> worth a changelog comment to help justify this.\n>>\n>> The last sentence in the commit message is what this is about. Maybe it\n>> should be more verbose:\n>>\n>> \tSince the BPMP controls the powergates on Tegra186, there is no\n>> \tneed to manually power on and off the PCIe power partition. The\n>> \tBPMP generic power domain driver takes care of it instead.\n> \n> If you know Tegra intimately, maybe that clarifies it. I don't, so my\n> problem is that there's nothing in the patch that helps me verify\n> this. I infer that maybe there's something different in the DT that\n> means dev->pm_domain will be set for Tegra186, but not for other\n> variants?\n> \n> There should be something that helps the reader connect the dots.\n> \n> Bjorn\n> \nHi Bjorn,\n\nTill tegra210 pmc driver provides direct powergate power ON/OFF functions.\n\nIn tegra186.dtsi power-domains property is added in pcie@10003000 DT node. reference: https://patchwork.ozlabs.org/patch/819113/\npowergate-bpmp.c is the pm domain provider for pcie powergate. \nPlatform driver will look for pm domain provider of Tegra pcie host driver and sets dev->pm_domains.\npowergate-bpmp.c driver registers powergate power ON/OFF functions to generic PD power_on/powerr_off callback functions.\nGeneric power domain will take care of calling power_on before Tegra PCIe probe.\n\nSo in this patch I used dev->pm_domain to skip pmc driver calls.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yHDbK2v3sz9t5s\n\tfor ;\n\tThu, 19 Oct 2017 01:14:53 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751184AbdJROOw (ORCPT );\n\tWed, 18 Oct 2017 10:14:52 -0400","from hqemgate16.nvidia.com ([216.228.121.65]:14560 \"EHLO\n\thqemgate16.nvidia.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750775AbdJROOv (ORCPT\n\t); Wed, 18 Oct 2017 10:14:51 -0400","from hqpgpgate102.nvidia.com (Not Verified[216.228.121.13]) by\n\thqemgate16.nvidia.com\n\tid ; Wed, 18 Oct 2017 07:14:33 -0700","from HQMAIL108.nvidia.com ([172.20.161.6])\n\tby hqpgpgate102.nvidia.com (PGP Universal service);\n\tWed, 18 Oct 2017 07:14:45 -0700","from BGMAIL102.nvidia.com (10.25.59.11) by HQMAIL108.nvidia.com\n\t(172.18.146.13) with Microsoft SMTP Server (TLS) id 15.0.1293.2;\n\tWed, 18 Oct 2017 14:14:22 +0000","from [10.24.70.249] (10.24.70.249) by bgmail102.nvidia.com\n\t(10.25.59.11) with Microsoft SMTP Server (TLS) id 15.0.1293.2;\n\tWed, 18 Oct 2017 14:14:18 +0000"],"X-PGP-Universal":"processed;\n\tby hqpgpgate102.nvidia.com on Wed, 18 Oct 2017 07:14:45 -0700","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","To":"Bjorn Helgaas ,\n\tThierry Reding ","CC":", ,\n\t, ,\n\t","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>\n\t<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>\n\t<20171017202749.GB10482@ulmo>\n\t<20171018132752.GO5641@bhelgaas-glaptop.roam.corp.google.com>","X-Nvconfidentiality":"public","From":"Manikanta Maddireddy ","Message-ID":"<9ceb5f5c-7fcc-d729-e5d8-d13142bee8ed@nvidia.com>","Date":"Wed, 18 Oct 2017 19:44:12 +0530","User-Agent":"Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171018132752.GO5641@bhelgaas-glaptop.roam.corp.google.com>","X-Originating-IP":"[10.24.70.249]","X-ClientProxiedBy":"BGMAIL101.nvidia.com (10.25.59.10) To bgmail102.nvidia.com\n\t(10.25.59.11)","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1789757,"web_url":"http://patchwork.ozlabs.org/comment/1789757/","msgid":"<20171018162937.GA32397@bhelgaas-glaptop.roam.corp.google.com>","list_archive_url":null,"date":"2017-10-18T16:29:37","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":67298,"url":"http://patchwork.ozlabs.org/api/people/67298/","name":"Bjorn Helgaas","email":"helgaas@kernel.org"},"content":"On Wed, Oct 18, 2017 at 07:44:12PM +0530, Manikanta Maddireddy wrote:\n> \n> \n> On 18-Oct-17 6:57 PM, Bjorn Helgaas wrote:\n> > On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:\n> >> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:\n> >>> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n> >>>> UPHY programming is performed by BPMP, PHY enable calls are\n> >>>> not required for Tegra186 PCIe. Power partition ungate is\n> >>>> done by BPMP powergate driver.\n> >>>>\n> >>>> Signed-off-by: Manikanta Maddireddy \n> >>>> Reviewed-by: Mikko Perttunen \n> >>>> Tested-by: Mikko Perttunen \n> >>>> ---\n> >>>> V2: Add soc->program_uphy check for phy_exit call\n> >>>> drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n> >>>> 1 file changed, 108 insertions(+), 24 deletions(-)\n> >>>\n> >>>> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> >>>> \t\tafi_writel(pcie, value, AFI_FUSE);\n> >>>> \t}\n> >>>> \n> >>>> -\terr = tegra_pcie_phy_power_on(pcie);\n> >>>> -\tif (err < 0) {\n> >>>> -\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> >>>> -\t\treturn err;\n> >>>> +\tif (soc->program_uphy) {\n> >>>> +\t\terr = tegra_pcie_phy_power_on(pcie);\n> >>>> +\t\tif (err < 0) {\n> >>>> +\t\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n> >>>> +\t\t\treturn err;\n> >>>> +\t\t}\n> >>>\n> >>> This looks good: it's obvious that the previously-supported devices\n> >>> continue to work the same way because you set \".program_uphy = true\"\n> >>> for them. (It would be even more obvious if you changed the sense so\n> >>> that only the new device had to add this initializaer, but in general\n> >>> I prefer the positive logic as you have here, and I did verify that\n> >>> you added the initializer for all tegra_pcie_soc variants.)\n> >>>\n> >>>> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n> >>>> static void tegra_pcie_power_off(struct tegra_pcie *pcie)\n> >>>> {\n> >>>> \tstruct device *dev = pcie->dev;\n> >>>> +\tconst struct tegra_pcie_soc *soc = pcie->soc;\n> >>>> \tint err;\n> >>>> \n> >>>> \t/* TODO: disable and unprepare clocks? */\n> >>>> \n> >>>> -\terr = tegra_pcie_phy_power_off(pcie);\n> >>>> -\tif (err < 0)\n> >>>> -\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> >>>> +\tif (soc->program_uphy) {\n> >>>> +\t\terr = tegra_pcie_phy_power_off(pcie);\n> >>>> +\t\tif (err < 0)\n> >>>> +\t\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n> >>>> +\t}\n> >>>> \n> >>>> \treset_control_assert(pcie->pcie_xrst);\n> >>>> \treset_control_assert(pcie->afi_rst);\n> >>>> \treset_control_assert(pcie->pex_rst);\n> >>>> \n> >>>> -\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> >>>> +\tif (!dev->pm_domain)\n> >>>> +\t\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n> >>>\n> >>> But this one isn't obvious because nothing in your patch obviously\n> >>> affects dev->pm_domain, so I can't tell whether this is safe. It's\n> >>> worth a changelog comment to help justify this.\n> >>\n> >> The last sentence in the commit message is what this is about. Maybe it\n> >> should be more verbose:\n> >>\n> >> \tSince the BPMP controls the powergates on Tegra186, there is no\n> >> \tneed to manually power on and off the PCIe power partition. The\n> >> \tBPMP generic power domain driver takes care of it instead.\n> > \n> > If you know Tegra intimately, maybe that clarifies it. I don't, so my\n> > problem is that there's nothing in the patch that helps me verify\n> > this. I infer that maybe there's something different in the DT that\n> > means dev->pm_domain will be set for Tegra186, but not for other\n> > variants?\n> > \n> > There should be something that helps the reader connect the dots.\n> \n> Till tegra210 pmc driver provides direct powergate power ON/OFF functions.\n> \n> In tegra186.dtsi power-domains property is added in pcie@10003000 DT node. reference: https://patchwork.ozlabs.org/patch/819113/\n> powergate-bpmp.c is the pm domain provider for pcie powergate. \n> Platform driver will look for pm domain provider of Tegra pcie host driver and sets dev->pm_domains.\n> powergate-bpmp.c driver registers powergate power ON/OFF functions to generic PD power_on/powerr_off callback functions.\n> Generic power domain will take care of calling power_on before Tegra PCIe probe.\n> \n> So in this patch I used dev->pm_domain to skip pmc driver calls.\n\nThanks, I updated the changelog as follows:\n\n\ncommit 9cea513d8cbc75ee26327d3d8971fe7b58288d8f\nAuthor: Manikanta Maddireddy \nDate: Wed Sep 27 17:28:35 2017 +0530\n\n PCI: tegra: Add Tegra186 PCIe support\n \n Add Tegra186 PCIe support. UPHY programming is performed by BPMP; PHY\n enable calls are not required for Tegra186 PCIe.\n \n Power partition ungate is done by BPMP powergate driver. The Tegra186\n DT description must include a \"power-domains\" property, which results in\n dev->pm_domain being set.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","mail.kernel.org;\n\tdmarc=none (p=none dis=none) header.from=kernel.org","mail.kernel.org;\n\tspf=none smtp.mailfrom=helgaas@kernel.org"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yHHZt2yPrz9t4c\n\tfor ;\n\tThu, 19 Oct 2017 03:29:42 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751613AbdJRQ3k (ORCPT );\n\tWed, 18 Oct 2017 12:29:40 -0400","from mail.kernel.org ([198.145.29.99]:45212 \"EHLO mail.kernel.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751348AbdJRQ3j (ORCPT );\n\tWed, 18 Oct 2017 12:29:39 -0400","from localhost (unknown [69.71.4.159])\n\t(using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits))\n\t(No client certificate requested)\n\tby mail.kernel.org (Postfix) with ESMTPSA id 9C4BB21879;\n\tWed, 18 Oct 2017 16:29:38 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mail.kernel.org 9C4BB21879","Date":"Wed, 18 Oct 2017 11:29:37 -0500","From":"Bjorn Helgaas ","To":"Manikanta Maddireddy ","Cc":"Thierry Reding , bhelgaas@google.com,\n\tjonathanh@nvidia.com, linux-tegra@vger.kernel.org,\n\tlinux-pci@vger.kernel.org, mperttunen@nvidia.com","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","Message-ID":"<20171018162937.GA32397@bhelgaas-glaptop.roam.corp.google.com>","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>\n\t<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>\n\t<20171017202749.GB10482@ulmo>\n\t<20171018132752.GO5641@bhelgaas-glaptop.roam.corp.google.com>\n\t<9ceb5f5c-7fcc-d729-e5d8-d13142bee8ed@nvidia.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<9ceb5f5c-7fcc-d729-e5d8-d13142bee8ed@nvidia.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}},{"id":1790156,"web_url":"http://patchwork.ozlabs.org/comment/1790156/","msgid":"","list_archive_url":null,"date":"2017-10-19T06:51:27","subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","submitter":{"id":72399,"url":"http://patchwork.ozlabs.org/api/people/72399/","name":"Manikanta Maddireddy","email":"mmaddireddy@nvidia.com"},"content":"On 18-Oct-17 9:59 PM, Bjorn Helgaas wrote:\n> On Wed, Oct 18, 2017 at 07:44:12PM +0530, Manikanta Maddireddy wrote:\n>>\n>>\n>> On 18-Oct-17 6:57 PM, Bjorn Helgaas wrote:\n>>> On Tue, Oct 17, 2017 at 10:27:49PM +0200, Thierry Reding wrote:\n>>>> On Tue, Oct 17, 2017 at 12:34:28PM -0500, Bjorn Helgaas wrote:\n>>>>> On Wed, Sep 27, 2017 at 05:28:35PM +0530, Manikanta Maddireddy wrote:\n>>>>>> UPHY programming is performed by BPMP, PHY enable calls are\n>>>>>> not required for Tegra186 PCIe. Power partition ungate is\n>>>>>> done by BPMP powergate driver.\n>>>>>>\n>>>>>> Signed-off-by: Manikanta Maddireddy \n>>>>>> Reviewed-by: Mikko Perttunen \n>>>>>> Tested-by: Mikko Perttunen \n>>>>>> ---\n>>>>>> V2: Add soc->program_uphy check for phy_exit call\n>>>>>> drivers/pci/host/pci-tegra.c | 132 +++++++++++++++++++++++++++++++++++--------\n>>>>>> 1 file changed, 108 insertions(+), 24 deletions(-)\n>>>>>\n>>>>>> @@ -1012,10 +1016,12 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n>>>>>> \t\tafi_writel(pcie, value, AFI_FUSE);\n>>>>>> \t}\n>>>>>> \n>>>>>> -\terr = tegra_pcie_phy_power_on(pcie);\n>>>>>> -\tif (err < 0) {\n>>>>>> -\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n>>>>>> -\t\treturn err;\n>>>>>> +\tif (soc->program_uphy) {\n>>>>>> +\t\terr = tegra_pcie_phy_power_on(pcie);\n>>>>>> +\t\tif (err < 0) {\n>>>>>> +\t\t\tdev_err(dev, \"failed to power on PHY(s): %d\\n\", err);\n>>>>>> +\t\t\treturn err;\n>>>>>> +\t\t}\n>>>>>\n>>>>> This looks good: it's obvious that the previously-supported devices\n>>>>> continue to work the same way because you set \".program_uphy = true\"\n>>>>> for them. (It would be even more obvious if you changed the sense so\n>>>>> that only the new device had to add this initializaer, but in general\n>>>>> I prefer the positive logic as you have here, and I did verify that\n>>>>> you added the initializer for all tegra_pcie_soc variants.)\n>>>>>\n>>>>>> @@ -1048,19 +1054,23 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie)\n>>>>>> static void tegra_pcie_power_off(struct tegra_pcie *pcie)\n>>>>>> {\n>>>>>> \tstruct device *dev = pcie->dev;\n>>>>>> +\tconst struct tegra_pcie_soc *soc = pcie->soc;\n>>>>>> \tint err;\n>>>>>> \n>>>>>> \t/* TODO: disable and unprepare clocks? */\n>>>>>> \n>>>>>> -\terr = tegra_pcie_phy_power_off(pcie);\n>>>>>> -\tif (err < 0)\n>>>>>> -\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n>>>>>> +\tif (soc->program_uphy) {\n>>>>>> +\t\terr = tegra_pcie_phy_power_off(pcie);\n>>>>>> +\t\tif (err < 0)\n>>>>>> +\t\t\tdev_err(dev, \"failed to power off PHY(s): %d\\n\", err);\n>>>>>> +\t}\n>>>>>> \n>>>>>> \treset_control_assert(pcie->pcie_xrst);\n>>>>>> \treset_control_assert(pcie->afi_rst);\n>>>>>> \treset_control_assert(pcie->pex_rst);\n>>>>>> \n>>>>>> -\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n>>>>>> +\tif (!dev->pm_domain)\n>>>>>> +\t\ttegra_powergate_power_off(TEGRA_POWERGATE_PCIE);\n>>>>>\n>>>>> But this one isn't obvious because nothing in your patch obviously\n>>>>> affects dev->pm_domain, so I can't tell whether this is safe. It's\n>>>>> worth a changelog comment to help justify this.\n>>>>\n>>>> The last sentence in the commit message is what this is about. Maybe it\n>>>> should be more verbose:\n>>>>\n>>>> \tSince the BPMP controls the powergates on Tegra186, there is no\n>>>> \tneed to manually power on and off the PCIe power partition. The\n>>>> \tBPMP generic power domain driver takes care of it instead.\n>>>\n>>> If you know Tegra intimately, maybe that clarifies it. I don't, so my\n>>> problem is that there's nothing in the patch that helps me verify\n>>> this. I infer that maybe there's something different in the DT that\n>>> means dev->pm_domain will be set for Tegra186, but not for other\n>>> variants?\n>>>\n>>> There should be something that helps the reader connect the dots.\n>>\n>> Till tegra210 pmc driver provides direct powergate power ON/OFF functions.\n>>\n>> In tegra186.dtsi power-domains property is added in pcie@10003000 DT node. reference: https://patchwork.ozlabs.org/patch/819113/\n>> powergate-bpmp.c is the pm domain provider for pcie powergate. \n>> Platform driver will look for pm domain provider of Tegra pcie host driver and sets dev->pm_domains.\n>> powergate-bpmp.c driver registers powergate power ON/OFF functions to generic PD power_on/powerr_off callback functions.\n>> Generic power domain will take care of calling power_on before Tegra PCIe probe.\n>>\n>> So in this patch I used dev->pm_domain to skip pmc driver calls.\n> \n> Thanks, I updated the changelog as follows:\n> \n> \n> commit 9cea513d8cbc75ee26327d3d8971fe7b58288d8f\n> Author: Manikanta Maddireddy \n> Date: Wed Sep 27 17:28:35 2017 +0530\n> \n> PCI: tegra: Add Tegra186 PCIe support\n> \n> Add Tegra186 PCIe support. UPHY programming is performed by BPMP; PHY\n> enable calls are not required for Tegra186 PCIe.\n> \n> Power partition ungate is done by BPMP powergate driver. The Tegra186\n> DT description must include a \"power-domains\" property, which results in\n> dev->pm_domain being set.\n> \nThank you Bjorn","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-pci-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yHfkj3W8Vz9t44\n\tfor ;\n\tThu, 19 Oct 2017 17:52:45 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751628AbdJSGwn (ORCPT );\n\tThu, 19 Oct 2017 02:52:43 -0400","from hqemgate15.nvidia.com ([216.228.121.64]:1394 \"EHLO\n\thqemgate15.nvidia.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751585AbdJSGwm (ORCPT\n\t); Thu, 19 Oct 2017 02:52:42 -0400","from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by\n\thqemgate15.nvidia.com\n\tid ; Wed, 18 Oct 2017 23:52:01 -0700","from HQMAIL103.nvidia.com ([172.20.161.6])\n\tby hqpgpgate101.nvidia.com (PGP Universal service);\n\tWed, 18 Oct 2017 23:52:14 -0700","from BGMAIL102.nvidia.com (10.25.59.11) by HQMAIL103.nvidia.com\n\t(172.20.187.11) with Microsoft SMTP Server (TLS) id 15.0.1293.2;\n\tThu, 19 Oct 2017 06:51:38 +0000","from [10.24.71.38] (10.24.71.38) by bgmail102.nvidia.com\n\t(10.25.59.11) with Microsoft SMTP Server (TLS) id 15.0.1293.2;\n\tThu, 19 Oct 2017 06:51:34 +0000"],"X-PGP-Universal":"processed;\n\tby hqpgpgate101.nvidia.com on Wed, 18 Oct 2017 23:52:14 -0700","Subject":"Re: [PATCH V2 2/4] PCI: tegra: Add Tegra186 PCIe support","To":"Bjorn Helgaas ","CC":"Thierry Reding , ,\n\t, ,\n\t, ","References":"<1506513517-25870-1-git-send-email-mmaddireddy@nvidia.com>\n\t<1506513517-25870-3-git-send-email-mmaddireddy@nvidia.com>\n\t<20171017173428.GE5641@bhelgaas-glaptop.roam.corp.google.com>\n\t<20171017202749.GB10482@ulmo>\n\t<20171018132752.GO5641@bhelgaas-glaptop.roam.corp.google.com>\n\t<9ceb5f5c-7fcc-d729-e5d8-d13142bee8ed@nvidia.com>\n\t<20171018162937.GA32397@bhelgaas-glaptop.roam.corp.google.com>","X-Nvconfidentiality":"public","From":"Manikanta Maddireddy ","Message-ID":"","Date":"Thu, 19 Oct 2017 12:21:27 +0530","User-Agent":"Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171018162937.GA32397@bhelgaas-glaptop.roam.corp.google.com>","X-Originating-IP":"[10.24.71.38]","X-ClientProxiedBy":"BGMAIL104.nvidia.com (10.25.59.13) To bgmail102.nvidia.com\n\t(10.25.59.11)","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"linux-pci-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-pci@vger.kernel.org"}}]