2020-04-16 00:19:42

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH 8/8] PCI: aardvark: Add FIXME for code which access PCIE_CORE_CMD_STATUS_REG

On Wednesday 15 April 2020 18:03:48 Pali Rohár wrote:
> Register PCIE_CORE_CMD_STATUS_REG is applicable only when aardvark
> controller is configured for Endpoint mode. Which is not the case of
> current kernel driver.
>
> Signed-off-by: Pali Rohár <[email protected]>
> ---
> 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 6a97a3838098..a1cebc734f2d 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -423,6 +423,12 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
> } while (1);
>
> + /*
> + * 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.20.1
>

Just to note, above code needs to be removed or fixed or properly
commented why is correct and what is doing.

I really do not know, so I'm adding at least FIXME comment to document
suspicious code which may cause unexpected problems.