Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 5 Mar 2021 06:19:34 -0600
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Om Prakash Singh <omp@nvidia.com>
Cc:     vidyas@nvidia.com, jingoohan1@gmail.com,
        gustavo.pimentel@synopsys.com, lorenzo.pieralisi@arm.com,
        amurray@thegoodpenguin.co.uk, bhelgaas@google.com, kishon@ti.com,
        thierry.reding@gmail.com, Jisheng.Zhang@synaptics.com,
        jonathanh@nvidia.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, kthota@nvidia.com,
        mmaddireddy@nvidia.com, oop.singh@gmail.com
Subject: Re: [PATCH] PCI: tegra: Disable PTM capabilities for EP mode
Message-ID: <20210305121934.GA1067436@bjorn-Precision-5520>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1614931954-11741-1-git-send-email-omp@nvidia.com>
Precedence: bulk

On Fri, Mar 05, 2021 at 01:42:34PM +0530, Om Prakash Singh wrote:
> PCIe EP compliance expect PTM capabilities (ROOT_CAPABLE, RES_CAPABLE,
> CLK_GRAN) to be disabled.

I guess this is just enforcing the PCIe spec requirements that only
Root Ports, RCRBs, and Switches are allowed to set the PTM Responder
Capable bit, and that the Local Clock Granularity is RsvdP if PTM Root
Capable is zero?  (PCIe r5.0, sec 7.9.16.2)

Should this be done more generally somewhere in the dwc code as
opposed to in the tegra code?

> Signed-off-by: Om Prakash Singh <omp@nvidia.com>
> ---
>  drivers/pci/controller/dwc/pcie-tegra194.c | 17 ++++++++++++++++-
>  include/uapi/linux/pci_regs.h              |  1 +
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 6fa216e..a588312 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1639,7 +1639,7 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
>  	struct dw_pcie *pci = &pcie->pci;
>  	struct dw_pcie_ep *ep = &pci->ep;
>  	struct device *dev = pcie->dev;
> -	u32 val;
> +	u32 val, ptm_cap_base = 0;

Unnecessary init.

>  	int ret;
>  
>  	if (pcie->ep_state == EP_STATE_ENABLED)
> @@ -1760,6 +1760,21 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
>  						      PCI_CAP_ID_EXP);
>  	clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ);
>  
> +	/* Disable PTM root and responder capability */
> +	ptm_cap_base = dw_pcie_find_ext_capability(&pcie->pci,
> +						   PCI_EXT_CAP_ID_PTM);
> +	if (ptm_cap_base) {
> +		dw_pcie_dbi_ro_wr_en(pci);
> +		val = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> +		val &= ~PCI_PTM_CAP_ROOT;
> +		dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, val);
> +
> +		val = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP);
> +		val &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK);
> +		dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, val);
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +	}
> +
>  	val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK);
>  	val |= MSIX_ADDR_MATCH_LOW_OFF_EN;
>  	dw_pcie_writel_dbi(pci, MSIX_ADDR_MATCH_LOW_OFF, val);
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e709ae8..9dd6f8d 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -1050,6 +1050,7 @@
>  /* Precision Time Measurement */
>  #define PCI_PTM_CAP			0x04	    /* PTM Capability */
>  #define  PCI_PTM_CAP_REQ		0x00000001  /* Requester capable */
> +#define  PCI_PTM_CAP_RES		0x00000002  /* Responder capable */
>  #define  PCI_PTM_CAP_ROOT		0x00000004  /* Root capable */
>  #define  PCI_PTM_GRANULARITY_MASK	0x0000FF00  /* Clock granularity */
>  #define PCI_PTM_CTRL			0x08	    /* PTM Control */
> -- 
> 2.7.4
>