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

From: Florian Fainelli <f.fainelli@gmail.com>
To: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Rob Herring <robh@kernel.org>
Cc: tim.gover@raspberrypi.org, linux-pci@vger.kernel.org,
	linux-usb@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	linux-rpi-kernel@lists.infradead.org, gregkh@linuxfoundation.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v8 4/4] USB: pci-quirks: Add Raspberry Pi 4 quirk
Date: Tue, 2 Jun 2020 14:57:40 -0700	[thread overview]
Message-ID: <edcbc0a6-f901-d8ff-748f-73017397799d@gmail.com> (raw)
In-Reply-To: <7cbe4da8ba4a1524824473f8c58720f412a00fc2.camel@suse.de>

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@suse.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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel