linux-kernel - Re: [PATCH v8 4/4] USB: pci-quirks: Add Raspberry Pi 4 quirk

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]

Message-ID: <edcbc0a6-f901-d8ff-748f-73017397799d@gmail.com>
Date:   Tue, 2 Jun 2020 14:57:40 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Mathias Nyman <mathias.nyman@...el.com>,
        Rob Herring <robh@...nel.org>
Cc:     linux-usb@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org,
        bcm-kernel-feedback-list@...adcom.com, tim.gover@...pberrypi.org,
        linux-pci@...r.kernel.org, gregkh@...uxfoundation.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 4/4] USB: pci-quirks: Add Raspberry Pi 4 quirk



On 6/2/2020 3:05 AM, Nicolas Saenz Julienne wrote:
> On Tue, 2020-05-05 at 18:13 +0200, Nicolas Saenz Julienne wrote:
>> On the Raspberry Pi 4, after a PCI reset, VL805's firmware may either be
>> loaded directly from an EEPROM or, if not present, by the SoC's
>> VideoCore. Inform VideoCore that VL805 was just reset.
>>
>> Also, as this creates a dependency between USB_PCI and VideoCore's
>> firmware interface, and since USB_PCI can't be set as a module neither
>> this can. Reflect that on the firmware interface Kconfg.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@...e.de>
>> ---
> 
> It was pointed out to me on the u-boot mailing lists that all this could be
> implemented trough a reset controller. In other words have xhci get the reset
> controller trough device-tree, assert it, ultamately causing the firmware
> routine to be run.

That is actually a clever way to solve that problem.

> 
> As much as it pains me to go over stuff that's already 'fixed', it seems to me
> it's a better solution. On one hand we get over the device-tree dependency mess
> (see patch #3), and on the other we transform a pci-quirk into something less
> hacky.
> 
> That said, before getting my hands dirty, I was wondering if there is any
> obvious reasons why I shouldn't do this, note that:
> 
> - We're talking here of a PCIe XCHI device, maybe there's an issue integrating
>   it with DT, given the fact that, as of today, it's not really represented
>   there.

You can always provide a PCIe device representation within the Device
Tree, this is not very common, but it is sometimes useful for e.g.:
assigning MAC addresses, see this example for instance:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi#n647

(does not assign a MAC address, but it could). This should allow your
XHCI pci_device::of_node pointer to point to node declared in the Device
Tree. There you could add a 'resets' property accordingly.

> 
> - There is no reset controller support in xhci-pci, maybe there are good
>   reasons why. For instance, it's not something that's reflected in any way in
>   the spec.

It seems to me this is not usually necessary for PC systems, so it was
not really needed until now. Maybe you can write a small wrapper around
xhci-pci.c, similar to what xhci-plat.c does which is responsible for
grabbing and releasing the reset.
-- 
Florian