[{"id":1786442,"web_url":"http://patchwork.ozlabs.org/comment/1786442/","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-tegra-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (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 3yDD144Yzpz9sxR\n\tfor ;\n\tSat, 14 Oct 2017 03:38:16 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752345AbdJMQiN (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);\n\tFri, 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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}},{"id":1788699,"web_url":"http://patchwork.ozlabs.org/comment/1788699/","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\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-tegra\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","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-tegra-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 3yGj4C5V7Wz9t3h\n\tfor ;\n\tWed, 18 Oct 2017 04:34:35 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1759495AbdJQRec (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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}},{"id":1788841,"web_url":"http://patchwork.ozlabs.org/comment/1788841/","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-tegra-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (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 3yGmwB4hJHz9t2c\n\tfor ;\n\tWed, 18 Oct 2017 07:27:54 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1758640AbdJQU1x (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);\n\tTue, 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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}},{"id":1789584,"web_url":"http://patchwork.ozlabs.org/comment/1789584/","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\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-tegra\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","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-tegra-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 3yHChd2X7yz9t3h\n\tfor ;\n\tThu, 19 Oct 2017 00:34:25 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754408AbdJRN2r (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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}},{"id":1789621,"web_url":"http://patchwork.ozlabs.org/comment/1789621/","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.\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-tegra\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","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-tegra-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 3yHDbJ5dL5z9t4P\n\tfor ;\n\tThu, 19 Oct 2017 01:14:52 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751343AbdJROOw (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);\n\tWed, 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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}},{"id":1789756,"web_url":"http://patchwork.ozlabs.org/comment/1789756/","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.\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-tegra\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","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-tegra-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 3yHHZs4CLlz9t3h\n\tfor ;\n\tThu, 19 Oct 2017 03:29:41 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751398AbdJRQ3k (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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}},{"id":1790155,"web_url":"http://patchwork.ozlabs.org/comment/1790155/","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\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-tegra\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","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-tegra-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 3yHfkh54jLz9t42\n\tfor ;\n\tThu, 19 Oct 2017 17:52:44 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751626AbdJSGwn (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);\n\tThu, 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-tegra-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-tegra@vger.kernel.org"}}]