Re: [PATCH] PCI: rockchip: Fix bus checks in rockchip_pcie_valid_device()

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Fri, Sep 04, 2020 at 03:09:04PM +0100, Lorenzo Pieralisi wrote:
> The root bus checks rework in:
> 
> commit d84c572de1a3 ("PCI: rockchip: Use pci_is_root_bus() to check if bus is root bus")
> 
> caused a regression whereby in rockchip_pcie_valid_device() if
> the bus parameter is the root bus and the dev value == 0 the
> function should return 1 (ie true) without checking if the
> bus->parent pointer is a root bus because that triggers a NULL
> pointer dereference.
> 
> Fix this by streamlining the root bus detection.
> 
> Fixes: d84c572de1a3 ("PCI: rockchip: Use pci_is_root_bus() to check if bus is root bus")
> Reported-by: Samuel Dionne-Riel <samuel@dionne-riel.com>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>  drivers/pci/controller/pcie-rockchip-host.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Hi Bjorn,

this is a fix for a patch we merged in the last merge window, can
we send it for one of the upcoming -rcX please ?

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index 0bb2fb3e8a0b..9705059523a6 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -71,16 +71,13 @@ static void rockchip_pcie_update_txcredit_mui(struct rockchip_pcie *rockchip)
>  static int rockchip_pcie_valid_device(struct rockchip_pcie *rockchip,
>  				      struct pci_bus *bus, int dev)
>  {
> -	/* access only one slot on each root port */
> -	if (pci_is_root_bus(bus) && dev > 0)
> -		return 0;
> -
>  	/*
> -	 * do not read more than one device on the bus directly attached
> +	 * Access only one slot on each root port.
> +	 * Do not read more than one device on the bus directly attached
>  	 * to RC's downstream side.
>  	 */
> -	if (pci_is_root_bus(bus->parent) && dev > 0)
> -		return 0;
> +	if (pci_is_root_bus(bus) || pci_is_root_bus(bus->parent))
> +		return dev == 0;
>  
>  	return 1;
>  }
> -- 
> 2.26.1
>