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

From: Florian Fainelli
Date: Tue Jun 02 2020 - 17:57:49 EST

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@xxxxxxx>
>> ---
> 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:

(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.