linux-kernel - Re: [PATCH v2 2/2] usb: xhci: Load Raspberry Pi 4 VL805's firmware

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] [day] [month] [year] [list]

Date:   Tue, 05 May 2020 17:04:04 +0200
From:   Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
To:     Matthias Brugger <mbrugger@...e.com>, u-boot@...ts.denx.de,
        bmeng.cn@...il.com, marex@...x.de, linux-kernel@...r.kernel.org
Cc:     sjg@...omium.org, m.szyprowski@...sung.com, s.nawrocki@...sung.com
Subject: Re: [PATCH v2 2/2] usb: xhci: Load Raspberry Pi 4 VL805's firmware

On Tue, 2020-05-05 at 16:59 +0200, Matthias Brugger wrote:
[...]
> > > > > > +#ifdef CONFIG_BCM2711
> > > > > 
> > > > > This won't work with rpi_arm64_defconfig.
> > > > > Can't we just evaluate at runtime if we need to do anything in
> > > > > xhci_pci_fixup.
> > > > 
> > > > I can't see why, who is going to call xhci_pci_probe() in RPi3?
> > > > 
> > > 
> > > If you do make rpi_arm64_defconfig and inspect the .config, you will see
> > > that
> > > CONFIG_BCM2711 is not defined, so this code won't be called on RPi4.
> > 
> > Oh! Understood.
> > 
> > Well, given that only xhci_pci_probe() is called if we're running on RPi4, I
> > think I can disregard those defines altogether. I'll double-check that.
> > 
> 
> Yes but from my understanding we only need to call the function on newer
> revisions of RPi4. Does it have any side effect on older revisions, e.g. we
> get
> error messages (see below)?

The firmware quirk supports older rpi4s and simply does nothing. Note that the
downstream Linux implementation runs this on all rpi4s.

> [...]>>>> I wonder if the newer RPi4 have also a newer revision ID (see
> > > > > get_board_rev).
> > > > > If
> > > > > so we could add another bool to struct rpi_model which will indicate
> > > > > us if
> > > > > we
> > > > > need to notify VideoCore about vl805's firmware.
> > > > > 
> > > > > > +void xhci_pci_fixup(struct udevice *dev)
> > > > > > +{
> > > > > > +	int ret;
> > > > > > +
> > > > > > +	ret = bcm2711_notify_vl805_reset();
> > > > > > +	if (ret)
> > > > > > +		printf("RPI: Failed to notify VideoCore about vl805's
> > > > > > firmware\n");
> 
> We already have
> printf("bcm2711: Faild to load vl805's firmware, %d\n", ret); in
> bcm2711_notify_vl805_reset(). Do we really need two error messages?

Agree, it's a little redundant. I'll get rid of it.

Regards,
Nicolas



Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)