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

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

On Mon, Nov 28, 2022 at 12:41:11PM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Monday, November 28, 2022 8:59 PM
> > 
> > On Mon, Nov 28, 2022 at 02:52:56AM +0000, Yoshihiro Shimoda wrote:
> > > Hi Serge,
> > >
> > > > From: Serge Semin, Sent: Monday, November 28, 2022 8:56 AM
> > > >
> <snip>
> > > > 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?
> > >
> > > The dbi+0x8f8 and the +0x978 registers' values are 0x00000000.
> > > --------------- (sorry, replace tabs with spaces...)---------------
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -843,6 +843,10 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> > >  {
> > >         u32 val;
> > >
> > 
> > > +       dev_info(pci->dev, "%s: +0x8f8 = %08x, +0x978 = %08x\n", __func__,
> > > +               dw_pcie_readl_dma(pci, 0x8f8),
> > > +               dw_pcie_readl_dma(pci, 0x978));
> > > +
> > 
> > No, this should have been the dw_pcie_readl_dbi() calls instead of
> > dw_pcie_readl_!dma!(). What I try to understand from these values is
> > the real version of your controller (dbi+0x8f8) and whether the legacy
> > eDMA viewport registers range follows the documented convention of
> > having FFs in the dbi+0x978 register. My current assumption that
> > either your IP-core is newer than v5.30a or has some vendor-specific
> > modification. But let's see the value first.
> 

> Oops! I'm sorry for my bad code. After fixed the code, the values are:
> ---
> [    1.108943] pcie-rcar-gen4 e65d0000.pcie: dw_pcie_edma_find_chip: +0x8f8 = 3532302a, +0x978 = 00000000
> ---

@Yoshihiro. Got it. Thanks. Could you please confirm (double-check)
that what the content of the +0x978 CSR was really read by means of the
dw_pcie_readl_dbi() method?

@Mani, could you please have a look at the content of the +0x8f8 and
+0x978 CSRs in the CDM space of the available to you controllers?

I've reproduced the behavior what discovered by @Yoshihiro, but on
v5.40a IP-core only. It was expected for that controller version, but
v5.20a was supposed to have FFs in +0x978 for the unrolled iATU/eDMA
space. It's strange to see it didn't.

-Sergey

> 
> <snip>
> > > So, should I change the condition like below?
> > >
> > > ---
> > > -	if (val == 0xFFFFFFFF && pci->edma.reg_base) {
> > > +	if ((val == 0xFFFFFFFF || val == 0x00000000) && pci->edma.reg_base) {
> > > ...
> > > -	} else if (val != 0xFFFFFFFF) {
> > > -	} else if (!(val == 0xFFFFFFFF || val == 0x00000000)) {
> > > ---
> > 
> > Definitely no. Even though it's impossible to have the eDMA controller
> > configured with zero number of read and write channels we shouldn't
> > assume that getting a zero value from the DMA_CTRL_VIEWPORT_OFF offset
> > means having the unrolled eDMA CSRs mapping. Let's have a look at the
> > content of the dbi+0x8f8 and dbi+0x978 offsets first. Based on these
> > values we'll come up with what to do next.
> 
> I got it.
> 
> Best regards,
> Yoshihiro Shimoda
> 
> > -Serge(y)
> > 
> > >
> > > Best regards,
> > > Yoshihiro Shimoda
> > >
> > > > -Sergey
> > > >
> > > > > >  	} else {
> > > > > >  		return -ENODEV;
> > > > > >  	}
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > > >
> > > > > --
> > > > > மணிவண்ணன் சதாசிவம்