* [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" @ 2023-06-08 9:36 Vidya Sagar 2023-06-08 16:33 ` Bjorn Helgaas ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Vidya Sagar @ 2023-06-08 9:36 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin Cc: linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") Consider a PCIe hierarchy with a PCIe switch and a device connected downstream of the switch that has support for MPS which is the minimum in the hierarchy, and root port programmed with an MPS in its DevCtl register that is greater than the minimum. In this scenario, the default bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the MPS settings in the hierarchy correctly resulting in the device with support for minimum MPS in the hierarchy receiving the TLPs of size more than that. Although this can be addresed by appending "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a good idea to always have this commandline argument even for the basic functionality to work. Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") avoids this requirement and ensures that the basic functionality of the devices irrespective of the hierarchy and the MPS of the devices in the hierarchy. To reap the benefits of having support for higher MPS, optionally, one can always append the kernel command line with "pci=pcie_bus_perf". Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 4fdadc7b045f..877d81b13334 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) struct dw_pcie *pci = to_dw_pcie_from_pp(pp); struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); u32 val; - u16 val_16; pp->bridge->ops = &tegra_pci_ops; @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) struct device *dev = pcie->dev; u32 val; int ret; - u16 val_16; if (pcie->ep_state == EP_STATE_ENABLED) return; @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - /* Clear Slot Clock Configuration bit if SRNS configuration */ if (pcie->enable_srns) { val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, val_16); } - clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-08 9:36 [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" Vidya Sagar @ 2023-06-08 16:33 ` Bjorn Helgaas 2023-06-09 2:23 ` Vidya Sagar 2023-06-19 1:42 ` [PATCH V2] " Vidya Sagar 2023-07-25 7:51 ` [PATCH V1] " Manivannan Sadhasivam 2 siblings, 1 reply; 25+ messages in thread From: Bjorn Helgaas @ 2023-06-08 16:33 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > support for 256 Byte payload") > > Consider a PCIe hierarchy with a PCIe switch and a device connected > downstream of the switch that has support for MPS which is the minimum > in the hierarchy, and root port programmed with an MPS in its DevCtl > register that is greater than the minimum. In this scenario, the default > bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't > configure the MPS settings in the hierarchy correctly resulting in the > device with support for minimum MPS in the hierarchy receiving the TLPs > of size more than that. Although this can be addresed by appending > "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a > good idea to always have this commandline argument even for the basic > functionality to work. > Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 > Byte payload") avoids this requirement and ensures that the basic > functionality of the devices irrespective of the hierarchy and the MPS of > the devices in the hierarchy. > To reap the benefits of having support for higher MPS, optionally, one can > always append the kernel command line with "pci=pcie_bus_perf". Please add blank lines between paragraphs and wrap to fill 75 columns. Also add a period at the end of the very first sentence. s/addresed/addressed/ I guess that without 4fb8e46c1bc4, Linux configured everything with 128 byte MPS, and 4fb8e46c1bc4 was intended as an optimization to allow 256 byte MPS. If the Root Port advertises Max_Payload_Size Supported as 256 bytes in DevCap, and the PCI core doesn't configure MPS=256 when possible, I'd argue that should be fixed in the PCI core without a driver change like this. Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-08 16:33 ` Bjorn Helgaas @ 2023-06-09 2:23 ` Vidya Sagar 2023-06-14 10:39 ` Jon Hunter 0 siblings, 1 reply; 25+ messages in thread From: Vidya Sagar @ 2023-06-09 2:23 UTC (permalink / raw) To: Bjorn Helgaas Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On 6/8/2023 10:03 PM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: >> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable >> support for 256 Byte payload") >> >> Consider a PCIe hierarchy with a PCIe switch and a device connected >> downstream of the switch that has support for MPS which is the minimum >> in the hierarchy, and root port programmed with an MPS in its DevCtl >> register that is greater than the minimum. In this scenario, the default >> bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't >> configure the MPS settings in the hierarchy correctly resulting in the >> device with support for minimum MPS in the hierarchy receiving the TLPs >> of size more than that. Although this can be addresed by appending >> "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a >> good idea to always have this commandline argument even for the basic >> functionality to work. >> Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 >> Byte payload") avoids this requirement and ensures that the basic >> functionality of the devices irrespective of the hierarchy and the MPS of >> the devices in the hierarchy. >> To reap the benefits of having support for higher MPS, optionally, one can >> always append the kernel command line with "pci=pcie_bus_perf". > > Please add blank lines between paragraphs and wrap to fill 75 columns. > Also add a period at the end of the very first sentence. > > s/addresed/addressed/ > I'll address your comments in the next patch. > I guess that without 4fb8e46c1bc4, Linux configured everything with > 128 byte MPS, and 4fb8e46c1bc4 was intended as an optimization to > allow 256 byte MPS. Correct. > > If the Root Port advertises Max_Payload_Size Supported as 256 bytes in > DevCap, and the PCI core doesn't configure MPS=256 when possible, I'd > argue that should be fixed in the PCI core without a driver change > like this. Well, kernel does configure MPS=256 but only if the 'perf' configuration option is selected. 'perf' configuration option also changes the MRRS, to extract the maximum performance. I'm not sure about the reasons for not making 'perf' configuration as the default configuration though. (IIUC, this is what you are coming to, right?) The current patch which is a revert of an earlier patch is to keep things working for different PCIe hierarchies given the default configuration that kernel is using at the moment. -Vidya Sagar > > Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-09 2:23 ` Vidya Sagar @ 2023-06-14 10:39 ` Jon Hunter 0 siblings, 0 replies; 25+ messages in thread From: Jon Hunter @ 2023-06-14 10:39 UTC (permalink / raw) To: Vidya Sagar, Bjorn Helgaas Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv Hi Bjorn, On 09/06/2023 03:23, Vidya Sagar wrote: > > > On 6/8/2023 10:03 PM, Bjorn Helgaas wrote: >> External email: Use caution opening links or attachments >> >> >> On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: >>> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable >>> support for 256 Byte payload") >>> >>> Consider a PCIe hierarchy with a PCIe switch and a device connected >>> downstream of the switch that has support for MPS which is the minimum >>> in the hierarchy, and root port programmed with an MPS in its DevCtl >>> register that is greater than the minimum. In this scenario, the default >>> bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't >>> configure the MPS settings in the hierarchy correctly resulting in the >>> device with support for minimum MPS in the hierarchy receiving the TLPs >>> of size more than that. Although this can be addresed by appending >>> "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a >>> good idea to always have this commandline argument even for the basic >>> functionality to work. >>> Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 >>> Byte payload") avoids this requirement and ensures that the basic >>> functionality of the devices irrespective of the hierarchy and the >>> MPS of >>> the devices in the hierarchy. >>> To reap the benefits of having support for higher MPS, optionally, >>> one can >>> always append the kernel command line with "pci=pcie_bus_perf". >> >> Please add blank lines between paragraphs and wrap to fill 75 columns. >> Also add a period at the end of the very first sentence. >> >> s/addresed/addressed/ >> > I'll address your comments in the next patch. > >> I guess that without 4fb8e46c1bc4, Linux configured everything with >> 128 byte MPS, and 4fb8e46c1bc4 was intended as an optimization to >> allow 256 byte MPS. > Correct. > >> >> If the Root Port advertises Max_Payload_Size Supported as 256 bytes in >> DevCap, and the PCI core doesn't configure MPS=256 when possible, I'd >> argue that should be fixed in the PCI core without a driver change >> like this. > Well, kernel does configure MPS=256 but only if the 'perf' configuration > option is selected. 'perf' configuration option also changes the MRRS, > to extract the maximum performance. I'm not sure about the reasons for > not making 'perf' configuration as the default configuration though. > (IIUC, this is what you are coming to, right?) > > The current patch which is a revert of an earlier patch is to keep > things working for different PCIe hierarchies given the default > configuration that kernel is using at the moment. Any more feedback on this? If not, would be great to queue for v6.5. Feel free to add my ... Acked-by: Jon Hunter <jonathanh@nvidia.com> Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V2] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-08 9:36 [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" Vidya Sagar 2023-06-08 16:33 ` Bjorn Helgaas @ 2023-06-19 1:42 ` Vidya Sagar 2023-06-19 3:42 ` kernel test robot 2023-06-19 10:26 ` [PATCH V3] " Vidya Sagar 2023-07-25 7:51 ` [PATCH V1] " Manivannan Sadhasivam 2 siblings, 2 replies; 25+ messages in thread From: Vidya Sagar @ 2023-06-19 1:42 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin Cc: linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload"). Consider a PCIe hierarchy with a PCIe switch and a device connected downstream of the switch that has support for MPS which is the minimum in the hierarchy, and root port programmed with an MPS in its DevCtl register that is greater than the minimum. In this scenario, the default bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the MPS settings in the hierarchy correctly resulting in the device with support for minimum MPS in the hierarchy receiving the TLPs of size more than that. Although this can be addressed by appending "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a good idea to always have this commandline argument even for the basic functionality to work. Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") avoids this requirement and ensures that the basic functionality of the devices irrespective of the hierarchy and the MPS of the devices in the hierarchy. To reap the benefits of having support for higher MPS, optionally, one can always append the kernel command line with "pci=pcie_bus_perf". Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- V2: * Addressed review comments from Bjorn drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 4fdadc7b045f..877d81b13334 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) struct dw_pcie *pci = to_dw_pcie_from_pp(pp); struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); u32 val; - u16 val_16; pp->bridge->ops = &tegra_pci_ops; @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) struct device *dev = pcie->dev; u32 val; int ret; - u16 val_16; if (pcie->ep_state == EP_STATE_ENABLED) return; @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - /* Clear Slot Clock Configuration bit if SRNS configuration */ if (pcie->enable_srns) { val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, val_16); } - clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V2] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-19 1:42 ` [PATCH V2] " Vidya Sagar @ 2023-06-19 3:42 ` kernel test robot 2023-06-19 10:26 ` [PATCH V3] " Vidya Sagar 1 sibling, 0 replies; 25+ messages in thread From: kernel test robot @ 2023-06-19 3:42 UTC (permalink / raw) To: Vidya Sagar, lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin Cc: oe-kbuild-all, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv Hi Vidya, kernel test robot noticed the following build errors: [auto build test ERROR on pci/next] [also build test ERROR on pci/for-linus linus/master v6.4-rc7 next-20230616] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Vidya-Sagar/Revert-PCI-tegra194-Enable-support-for-256-Byte-payload/20230619-094403 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20230619014218.1970846-1-vidyas%40nvidia.com patch subject: [PATCH V2] Revert "PCI: tegra194: Enable support for 256 Byte payload" config: arm64-defconfig (https://download.01.org/0day-ci/archive/20230619/202306191113.M2RDKBvQ-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 12.3.0 reproduce: (https://download.01.org/0day-ci/archive/20230619/202306191113.M2RDKBvQ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202306191113.M2RDKBvQ-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/pci/controller/dwc/pcie-tegra194.c: In function 'tegra_pcie_dw_host_init': >> drivers/pci/controller/dwc/pcie-tegra194.c:906:17: error: 'val_16' undeclared (first use in this function) 906 | val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + | ^~~~~~ drivers/pci/controller/dwc/pcie-tegra194.c:906:17: note: each undeclared identifier is reported only once for each function it appears in drivers/pci/controller/dwc/pcie-tegra194.c: In function 'pex_ep_event_pex_rst_deassert': drivers/pci/controller/dwc/pcie-tegra194.c:1865:17: error: 'val_16' undeclared (first use in this function) 1865 | val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + | ^~~~~~ vim +/val_16 +906 drivers/pci/controller/dwc/pcie-tegra194.c 56e15a238d9278 Vidya Sagar 2019-08-13 867 64451ac83fe6ab Bjorn Helgaas 2022-08-04 868 static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) 56e15a238d9278 Vidya Sagar 2019-08-13 869 { 56e15a238d9278 Vidya Sagar 2019-08-13 870 struct dw_pcie *pci = to_dw_pcie_from_pp(pp); f1ab409d578752 Vidya Sagar 2022-07-21 871 struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); 56e15a238d9278 Vidya Sagar 2019-08-13 872 u32 val; 56e15a238d9278 Vidya Sagar 2019-08-13 873 275e88b06a277c Rob Herring 2020-12-18 874 pp->bridge->ops = &tegra_pci_ops; 275e88b06a277c Rob Herring 2020-12-18 875 369b868f4a2ef8 Vidya Sagar 2020-11-26 876 if (!pcie->pcie_cap_base) 369b868f4a2ef8 Vidya Sagar 2020-11-26 877 pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, 369b868f4a2ef8 Vidya Sagar 2020-11-26 878 PCI_CAP_ID_EXP); 369b868f4a2ef8 Vidya Sagar 2020-11-26 879 56e15a238d9278 Vidya Sagar 2019-08-13 880 val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); 56e15a238d9278 Vidya Sagar 2019-08-13 881 val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); 56e15a238d9278 Vidya Sagar 2019-08-13 882 dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); 56e15a238d9278 Vidya Sagar 2019-08-13 883 56e15a238d9278 Vidya Sagar 2019-08-13 884 val = dw_pcie_readl_dbi(pci, PCI_PREF_MEMORY_BASE); 56e15a238d9278 Vidya Sagar 2019-08-13 885 val |= CFG_PREF_MEM_LIMIT_BASE_MEM_DECODE; 56e15a238d9278 Vidya Sagar 2019-08-13 886 val |= CFG_PREF_MEM_LIMIT_BASE_MEM_LIMIT_DECODE; 56e15a238d9278 Vidya Sagar 2019-08-13 887 dw_pcie_writel_dbi(pci, PCI_PREF_MEMORY_BASE, val); 56e15a238d9278 Vidya Sagar 2019-08-13 888 56e15a238d9278 Vidya Sagar 2019-08-13 889 dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 0); 56e15a238d9278 Vidya Sagar 2019-08-13 890 56e15a238d9278 Vidya Sagar 2019-08-13 891 /* Enable as 0xFFFF0001 response for CRS */ 56e15a238d9278 Vidya Sagar 2019-08-13 892 val = dw_pcie_readl_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT); 56e15a238d9278 Vidya Sagar 2019-08-13 893 val &= ~(AMBA_ERROR_RESPONSE_CRS_MASK << AMBA_ERROR_RESPONSE_CRS_SHIFT); 56e15a238d9278 Vidya Sagar 2019-08-13 894 val |= (AMBA_ERROR_RESPONSE_CRS_OKAY_FFFF0001 << 56e15a238d9278 Vidya Sagar 2019-08-13 895 AMBA_ERROR_RESPONSE_CRS_SHIFT); 56e15a238d9278 Vidya Sagar 2019-08-13 896 dw_pcie_writel_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT, val); 56e15a238d9278 Vidya Sagar 2019-08-13 897 56e15a238d9278 Vidya Sagar 2019-08-13 898 /* Configure Max lane width from DT */ 56e15a238d9278 Vidya Sagar 2019-08-13 899 val = dw_pcie_readl_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP); 56e15a238d9278 Vidya Sagar 2019-08-13 900 val &= ~PCI_EXP_LNKCAP_MLW; 56e15a238d9278 Vidya Sagar 2019-08-13 901 val |= (pcie->num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT); 56e15a238d9278 Vidya Sagar 2019-08-13 902 dw_pcie_writel_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP, val); 56e15a238d9278 Vidya Sagar 2019-08-13 903 a54e190737181c Vidya Sagar 2022-07-21 904 /* Clear Slot Clock Configuration bit if SRNS configuration */ a54e190737181c Vidya Sagar 2022-07-21 905 if (pcie->enable_srns) { a54e190737181c Vidya Sagar 2022-07-21 @906 val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + a54e190737181c Vidya Sagar 2022-07-21 907 PCI_EXP_LNKSTA); a54e190737181c Vidya Sagar 2022-07-21 908 val_16 &= ~PCI_EXP_LNKSTA_SLC; a54e190737181c Vidya Sagar 2022-07-21 909 dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, a54e190737181c Vidya Sagar 2022-07-21 910 val_16); a54e190737181c Vidya Sagar 2022-07-21 911 } a54e190737181c Vidya Sagar 2022-07-21 912 56e15a238d9278 Vidya Sagar 2019-08-13 913 config_gen3_gen4_eq_presets(pcie); 56e15a238d9278 Vidya Sagar 2019-08-13 914 56e15a238d9278 Vidya Sagar 2019-08-13 915 init_host_aspm(pcie); 56e15a238d9278 Vidya Sagar 2019-08-13 916 6b6fafc1abc7c0 Vidya Sagar 2020-12-03 917 /* Disable ASPM-L1SS advertisement if there is no CLKREQ routing */ 6b6fafc1abc7c0 Vidya Sagar 2020-12-03 918 if (!pcie->supports_clkreq) { 6b6fafc1abc7c0 Vidya Sagar 2020-12-03 919 disable_aspm_l11(pcie); 6b6fafc1abc7c0 Vidya Sagar 2020-12-03 920 disable_aspm_l12(pcie); 6b6fafc1abc7c0 Vidya Sagar 2020-12-03 921 } 6b6fafc1abc7c0 Vidya Sagar 2020-12-03 922 a54e190737181c Vidya Sagar 2022-07-21 923 if (!pcie->of_data->has_l1ss_exit_fix) { 56e15a238d9278 Vidya Sagar 2019-08-13 924 val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF); 56e15a238d9278 Vidya Sagar 2019-08-13 925 val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL; 56e15a238d9278 Vidya Sagar 2019-08-13 926 dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val); a54e190737181c Vidya Sagar 2022-07-21 927 } 56e15a238d9278 Vidya Sagar 2019-08-13 928 56e15a238d9278 Vidya Sagar 2019-08-13 929 if (pcie->update_fc_fixup) { 56e15a238d9278 Vidya Sagar 2019-08-13 930 val = dw_pcie_readl_dbi(pci, CFG_TIMER_CTRL_MAX_FUNC_NUM_OFF); 56e15a238d9278 Vidya Sagar 2019-08-13 931 val |= 0x1 << CFG_TIMER_CTRL_ACK_NAK_SHIFT; 56e15a238d9278 Vidya Sagar 2019-08-13 932 dw_pcie_writel_dbi(pci, CFG_TIMER_CTRL_MAX_FUNC_NUM_OFF, val); 56e15a238d9278 Vidya Sagar 2019-08-13 933 } 56e15a238d9278 Vidya Sagar 2019-08-13 934 56e15a238d9278 Vidya Sagar 2019-08-13 935 clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); 56e15a238d9278 Vidya Sagar 2019-08-13 936 275e88b06a277c Rob Herring 2020-12-18 937 return 0; 275e88b06a277c Rob Herring 2020-12-18 938 } 275e88b06a277c Rob Herring 2020-12-18 939 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-19 1:42 ` [PATCH V2] " Vidya Sagar 2023-06-19 3:42 ` kernel test robot @ 2023-06-19 10:26 ` Vidya Sagar 2023-06-27 10:48 ` Krzysztof Wilczyński ` (2 more replies) 1 sibling, 3 replies; 25+ messages in thread From: Vidya Sagar @ 2023-06-19 10:26 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin Cc: linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload"). Consider a PCIe hierarchy with a PCIe switch and a device connected downstream of the switch that has support for MPS which is the minimum in the hierarchy, and root port programmed with an MPS in its DevCtl register that is greater than the minimum. In this scenario, the default bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the MPS settings in the hierarchy correctly resulting in the device with support for minimum MPS in the hierarchy receiving the TLPs of size more than that. Although this can be addressed by appending "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a good idea to always have this commandline argument even for the basic functionality to work. Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") avoids this requirement and ensures that the basic functionality of the devices irrespective of the hierarchy and the MPS of the devices in the hierarchy. To reap the benefits of having support for higher MPS, optionally, one can always append the kernel command line with "pci=pcie_bus_perf". Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- V3: * Fixed a build issue V2: * Addressed review comments from Bjorn drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 4fdadc7b045f..a772faff14b5 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) struct device *dev = pcie->dev; u32 val; int ret; - u16 val_16; if (pcie->ep_state == EP_STATE_ENABLED) return; @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - /* Clear Slot Clock Configuration bit if SRNS configuration */ if (pcie->enable_srns) { + u16 val_16; + val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA); val_16 &= ~PCI_EXP_LNKSTA_SLC; dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, val_16); } - clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-19 10:26 ` [PATCH V3] " Vidya Sagar @ 2023-06-27 10:48 ` Krzysztof Wilczyński 2023-07-13 21:39 ` Bjorn Helgaas 2023-07-18 2:52 ` [PATCH V4] " Vidya Sagar 2 siblings, 0 replies; 25+ messages in thread From: Krzysztof Wilczyński @ 2023-06-27 10:48 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv Hello, > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > support for 256 Byte payload"). > > Consider a PCIe hierarchy with a PCIe switch and a device connected > downstream of the switch that has support for MPS which is the minimum in > the hierarchy, and root port programmed with an MPS in its DevCtl register > that is greater than the minimum. In this scenario, the default bus > configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the > MPS settings in the hierarchy correctly resulting in the device with > support for minimum MPS in the hierarchy receiving the TLPs of size more > than that. Although this can be addressed by appending "pci=pcie_bus_safe" > to the kernel command line, it doesn't seem to be a good idea to always > have this commandline argument even for the basic functionality to work. > > Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 > Byte payload") avoids this requirement and ensures that the basic > functionality of the devices irrespective of the hierarchy and the MPS of > the devices in the hierarchy. > > To reap the benefits of having support for higher MPS, optionally, one can > always append the kernel command line with "pci=pcie_bus_perf". Applied to controller/tegra194, thank you! [1/1] Revert "PCI: tegra194: Enable support for 256 Byte payload" https://git.kernel.org/pci/pci/c/606295f16360 Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-19 10:26 ` [PATCH V3] " Vidya Sagar 2023-06-27 10:48 ` Krzysztof Wilczyński @ 2023-07-13 21:39 ` Bjorn Helgaas 2023-07-18 2:33 ` Vidya Sagar 2023-07-18 2:52 ` [PATCH V4] " Vidya Sagar 2 siblings, 1 reply; 25+ messages in thread From: Bjorn Helgaas @ 2023-07-13 21:39 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Mon, Jun 19, 2023 at 03:56:04PM +0530, Vidya Sagar wrote: > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > support for 256 Byte payload"). > > Consider a PCIe hierarchy with a PCIe switch and a device connected > downstream of the switch that has support for MPS which is the minimum in > the hierarchy, and root port programmed with an MPS in its DevCtl register > that is greater than the minimum. In this scenario, the default bus > configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the > MPS settings in the hierarchy correctly resulting in the device with > support for minimum MPS in the hierarchy receiving the TLPs of size more > than that. Although this can be addressed by appending "pci=pcie_bus_safe" > to the kernel command line, it doesn't seem to be a good idea to always > have this commandline argument even for the basic functionality to work. I think this has some irrelevant detail (IIUC the problem should happen even without a switch) and could be more specific (I think the problem case is RP MPS=256, EP only supports MPS=128). > Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 > Byte payload") avoids this requirement and ensures that the basic > functionality of the devices irrespective of the hierarchy and the MPS of > the devices in the hierarchy. "Ensure" is a transitive verb, so "... ensures that the basic functionality ..." is missing whatever the object should be. Maybe something like the following? After 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload"), we set MPS=256 for tegra194 Root Ports. By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" parameter), Linux configures the MPS of every device to match the upstream bridge, which is impossible if the Root Port has MPS=256 and a device only supports MPS=128. This scenario results in uncorrectable Malformed TLP errors if the Root Port sends TLPs with payloads larger than 128 bytes. These errors can be avoided by using the "pci=pcie_bus_safe" parameter, but it doesn't seem to be a good idea to always have this parameter even for basic functionality to work. Revert 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") so the Root Ports default to MPS=128, which all devices support. If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" to get the benefit of larger MPS settings. > To reap the benefits of having support for higher MPS, optionally, one can > always append the kernel command line with "pci=pcie_bus_perf". > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V3: > * Fixed a build issue > > V2: > * Addressed review comments from Bjorn > > drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 4fdadc7b045f..a772faff14b5 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > struct device *dev = pcie->dev; > u32 val; > int ret; > - u16 val_16; > > if (pcie->ep_state == EP_STATE_ENABLED) > return; > @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > /* Clear Slot Clock Configuration bit if SRNS configuration */ > if (pcie->enable_srns) { > + u16 val_16; > + > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > PCI_EXP_LNKSTA); > val_16 &= ~PCI_EXP_LNKSTA_SLC; > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > val_16); > } > - > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-13 21:39 ` Bjorn Helgaas @ 2023-07-18 2:33 ` Vidya Sagar 2023-07-18 11:09 ` Bjorn Helgaas 0 siblings, 1 reply; 25+ messages in thread From: Vidya Sagar @ 2023-07-18 2:33 UTC (permalink / raw) To: Bjorn Helgaas Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On 7/14/2023 3:09 AM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > On Mon, Jun 19, 2023 at 03:56:04PM +0530, Vidya Sagar wrote: >> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable >> support for 256 Byte payload"). >> >> Consider a PCIe hierarchy with a PCIe switch and a device connected >> downstream of the switch that has support for MPS which is the minimum in >> the hierarchy, and root port programmed with an MPS in its DevCtl register >> that is greater than the minimum. In this scenario, the default bus >> configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the >> MPS settings in the hierarchy correctly resulting in the device with >> support for minimum MPS in the hierarchy receiving the TLPs of size more >> than that. Although this can be addressed by appending "pci=pcie_bus_safe" >> to the kernel command line, it doesn't seem to be a good idea to always >> have this commandline argument even for the basic functionality to work. > > I think this has some irrelevant detail (IIUC the problem should > happen even without a switch) and could be more specific (I think the > problem case is RP MPS=256, EP only supports MPS=128). The issue is present only if there is a switch. > >> Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 >> Byte payload") avoids this requirement and ensures that the basic >> functionality of the devices irrespective of the hierarchy and the MPS of >> the devices in the hierarchy. > > "Ensure" is a transitive verb, so "... ensures that the basic > functionality ..." is missing whatever the object should be. I think I missed to add 'work' in the end after 'hierarchy'. My bad. > > Maybe something like the following? > > After 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we set MPS=256 for tegra194 Root Ports. > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > parameter), Linux configures the MPS of every device to match the > upstream bridge, which is impossible if the Root Port has MPS=256 > and a device only supports MPS=128. > > This scenario results in uncorrectable Malformed TLP errors if the > Root Port sends TLPs with payloads larger than 128 bytes. These > errors can be avoided by using the "pci=pcie_bus_safe" parameter, > but it doesn't seem to be a good idea to always have this parameter > even for basic functionality to work. > > Revert 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload") so the Root Ports default to MPS=128, which all devices > support. > > If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" > to get the benefit of larger MPS settings. Thanks, I'll send a new patch with the above commit message. > >> To reap the benefits of having support for higher MPS, optionally, one can >> always append the kernel command line with "pci=pcie_bus_perf". >> >> Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> V3: >> * Fixed a build issue >> >> V2: >> * Addressed review comments from Bjorn >> >> drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 4fdadc7b045f..a772faff14b5 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); >> val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); >> dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); >> @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> struct device *dev = pcie->dev; >> u32 val; >> int ret; >> - u16 val_16; >> >> if (pcie->ep_state == EP_STATE_ENABLED) >> return; >> @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> /* Clear Slot Clock Configuration bit if SRNS configuration */ >> if (pcie->enable_srns) { >> + u16 val_16; >> + >> val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + >> PCI_EXP_LNKSTA); >> val_16 &= ~PCI_EXP_LNKSTA_SLC; >> dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, >> val_16); >> } >> - >> clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); >> >> val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-18 2:33 ` Vidya Sagar @ 2023-07-18 11:09 ` Bjorn Helgaas 2023-07-19 11:01 ` Vidya Sagar 0 siblings, 1 reply; 25+ messages in thread From: Bjorn Helgaas @ 2023-07-18 11:09 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Tue, Jul 18, 2023 at 08:03:47AM +0530, Vidya Sagar wrote: > On 7/14/2023 3:09 AM, Bjorn Helgaas wrote: > > On Mon, Jun 19, 2023 at 03:56:04PM +0530, Vidya Sagar wrote: > > > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > > > support for 256 Byte payload"). > > > > > > Consider a PCIe hierarchy with a PCIe switch and a device connected > > > downstream of the switch that has support for MPS which is the minimum in > > > the hierarchy, and root port programmed with an MPS in its DevCtl register > > > that is greater than the minimum. In this scenario, the default bus > > > configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the > > > MPS settings in the hierarchy correctly resulting in the device with > > > support for minimum MPS in the hierarchy receiving the TLPs of size more > > > than that. Although this can be addressed by appending "pci=pcie_bus_safe" > > > to the kernel command line, it doesn't seem to be a good idea to always > > > have this commandline argument even for the basic functionality to work. > > > > I think this has some irrelevant detail (IIUC the problem should > > happen even without a switch) and could be more specific (I think the > > problem case is RP MPS=256, EP only supports MPS=128). > > The issue is present only if there is a switch. So if there's no switch, and an EP that only supports MPS=128, the PCI core changes the RP MPS setting to 128? Just based on reading the code, I thought we would leave RP MPS=256 and EP MPS=128, which would be a problem. But maybe the PCI core changes the RP down to MPS=128? Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-18 11:09 ` Bjorn Helgaas @ 2023-07-19 11:01 ` Vidya Sagar 2023-07-19 14:57 ` Bjorn Helgaas 0 siblings, 1 reply; 25+ messages in thread From: Vidya Sagar @ 2023-07-19 11:01 UTC (permalink / raw) To: Bjorn Helgaas Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On 7/18/2023 4:39 PM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > On Tue, Jul 18, 2023 at 08:03:47AM +0530, Vidya Sagar wrote: >> On 7/14/2023 3:09 AM, Bjorn Helgaas wrote: >>> On Mon, Jun 19, 2023 at 03:56:04PM +0530, Vidya Sagar wrote: >>>> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable >>>> support for 256 Byte payload"). >>>> >>>> Consider a PCIe hierarchy with a PCIe switch and a device connected >>>> downstream of the switch that has support for MPS which is the minimum in >>>> the hierarchy, and root port programmed with an MPS in its DevCtl register >>>> that is greater than the minimum. In this scenario, the default bus >>>> configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the >>>> MPS settings in the hierarchy correctly resulting in the device with >>>> support for minimum MPS in the hierarchy receiving the TLPs of size more >>>> than that. Although this can be addressed by appending "pci=pcie_bus_safe" >>>> to the kernel command line, it doesn't seem to be a good idea to always >>>> have this commandline argument even for the basic functionality to work. >>> >>> I think this has some irrelevant detail (IIUC the problem should >>> happen even without a switch) and could be more specific (I think the >>> problem case is RP MPS=256, EP only supports MPS=128). >> >> The issue is present only if there is a switch. > > So if there's no switch, and an EP that only supports MPS=128, the PCI > core changes the RP MPS setting to 128? Just based on reading the Yes. The code after the if condition here takes care of that. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/probe.c?h=v6.5-rc2#n2049 > code, I thought we would leave RP MPS=256 and EP MPS=128, which would > be a problem. But maybe the PCI core changes the RP down to MPS=128? > > Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V3] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-19 11:01 ` Vidya Sagar @ 2023-07-19 14:57 ` Bjorn Helgaas 0 siblings, 0 replies; 25+ messages in thread From: Bjorn Helgaas @ 2023-07-19 14:57 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Wed, Jul 19, 2023 at 04:31:13PM +0530, Vidya Sagar wrote: > On 7/18/2023 4:39 PM, Bjorn Helgaas wrote: > > On Tue, Jul 18, 2023 at 08:03:47AM +0530, Vidya Sagar wrote: > > > On 7/14/2023 3:09 AM, Bjorn Helgaas wrote: > > > > On Mon, Jun 19, 2023 at 03:56:04PM +0530, Vidya Sagar wrote: > > > > > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > > > > > support for 256 Byte payload"). > > > > > > > > > > Consider a PCIe hierarchy with a PCIe switch and a device connected > > > > > downstream of the switch that has support for MPS which is the minimum in > > > > > the hierarchy, and root port programmed with an MPS in its DevCtl register > > > > > that is greater than the minimum. In this scenario, the default bus > > > > > configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't configure the > > > > > MPS settings in the hierarchy correctly resulting in the device with > > > > > support for minimum MPS in the hierarchy receiving the TLPs of size more > > > > > than that. Although this can be addressed by appending "pci=pcie_bus_safe" > > > > > to the kernel command line, it doesn't seem to be a good idea to always > > > > > have this commandline argument even for the basic functionality to work. > > > > > > > > I think this has some irrelevant detail (IIUC the problem should > > > > happen even without a switch) and could be more specific (I think the > > > > problem case is RP MPS=256, EP only supports MPS=128). > > > > > > The issue is present only if there is a switch. > > > > So if there's no switch, and an EP that only supports MPS=128, the PCI > > core changes the RP MPS setting to 128? Just based on reading the > > Yes. The code after the if condition here takes care of that. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/probe.c?h=v6.5-rc2#n2049 Oh, right, thanks. I vaguely remember the logic that if the immediate parent is a Root Port, there are no other branches in the hierarchy to worry about, so we can just configure the Root Port MPS to match the device. > > code, I thought we would leave RP MPS=256 and EP MPS=128, which would > > be a problem. But maybe the PCI core changes the RP down to MPS=128? > > > > Bjorn ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-19 10:26 ` [PATCH V3] " Vidya Sagar 2023-06-27 10:48 ` Krzysztof Wilczyński 2023-07-13 21:39 ` Bjorn Helgaas @ 2023-07-18 2:52 ` Vidya Sagar 2023-07-21 8:23 ` Jon Hunter 2023-08-01 20:40 ` Bjorn Helgaas 2 siblings, 2 replies; 25+ messages in thread From: Vidya Sagar @ 2023-07-18 2:52 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin Cc: linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, vidyas, sagar.tv After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload"), we set MPS=256 for tegra194 Root Ports. By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" parameter), Linux configures the MPS of every device to match the upstream bridge, which is impossible if the Root Port has MPS=256 and a device only supports MPS=128. This scenario results in uncorrectable Malformed TLP errors if the Root Port sends TLPs with payloads larger than 128 bytes. These errors can be avoided by using the "pci=pcie_bus_safe" parameter, but it doesn't seem to be a good idea to always have this parameter even for basic functionality to work. Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") so the Root Ports default to MPS=128, which all devices support. If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" to get the benefit of larger MPS settings. [ rewrote commit message based on Bjorn's suggestion ] Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- V4: * Rewrote commit message based on Bjorn's suggestion V3: * Fixed a build issue V2: * Addressed review comments from Bjorn drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 4fdadc7b045f..a772faff14b5 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) struct device *dev = pcie->dev; u32 val; int ret; - u16 val_16; if (pcie->ep_state == EP_STATE_ENABLED) return; @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, PCI_CAP_ID_EXP); - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); - /* Clear Slot Clock Configuration bit if SRNS configuration */ if (pcie->enable_srns) { + u16 val_16; + val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA); val_16 &= ~PCI_EXP_LNKSTA_SLC; dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, val_16); } - clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-18 2:52 ` [PATCH V4] " Vidya Sagar @ 2023-07-21 8:23 ` Jon Hunter 2023-07-21 10:35 ` Bjorn Helgaas 2023-08-01 20:40 ` Bjorn Helgaas 1 sibling, 1 reply; 25+ messages in thread From: Jon Hunter @ 2023-07-21 8:23 UTC (permalink / raw) To: Vidya Sagar, lpieralisi, kw, robh, bhelgaas, thierry.reding, Sergey.Semin Cc: linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv Hi Krzysztof, Bjorn, On 18/07/2023 03:52, Vidya Sagar wrote: > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we set MPS=256 for tegra194 Root Ports. > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > parameter), Linux configures the MPS of every device to match the > upstream bridge, which is impossible if the Root Port has MPS=256 > and a device only supports MPS=128. > > This scenario results in uncorrectable Malformed TLP errors if the > Root Port sends TLPs with payloads larger than 128 bytes. These > errors can be avoided by using the "pci=pcie_bus_safe" parameter, > but it doesn't seem to be a good idea to always have this parameter > even for basic functionality to work. > > Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload") so the Root Ports default to MPS=128, which all devices > support. > > If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" > to get the benefit of larger MPS settings. > > [ rewrote commit message based on Bjorn's suggestion ] > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V4: > * Rewrote commit message based on Bjorn's suggestion > > V3: > * Fixed a build issue > > V2: > * Addressed review comments from Bjorn > > drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 4fdadc7b045f..a772faff14b5 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > struct device *dev = pcie->dev; > u32 val; > int ret; > - u16 val_16; > > if (pcie->ep_state == EP_STATE_ENABLED) > return; > @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > /* Clear Slot Clock Configuration bit if SRNS configuration */ > if (pcie->enable_srns) { > + u16 val_16; > + > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > PCI_EXP_LNKSTA); > val_16 &= ~PCI_EXP_LNKSTA_SLC; > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > val_16); > } > - > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); I see a version of this patch here ... https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/tegra194 However, I don't see this in -next yet. If you are happy with this latest version, could we get this into -next? FWIW ... Acked-by: Jon Hunter <jonathanh@nvidia.com> Thanks! Jon -- nvpublic ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-21 8:23 ` Jon Hunter @ 2023-07-21 10:35 ` Bjorn Helgaas 2023-07-28 12:44 ` Jon Hunter 0 siblings, 1 reply; 25+ messages in thread From: Bjorn Helgaas @ 2023-07-21 10:35 UTC (permalink / raw) To: Jon Hunter Cc: Vidya Sagar, lpieralisi, kw, robh, bhelgaas, thierry.reding, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Fri, Jul 21, 2023 at 09:23:01AM +0100, Jon Hunter wrote: > ... > I see a version of this patch here ... > > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/tegra194 > > However, I don't see this in -next yet. If you are happy with this latest > version, could we get this into -next? I'm on vacation until Tuesday; will build a new -next branch Tuesday or Wednesday. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-21 10:35 ` Bjorn Helgaas @ 2023-07-28 12:44 ` Jon Hunter 0 siblings, 0 replies; 25+ messages in thread From: Jon Hunter @ 2023-07-28 12:44 UTC (permalink / raw) To: Bjorn Helgaas Cc: Vidya Sagar, lpieralisi, kw, robh, bhelgaas, thierry.reding, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv Hi Bjorn, On 21/07/2023 11:35, Bjorn Helgaas wrote: > On Fri, Jul 21, 2023 at 09:23:01AM +0100, Jon Hunter wrote: >> ... > >> I see a version of this patch here ... >> >> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/tegra194 >> >> However, I don't see this in -next yet. If you are happy with this latest >> version, could we get this into -next? > > I'm on vacation until Tuesday; will build a new -next branch Tuesday > or Wednesday. A friendly reminder on this. Can we queue this up for -next? I know that there is further discussion on-going about if the core could handle this, but right now PCIe is broken on our NVIDIA IGX Orin board and we would like to merge this now in the short-term at least. Thanks! Jon -- nvpublic ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-18 2:52 ` [PATCH V4] " Vidya Sagar 2023-07-21 8:23 ` Jon Hunter @ 2023-08-01 20:40 ` Bjorn Helgaas 2023-08-03 9:19 ` Vidya Sagar 1 sibling, 1 reply; 25+ messages in thread From: Bjorn Helgaas @ 2023-08-01 20:40 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Tue, Jul 18, 2023 at 08:22:21AM +0530, Vidya Sagar wrote: > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we set MPS=256 for tegra194 Root Ports. > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > parameter), Linux configures the MPS of every device to match the > upstream bridge, which is impossible if the Root Port has MPS=256 > and a device only supports MPS=128. Thanks for pointing out that I broke this log by omitting the mention of a switch. Is the rewording below better? If so, Krzysztof can amend the commit. After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload"), we initialize MPS=256 for tegra194 Root Ports before enumerating the hierarchy. Consider an Endpoint that supports only MPS=128. In the default situation (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" parameter), Linux tries to configure the MPS of every device to match the upstream bridge. If the Endpoint is directly below the Root Port, Linux can reduce the Root Port MPS to 128 to match the Endpoint. But if there's a switch in the middle, Linux doesn't reduce the Root Port MPS because other devices below the switch may already be configured with MPS larger than 128. > This scenario results in uncorrectable Malformed TLP errors if the > Root Port sends TLPs with payloads larger than 128 bytes. These > errors can be avoided by using the "pci=pcie_bus_safe" parameter, > but it doesn't seem to be a good idea to always have this parameter > even for basic functionality to work. > > Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload") so the Root Ports default to MPS=128, which all devices > support. > > If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" > to get the benefit of larger MPS settings. > > [ rewrote commit message based on Bjorn's suggestion ] > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") 4fb8e46c1bc4 appeared in v6.0-rc1, so this wouldn't be a candidate for v6.5, but it does sound like it should be tagged for stable? If so, Krzysztof can probably add that as well. > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > V4: > * Rewrote commit message based on Bjorn's suggestion > > V3: > * Fixed a build issue > > V2: > * Addressed review comments from Bjorn > > drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 4fdadc7b045f..a772faff14b5 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > struct device *dev = pcie->dev; > u32 val; > int ret; > - u16 val_16; > > if (pcie->ep_state == EP_STATE_ENABLED) > return; > @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > /* Clear Slot Clock Configuration bit if SRNS configuration */ > if (pcie->enable_srns) { > + u16 val_16; > + > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > PCI_EXP_LNKSTA); > val_16 &= ~PCI_EXP_LNKSTA_SLC; > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > val_16); > } > - > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-08-01 20:40 ` Bjorn Helgaas @ 2023-08-03 9:19 ` Vidya Sagar 2023-08-03 18:50 ` Krzysztof Wilczyński 0 siblings, 1 reply; 25+ messages in thread From: Vidya Sagar @ 2023-08-03 9:19 UTC (permalink / raw) To: Bjorn Helgaas Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On 8/2/2023 2:10 AM, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > On Tue, Jul 18, 2023 at 08:22:21AM +0530, Vidya Sagar wrote: >> After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte >> payload"), we set MPS=256 for tegra194 Root Ports. >> >> By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" >> parameter), Linux configures the MPS of every device to match the >> upstream bridge, which is impossible if the Root Port has MPS=256 >> and a device only supports MPS=128. > > Thanks for pointing out that I broke this log by omitting the mention > of a switch. Is the rewording below better? If so, Krzysztof can > amend the commit. Yes. The below rewording looks good. Thanks, Vidya Sagar > > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > payload"), we initialize MPS=256 for tegra194 Root Ports before enumerating > the hierarchy. > > Consider an Endpoint that supports only MPS=128. In the default situation > (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" parameter), Linux > tries to configure the MPS of every device to match the upstream bridge. > If the Endpoint is directly below the Root Port, Linux can reduce the Root > Port MPS to 128 to match the Endpoint. But if there's a switch in the > middle, Linux doesn't reduce the Root Port MPS because other devices below > the switch may already be configured with MPS larger than 128. > >> This scenario results in uncorrectable Malformed TLP errors if the >> Root Port sends TLPs with payloads larger than 128 bytes. These >> errors can be avoided by using the "pci=pcie_bus_safe" parameter, >> but it doesn't seem to be a good idea to always have this parameter >> even for basic functionality to work. >> >> Revert commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte >> payload") so the Root Ports default to MPS=128, which all devices >> support. >> >> If peer-to-peer DMA is not required, one can use "pci=pcie_bus_perf" >> to get the benefit of larger MPS settings. >> >> [ rewrote commit message based on Bjorn's suggestion ] >> >> Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > > 4fb8e46c1bc4 appeared in v6.0-rc1, so this wouldn't be a candidate for > v6.5, but it does sound like it should be tagged for stable? If so, > Krzysztof can probably add that as well. > >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> V4: >> * Rewrote commit message based on Bjorn's suggestion >> >> V3: >> * Fixed a build issue >> >> V2: >> * Addressed review comments from Bjorn >> >> drivers/pci/controller/dwc/pcie-tegra194.c | 14 ++------------ >> 1 file changed, 2 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 4fdadc7b045f..a772faff14b5 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -900,11 +900,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); >> val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); >> dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); >> @@ -1756,7 +1751,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> struct device *dev = pcie->dev; >> u32 val; >> int ret; >> - u16 val_16; >> >> if (pcie->ep_state == EP_STATE_ENABLED) >> return; >> @@ -1887,20 +1881,16 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> /* Clear Slot Clock Configuration bit if SRNS configuration */ >> if (pcie->enable_srns) { >> + u16 val_16; >> + >> val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + >> PCI_EXP_LNKSTA); >> val_16 &= ~PCI_EXP_LNKSTA_SLC; >> dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, >> val_16); >> } >> - >> clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); >> >> val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V4] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-08-03 9:19 ` Vidya Sagar @ 2023-08-03 18:50 ` Krzysztof Wilczyński 0 siblings, 0 replies; 25+ messages in thread From: Krzysztof Wilczyński @ 2023-08-03 18:50 UTC (permalink / raw) To: Vidya Sagar Cc: Bjorn Helgaas, lpieralisi, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv Hello! [...] > > > After commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte > > > payload"), we set MPS=256 for tegra194 Root Ports. > > > > > > By default (CONFIG_PCIE_BUS_DEFAULT set and no "pci=pcie_bus_*" > > > parameter), Linux configures the MPS of every device to match the > > > upstream bridge, which is impossible if the Root Port has MPS=256 > > > and a device only supports MPS=128. > > > > Thanks for pointing out that I broke this log by omitting the mention > > of a switch. Is the rewording below better? If so, Krzysztof can > > amend the commit. > Yes. The below rewording looks good. Updated commit at: https://git.kernel.org/pci/pci/c/ebfde1584d9f Thank you everyone! Krzysztof ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-06-08 9:36 [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" Vidya Sagar 2023-06-08 16:33 ` Bjorn Helgaas 2023-06-19 1:42 ` [PATCH V2] " Vidya Sagar @ 2023-07-25 7:51 ` Manivannan Sadhasivam 2023-07-25 8:19 ` Vidya Sagar 2 siblings, 1 reply; 25+ messages in thread From: Manivannan Sadhasivam @ 2023-07-25 7:51 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > support for 256 Byte payload") > > Consider a PCIe hierarchy with a PCIe switch and a device connected > downstream of the switch that has support for MPS which is the minimum > in the hierarchy, and root port programmed with an MPS in its DevCtl > register that is greater than the minimum. In this scenario, the default > bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't > configure the MPS settings in the hierarchy correctly resulting in the > device with support for minimum MPS in the hierarchy receiving the TLPs > of size more than that. Although this can be addresed by appending > "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a > good idea to always have this commandline argument even for the basic > functionality to work. > Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 > Byte payload") avoids this requirement and ensures that the basic > functionality of the devices irrespective of the hierarchy and the MPS of > the devices in the hierarchy. > To reap the benefits of having support for higher MPS, optionally, one can > always append the kernel command line with "pci=pcie_bus_perf". > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> I know that this patch is merged. But I happen to test a similar change on Qcom platform during a patch review and found that the PCI core changes MPS to 128 when a 128byte supported device is found: [ 3.174290] pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 128 (was 256, max 128) [ 3.186538] pci 0000:01:00.0: Max Payload Size set to 128 (was 128, max 128) This was just randomly tested on a platform whose Root port DEVCAP was 128, but it shouldn't matter. And I didn't change the default bus configuration. Wondering how you ended up facing issues with it. - Mani > --- > drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- > 1 file changed, 13 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 4fdadc7b045f..877d81b13334 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); > u32 val; > - u16 val_16; > > pp->bridge->ops = &tegra_pci_ops; > > @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > struct device *dev = pcie->dev; > u32 val; > int ret; > - u16 val_16; > > if (pcie->ep_state == EP_STATE_ENABLED) > return; > @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > PCI_CAP_ID_EXP); > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > - > /* Clear Slot Clock Configuration bit if SRNS configuration */ > if (pcie->enable_srns) { > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > val_16); > } > - > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); > -- > 2.25.1 > -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-25 7:51 ` [PATCH V1] " Manivannan Sadhasivam @ 2023-07-25 8:19 ` Vidya Sagar 2023-07-25 8:30 ` Manivannan Sadhasivam 0 siblings, 1 reply; 25+ messages in thread From: Vidya Sagar @ 2023-07-25 8:19 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On 7/25/2023 1:21 PM, Manivannan Sadhasivam wrote: > External email: Use caution opening links or attachments > > > On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: >> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable >> support for 256 Byte payload") >> >> Consider a PCIe hierarchy with a PCIe switch and a device connected >> downstream of the switch that has support for MPS which is the minimum >> in the hierarchy, and root port programmed with an MPS in its DevCtl >> register that is greater than the minimum. In this scenario, the default >> bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't >> configure the MPS settings in the hierarchy correctly resulting in the >> device with support for minimum MPS in the hierarchy receiving the TLPs >> of size more than that. Although this can be addresed by appending >> "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a >> good idea to always have this commandline argument even for the basic >> functionality to work. >> Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 >> Byte payload") avoids this requirement and ensures that the basic >> functionality of the devices irrespective of the hierarchy and the MPS of >> the devices in the hierarchy. >> To reap the benefits of having support for higher MPS, optionally, one can >> always append the kernel command line with "pci=pcie_bus_perf". >> >> Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > I know that this patch is merged. But I happen to test a similar change on Qcom > platform during a patch review and found that the PCI core changes MPS to 128 > when a 128byte supported device is found: > > [ 3.174290] pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 128 (was 256, max 128) > [ 3.186538] pci 0000:01:00.0: Max Payload Size set to 128 (was 128, max 128) > > This was just randomly tested on a platform whose Root port DEVCAP was 128, but > it shouldn't matter. And I didn't change the default bus configuration. > > Wondering how you ended up facing issues with it. If the endpiont device that has support only for 128byte MPS is connected directly to the root port, then, I agree that the PCIe sub-system takes care of changing the MPS to the common lowest MPS, but if the endpoint device is connected behind a PCIe switch, then the PCIe subsystem doesn't configure the MPS properly. -Vidya Sagar > > - Mani > >> --- >> drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- >> 1 file changed, 13 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 4fdadc7b045f..877d81b13334 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); >> u32 val; >> - u16 val_16; >> >> pp->bridge->ops = &tegra_pci_ops; >> >> @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); >> val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); >> dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); >> @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> struct device *dev = pcie->dev; >> u32 val; >> int ret; >> - u16 val_16; >> >> if (pcie->ep_state == EP_STATE_ENABLED) >> return; >> @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >> PCI_CAP_ID_EXP); >> >> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >> - >> /* Clear Slot Clock Configuration bit if SRNS configuration */ >> if (pcie->enable_srns) { >> val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + >> @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >> dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, >> val_16); >> } >> - >> clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); >> >> val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); >> -- >> 2.25.1 >> > > -- > மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-25 8:19 ` Vidya Sagar @ 2023-07-25 8:30 ` Manivannan Sadhasivam 2023-07-25 9:21 ` Vidya Sagar 0 siblings, 1 reply; 25+ messages in thread From: Manivannan Sadhasivam @ 2023-07-25 8:30 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Tue, Jul 25, 2023 at 01:49:35PM +0530, Vidya Sagar wrote: > > > On 7/25/2023 1:21 PM, Manivannan Sadhasivam wrote: > > External email: Use caution opening links or attachments > > > > > > On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: > > > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > > > support for 256 Byte payload") > > > > > > Consider a PCIe hierarchy with a PCIe switch and a device connected > > > downstream of the switch that has support for MPS which is the minimum > > > in the hierarchy, and root port programmed with an MPS in its DevCtl > > > register that is greater than the minimum. In this scenario, the default > > > bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't > > > configure the MPS settings in the hierarchy correctly resulting in the > > > device with support for minimum MPS in the hierarchy receiving the TLPs > > > of size more than that. Although this can be addresed by appending > > > "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a > > > good idea to always have this commandline argument even for the basic > > > functionality to work. > > > Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 > > > Byte payload") avoids this requirement and ensures that the basic > > > functionality of the devices irrespective of the hierarchy and the MPS of > > > the devices in the hierarchy. > > > To reap the benefits of having support for higher MPS, optionally, one can > > > always append the kernel command line with "pci=pcie_bus_perf". > > > > > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > > > I know that this patch is merged. But I happen to test a similar change on Qcom > > platform during a patch review and found that the PCI core changes MPS to 128 > > when a 128byte supported device is found: > > > > [ 3.174290] pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 128 (was 256, max 128) > > [ 3.186538] pci 0000:01:00.0: Max Payload Size set to 128 (was 128, max 128) > > > > This was just randomly tested on a platform whose Root port DEVCAP was 128, but > > it shouldn't matter. And I didn't change the default bus configuration. > > > > Wondering how you ended up facing issues with it. > > If the endpiont device that has support only for 128byte MPS is connected > directly to the root port, then, I agree that the PCIe sub-system takes care > of changing the MPS to the common lowest MPS, but if the endpoint device is > connected behind a PCIe switch, then the PCIe subsystem doesn't configure > the MPS properly. > Ah, I missed the fact that your issue only happens with a PCIe switch. But shouldn't this be fixed in the PCI core instead? I mean, PCI core should use 128byte in your case for Root port unless it is not allowed in the spec (which I doubt). - Mani > -Vidya Sagar > > > > > - Mani > > > > > --- > > > drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- > > > 1 file changed, 13 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > > > index 4fdadc7b045f..877d81b13334 100644 > > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > > > @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > > struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); > > > u32 val; > > > - u16 val_16; > > > > > > pp->bridge->ops = &tegra_pci_ops; > > > > > > @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > > > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > > > PCI_CAP_ID_EXP); > > > > > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > > > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > > > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > > > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > > > - > > > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > > > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > > > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > > > @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > > > struct device *dev = pcie->dev; > > > u32 val; > > > int ret; > > > - u16 val_16; > > > > > > if (pcie->ep_state == EP_STATE_ENABLED) > > > return; > > > @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > > > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > > > PCI_CAP_ID_EXP); > > > > > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > > > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > > > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > > > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > > > - > > > /* Clear Slot Clock Configuration bit if SRNS configuration */ > > > if (pcie->enable_srns) { > > > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > > > @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > > > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > > > val_16); > > > } > > > - > > > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > > > > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); > > > -- > > > 2.25.1 > > > > > > > -- > > மணிவண்ணன் சதாசிவம் -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-25 8:30 ` Manivannan Sadhasivam @ 2023-07-25 9:21 ` Vidya Sagar 2023-07-25 10:03 ` Manivannan Sadhasivam 0 siblings, 1 reply; 25+ messages in thread From: Vidya Sagar @ 2023-07-25 9:21 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On 7/25/2023 2:00 PM, Manivannan Sadhasivam wrote: > External email: Use caution opening links or attachments > > > On Tue, Jul 25, 2023 at 01:49:35PM +0530, Vidya Sagar wrote: >> >> >> On 7/25/2023 1:21 PM, Manivannan Sadhasivam wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: >>>> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable >>>> support for 256 Byte payload") >>>> >>>> Consider a PCIe hierarchy with a PCIe switch and a device connected >>>> downstream of the switch that has support for MPS which is the minimum >>>> in the hierarchy, and root port programmed with an MPS in its DevCtl >>>> register that is greater than the minimum. In this scenario, the default >>>> bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't >>>> configure the MPS settings in the hierarchy correctly resulting in the >>>> device with support for minimum MPS in the hierarchy receiving the TLPs >>>> of size more than that. Although this can be addresed by appending >>>> "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a >>>> good idea to always have this commandline argument even for the basic >>>> functionality to work. >>>> Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 >>>> Byte payload") avoids this requirement and ensures that the basic >>>> functionality of the devices irrespective of the hierarchy and the MPS of >>>> the devices in the hierarchy. >>>> To reap the benefits of having support for higher MPS, optionally, one can >>>> always append the kernel command line with "pci=pcie_bus_perf". >>>> >>>> Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") >>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >>> >>> I know that this patch is merged. But I happen to test a similar change on Qcom >>> platform during a patch review and found that the PCI core changes MPS to 128 >>> when a 128byte supported device is found: >>> >>> [ 3.174290] pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 128 (was 256, max 128) >>> [ 3.186538] pci 0000:01:00.0: Max Payload Size set to 128 (was 128, max 128) >>> >>> This was just randomly tested on a platform whose Root port DEVCAP was 128, but >>> it shouldn't matter. And I didn't change the default bus configuration. >>> >>> Wondering how you ended up facing issues with it. >> >> If the endpiont device that has support only for 128byte MPS is connected >> directly to the root port, then, I agree that the PCIe sub-system takes care >> of changing the MPS to the common lowest MPS, but if the endpoint device is >> connected behind a PCIe switch, then the PCIe subsystem doesn't configure >> the MPS properly. >> > > Ah, I missed the fact that your issue only happens with a PCIe switch. But > shouldn't this be fixed in the PCI core instead? > > I mean, PCI core should use 128byte in your case for Root port unless it is not > allowed in the spec (which I doubt). well, if the RP's DevCtl is set to 256MPS by default, then, the core wouldn't do it automatically unless either 'pcie_bus_safe' or 'pcie_bus_perf' is passed. > > - Mani > >> -Vidya Sagar >> >>> >>> - Mani >>> >>>> --- >>>> drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- >>>> 1 file changed, 13 deletions(-) >>>> >>>> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c >>>> index 4fdadc7b045f..877d81b13334 100644 >>>> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >>>> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >>>> @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >>>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >>>> struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); >>>> u32 val; >>>> - u16 val_16; >>>> >>>> pp->bridge->ops = &tegra_pci_ops; >>>> >>>> @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) >>>> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >>>> PCI_CAP_ID_EXP); >>>> >>>> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >>>> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >>>> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >>>> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >>>> - >>>> val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); >>>> val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); >>>> dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); >>>> @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >>>> struct device *dev = pcie->dev; >>>> u32 val; >>>> int ret; >>>> - u16 val_16; >>>> >>>> if (pcie->ep_state == EP_STATE_ENABLED) >>>> return; >>>> @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >>>> pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, >>>> PCI_CAP_ID_EXP); >>>> >>>> - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); >>>> - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; >>>> - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; >>>> - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); >>>> - >>>> /* Clear Slot Clock Configuration bit if SRNS configuration */ >>>> if (pcie->enable_srns) { >>>> val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + >>>> @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) >>>> dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, >>>> val_16); >>>> } >>>> - >>>> clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); >>>> >>>> val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); >>>> -- >>>> 2.25.1 >>>> >>> >>> -- >>> மணிவண்ணன் சதாசிவம் > > -- > மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" 2023-07-25 9:21 ` Vidya Sagar @ 2023-07-25 10:03 ` Manivannan Sadhasivam 0 siblings, 0 replies; 25+ messages in thread From: Manivannan Sadhasivam @ 2023-07-25 10:03 UTC (permalink / raw) To: Vidya Sagar Cc: lpieralisi, kw, robh, bhelgaas, thierry.reding, jonathanh, Sergey.Semin, linux-pci, linux-tegra, linux-kernel, kthota, mmaddireddy, sagar.tv On Tue, Jul 25, 2023 at 02:51:10PM +0530, Vidya Sagar wrote: > > > On 7/25/2023 2:00 PM, Manivannan Sadhasivam wrote: > > External email: Use caution opening links or attachments > > > > > > On Tue, Jul 25, 2023 at 01:49:35PM +0530, Vidya Sagar wrote: > > > > > > > > > On 7/25/2023 1:21 PM, Manivannan Sadhasivam wrote: > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote: > > > > > This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable > > > > > support for 256 Byte payload") > > > > > > > > > > Consider a PCIe hierarchy with a PCIe switch and a device connected > > > > > downstream of the switch that has support for MPS which is the minimum > > > > > in the hierarchy, and root port programmed with an MPS in its DevCtl > > > > > register that is greater than the minimum. In this scenario, the default > > > > > bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't > > > > > configure the MPS settings in the hierarchy correctly resulting in the > > > > > device with support for minimum MPS in the hierarchy receiving the TLPs > > > > > of size more than that. Although this can be addresed by appending > > > > > "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a > > > > > good idea to always have this commandline argument even for the basic > > > > > functionality to work. > > > > > Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 > > > > > Byte payload") avoids this requirement and ensures that the basic > > > > > functionality of the devices irrespective of the hierarchy and the MPS of > > > > > the devices in the hierarchy. > > > > > To reap the benefits of having support for higher MPS, optionally, one can > > > > > always append the kernel command line with "pci=pcie_bus_perf". > > > > > > > > > > Fixes: 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256 Byte payload") > > > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > > > > > > > I know that this patch is merged. But I happen to test a similar change on Qcom > > > > platform during a patch review and found that the PCI core changes MPS to 128 > > > > when a 128byte supported device is found: > > > > > > > > [ 3.174290] pci 0000:01:00.0: Upstream bridge's Max Payload Size set to 128 (was 256, max 128) > > > > [ 3.186538] pci 0000:01:00.0: Max Payload Size set to 128 (was 128, max 128) > > > > > > > > This was just randomly tested on a platform whose Root port DEVCAP was 128, but > > > > it shouldn't matter. And I didn't change the default bus configuration. > > > > > > > > Wondering how you ended up facing issues with it. > > > > > > If the endpiont device that has support only for 128byte MPS is connected > > > directly to the root port, then, I agree that the PCIe sub-system takes care > > > of changing the MPS to the common lowest MPS, but if the endpoint device is > > > connected behind a PCIe switch, then the PCIe subsystem doesn't configure > > > the MPS properly. > > > > > > > Ah, I missed the fact that your issue only happens with a PCIe switch. But > > shouldn't this be fixed in the PCI core instead? > > > > I mean, PCI core should use 128byte in your case for Root port unless it is not > > allowed in the spec (which I doubt). > well, if the RP's DevCtl is set to 256MPS by default, then, the core > wouldn't do it automatically unless either 'pcie_bus_safe' or > 'pcie_bus_perf' is passed. > That's what I'm referring to. The default configuration shouldn't cause Root port to send TLPs with unsupported payload. Moreover, this is not the case for immediate children. So definitely the PCI core should exhibit the same behavior for all downstream devices. - Mani > > > > > - Mani > > > > > -Vidya Sagar > > > > > > > > > > > - Mani > > > > > > > > > --- > > > > > drivers/pci/controller/dwc/pcie-tegra194.c | 13 ------------- > > > > > 1 file changed, 13 deletions(-) > > > > > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > > > > > index 4fdadc7b045f..877d81b13334 100644 > > > > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > > > > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > > > > > @@ -892,7 +892,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > > > > > struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > > > > > struct tegra_pcie_dw *pcie = to_tegra_pcie(pci); > > > > > u32 val; > > > > > - u16 val_16; > > > > > > > > > > pp->bridge->ops = &tegra_pci_ops; > > > > > > > > > > @@ -900,11 +899,6 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp) > > > > > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > > > > > PCI_CAP_ID_EXP); > > > > > > > > > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > > > > > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > > > > > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > > > > > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > > > > > - > > > > > val = dw_pcie_readl_dbi(pci, PCI_IO_BASE); > > > > > val &= ~(IO_BASE_IO_DECODE | IO_BASE_IO_DECODE_BIT8); > > > > > dw_pcie_writel_dbi(pci, PCI_IO_BASE, val); > > > > > @@ -1756,7 +1750,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > > > > > struct device *dev = pcie->dev; > > > > > u32 val; > > > > > int ret; > > > > > - u16 val_16; > > > > > > > > > > if (pcie->ep_state == EP_STATE_ENABLED) > > > > > return; > > > > > @@ -1887,11 +1880,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > > > > > pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci, > > > > > PCI_CAP_ID_EXP); > > > > > > > > > > - val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL); > > > > > - val_16 &= ~PCI_EXP_DEVCTL_PAYLOAD; > > > > > - val_16 |= PCI_EXP_DEVCTL_PAYLOAD_256B; > > > > > - dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_DEVCTL, val_16); > > > > > - > > > > > /* Clear Slot Clock Configuration bit if SRNS configuration */ > > > > > if (pcie->enable_srns) { > > > > > val_16 = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + > > > > > @@ -1900,7 +1888,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie) > > > > > dw_pcie_writew_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA, > > > > > val_16); > > > > > } > > > > > - > > > > > clk_set_rate(pcie->core_clk, GEN4_CORE_CLK_FREQ); > > > > > > > > > > val = (ep->msi_mem_phys & MSIX_ADDR_MATCH_LOW_OFF_MASK); > > > > > -- > > > > > 2.25.1 > > > > > > > > > > > > > -- > > > > மணிவண்ணன் சதாசிவம் > > > > -- > > மணிவண்ணன் சதாசிவம் -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2023-08-03 18:51 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-08 9:36 [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte payload" Vidya Sagar 2023-06-08 16:33 ` Bjorn Helgaas 2023-06-09 2:23 ` Vidya Sagar 2023-06-14 10:39 ` Jon Hunter 2023-06-19 1:42 ` [PATCH V2] " Vidya Sagar 2023-06-19 3:42 ` kernel test robot 2023-06-19 10:26 ` [PATCH V3] " Vidya Sagar 2023-06-27 10:48 ` Krzysztof Wilczyński 2023-07-13 21:39 ` Bjorn Helgaas 2023-07-18 2:33 ` Vidya Sagar 2023-07-18 11:09 ` Bjorn Helgaas 2023-07-19 11:01 ` Vidya Sagar 2023-07-19 14:57 ` Bjorn Helgaas 2023-07-18 2:52 ` [PATCH V4] " Vidya Sagar 2023-07-21 8:23 ` Jon Hunter 2023-07-21 10:35 ` Bjorn Helgaas 2023-07-28 12:44 ` Jon Hunter 2023-08-01 20:40 ` Bjorn Helgaas 2023-08-03 9:19 ` Vidya Sagar 2023-08-03 18:50 ` Krzysztof Wilczyński 2023-07-25 7:51 ` [PATCH V1] " Manivannan Sadhasivam 2023-07-25 8:19 ` Vidya Sagar 2023-07-25 8:30 ` Manivannan Sadhasivam 2023-07-25 9:21 ` Vidya Sagar 2023-07-25 10:03 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).