linux-kernel - Re: [PATCH v6 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware when present

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Apr 2020 15:41:49 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
Cc:     linux-kernel@...r.kernel.org,
        Florian Fainelli <f.fainelli@...il.com>,
        bcm-kernel-feedback-list@...adcom.com,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Andrew Murray <amurray@...goodpenguin.co.uk>,
        linux-usb@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, gregkh@...uxfoundation.org,
        tim.gover@...pberrypi.org, linux-pci@...r.kernel.org,
        wahrenst@....net, sergei.shtylyov@...entembedded.com
Subject: Re: [PATCH v6 3/4] PCI: brcmstb: Wait for Raspberry Pi's firmware
 when present

On Tue, Mar 24, 2020 at 07:28:11PM +0100, Nicolas Saenz Julienne wrote:
> xHCI's PCI fixup, run at the end of pcie-brcmstb's probe, depends on

Is there a function name for this fixup that you can mention?

> RPi4's VideoCore firmware interface to be up and running. It's possible
> for both initializations to race, so make sure it's available prior to
> starting.

I guess "both initializations" means brcm_pcie_probe() and something
else?  It'd be nice to include that function name here, too.

> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
> Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
> ---
>  drivers/pci/controller/pcie-brcmstb.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 3a10e678c7f4..a3d3070a5832 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -28,6 +28,8 @@
>  #include <linux/string.h>
>  #include <linux/types.h>
>  
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
>  #include "../pci.h"
>  
>  /* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
> @@ -917,11 +919,24 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node, *msi_np;
>  	struct pci_host_bridge *bridge;
> +	struct device_node *fw_np;
>  	struct brcm_pcie *pcie;
>  	struct pci_bus *child;
>  	struct resource *res;
>  	int ret;
>  
> +	/*
> +	 * We have to wait for the Raspberry Pi's firmware interface to be up
> +	 * as some PCI fixups depend on it.

It'd be nice to know the nature of this dependency between the
firmware interface and the fixups.  This may be useful for future
maintenance.  E.g., if PCI config access doesn't work until the
firmware interface is up, that would affect almost everything.  But
you say "some PCI fixups", so I suppose the actual dependency is
probably something else.

> +	 */
> +	fw_np = of_find_compatible_node(NULL, NULL,
> +					"raspberrypi,bcm2835-firmware");
> +	if (fw_np && !rpi_firmware_get(fw_np)) {
> +		of_node_put(fw_np);
> +		return -EPROBE_DEFER;
> +	}
> +	of_node_put(fw_np);
> +
>  	bridge = devm_pci_alloc_host_bridge(&pdev->dev, sizeof(*pcie));
>  	if (!bridge)
>  		return -ENOMEM;
> -- 
> 2.25.1
>