Re: [PATCH v7 5/9] PCI: dwc: Avoid reading a register to detect whether eDMA exists

[Serge Semin <fancer.lancer@gmail.com>]

On Tue, Nov 22, 2022 at 07:25:50PM +0530, Manivannan Sadhasivam wrote:
> + Serge (who authored EDMA support)

Thanks @Mani. It's strange to see a fix for a patch which hasn't been even
merged in yet and miss the patch author in the Cc list.)

@Yoshihiro, on the next patchset revisions please don't forget to add
my email address to the copy list.

> 
> Thanks,
> Mani
> 
> On Mon, Nov 21, 2022 at 09:43:56PM +0900, Yoshihiro Shimoda wrote:
> > Since reading value of PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL was
> > 0x00000000 on one of SoCs (R-Car S4-8), it cannot find the eDMA.
> > So, directly read the eDMA register if edma.reg_base is not zero.
> > 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 637d01807c67..2cc8584da6f4 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -836,8 +836,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  {
> >  	u32 val;
> >  

> > -	val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > +	if (pci->edma.reg_base) {
> >  		pci->edma.mf = EDMA_MF_EDMA_UNROLL;
> >  
> >  		val = dw_pcie_readl_dma(pci, PCIE_DMA_CTRL);
> > @@ -845,6 +844,7 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  		pci->edma.mf = EDMA_MF_EDMA_LEGACY;
> >  
> >  		pci->edma.reg_base = pci->dbi_base + PCIE_DMA_VIEWPORT_BASE;
> > +		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);

Look what you suggest here:
< u32 val;
< ...
< if (pci->edma.reg_base) {
< 	...
< } else if (val != 0xFFFFFFFF) {
< 	...
< } else {
< ...

It would be strange if your compiler didn't warn about 'val' being used
uninitialized here, which in its turn would introduce a regression for
the platforms with the indirectly accessible eDMA registers.

Anyway you can't just drop something what didn't work for you
hardware. The method you suggest to fix here works fine for multiple
DW PCIe IP-cores. Judging by the HW manuals it should work at least up
to v5.30a. Are you sure that your controller is of v5.20a? I see you
overwrite the IP-core version for the PCIe host driver only. Why is
that necessary? Does the version auto-detection procedure work
incorrectly for you? What does the dbi+0x8f8 CSR contain in the host
and EP registers space? Similarly could you also provide a content of
the +0x978 register?

-Sergey

> >  	} else {
> >  		return -ENODEV;
> >  	}
> > -- 
> > 2.25.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்