On Wed, Aug 24, 2022 at 01:17:54PM -0500, Bjorn Helgaas wrote: > On Wed, Aug 24, 2022 at 09:13:19PM +0300, Serge Semin wrote: > > On Wed, Aug 24, 2022 at 11:51:18AM -0500, Bjorn Helgaas wrote: > > > On Mon, Aug 22, 2022 at 09:53:32PM +0300, Serge Semin wrote: > > > > > + val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL); > > > > + if (val == 0xFFFFFFFF && pci->edma.reg_base) { > > > > + pci->edma.mf = EDMA_MF_EDMA_UNROLL; > > > > + > > > > + val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL); > > > > + } else if (val != 0xFFFFFFFF) { > > > > > > > > Consider PCI_POSSIBLE_ERROR() as an annotation about the meaning of > > > 0xFFFFFFFF and something to grep for. > > > > In this case FFs don't mean an error but a special value, which > > indicates that the eDMA is mapped via the unrolled CSRs space. The > > similar approach has been implemented for the iATU legacy/unroll setup > > auto-detection. So I don't see much reasons to have it grepped, so as > > to have a macro-based parametrization since the special value will > > unluckily change while having the explicit literal utilized gives a > > better understanding of the way the algorithm works. > > If 0xFFFFFFFF is the result of a successful PCIe Memory Read, Right. It is. > and not > something synthesized by the host bridge when it handles an > Unsupported Request completion, No it isn't. To be clear 0xFFs don't indicate some PCIe bus/controller malfunction, but they are a result of reading the DMA_CTRL_VIEWPORT_OFF register which doesn't exist. The manual explicitly says: "Note - When register does not exist, value is fixed to 32'hFFFF_FFFF". The register doesn't exist if either eDMA is unavailable or the eDMA CSRs are mapped via the unrolled state. That logic is used to auto-detect the eDMA availability and the way of it's CSRs mapping. > I'm fine with keeping it as is. Ok. Thanks. -Sergey