Re: [PATCH v2 5/9] PCI: aardvark: add FIXME comment for PCIE_CORE_CMD_STATUS_REG access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 23 April 2020 13:44:13 Bjorn Helgaas wrote:
> [+cc Rob]
> 
> On Tue, Apr 21, 2020 at 01:16:57PM +0200, Marek Behún wrote:
> > From: Pali Rohár <pali@xxxxxxxxxx>
> > 
> > Register PCIE_CORE_CMD_STATUS_REG is applicable only when the controller
> > is configured for Endpoint mode, which is not the case for the current
> > version of this driver.
> > 
> > Add a FIXME comment, since this needs to be explained, removed or fixed.
> 
> If it's not applicable, why not just remove it?

Problem is hat if I remove this part of code, ath10k cards stops
working. So for some unknown reasons setting this "not applicable"
register is required.

I have in-progress V3 version of this patch series with updated comment
and commit message of this patch (include this observation).

> > Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> > ---
> >  drivers/pci/controller/pci-aardvark.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > index e2d18094d8ca..e893d7d8859f 100644
> > --- a/drivers/pci/controller/pci-aardvark.c
> > +++ b/drivers/pci/controller/pci-aardvark.c
> > @@ -429,6 +429,12 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> >  
> >  	advk_pcie_train_link(pcie);
> >  
> > +	/*
> > +	 * FIXME: Following code which access PCIE_CORE_CMD_STATUS_REG register
> > +	 *        is suspicious. This register is applicable only when the PCI
> > +	 *        controller is configured for Endpoint mode. And not when it
> > +	 *        is configured for Root Complex.
> > +	 */
> >  	reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
> >  	reg |= PCIE_CORE_CMD_MEM_ACCESS_EN |
> >  		PCIE_CORE_CMD_IO_ACCESS_EN |
> > -- 
> > 2.24.1
> > 



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux