| Message ID | 20200612164632.25648-5-nsaenzjulienne@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | usb: xhci: Load Raspberry Pi 4 VL805's firmware | expand |
Hi Nicolas, On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne <nsaenzjulienne at suse.de> wrote: > > There is no distinction in DT between the PCI controller device and the > root bridge, whereas such distinction exists from dm's perspective. Make > sure the root bridge ofnode is assigned to the controller's platform > device node. > > This permits setups like this to work correctly: > > pcie { > compatible = "..."; > ... > dev { > reg = <0 0 0 0 0>; > ... > }; > }; > > Without this the dev node is assigned to the root bridge and the > actual device search starts one level lower than expected. > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de> > --- > drivers/pci/pci-uclass.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) Can you update the tests to handle this case please? Regards, Simon
On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote: > Hi Nicolas, > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne > <nsaenzjulienne at suse.de> wrote: > > There is no distinction in DT between the PCI controller device and the > > root bridge, whereas such distinction exists from dm's perspective. Make > > sure the root bridge ofnode is assigned to the controller's platform > > device node. > > > > This permits setups like this to work correctly: > > > > pcie { > > compatible = "..."; > > ... > > dev { > > reg = <0 0 0 0 0>; > > ... > > }; > > }; > > > > Without this the dev node is assigned to the root bridge and the > > actual device search starts one level lower than expected. > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de> > > --- > > drivers/pci/pci-uclass.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > Can you update the tests to handle this case please? I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give me some pointers on where/how to test this? Regards, Nicolas -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: This is a digitally signed message part URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200616/e0b6022e/attachment.sig>
Hi Nicolas, On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne <nsaenzjulienne at suse.de> wrote: > > On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote: > > Hi Nicolas, > > > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne > > <nsaenzjulienne at suse.de> wrote: > > > There is no distinction in DT between the PCI controller device and the > > > root bridge, whereas such distinction exists from dm's perspective. Make > > > sure the root bridge ofnode is assigned to the controller's platform > > > device node. > > > > > > This permits setups like this to work correctly: > > > > > > pcie { > > > compatible = "..."; > > > ... > > > dev { > > > reg = <0 0 0 0 0>; > > > ... > > > }; > > > }; > > > > > > Without this the dev node is assigned to the root bridge and the > > > actual device search starts one level lower than expected. > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de> > > > --- > > > drivers/pci/pci-uclass.c | 15 ++++++++++++++- > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > Can you update the tests to handle this case please? > > I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give me > some pointers on where/how to test this? > Yes it is at test/dm/pci.c and the device tree is test.dts 'make qcheck' to run all tests. To run one test, build for sandbox and then something like u-boot -T -c "ut dm pci_swapcase" for example. You can perhaps use an existing PCI controller in test.dts but feel free to add one more if you need it for your test. Make sure that you don't break other tests. Regards, Simon
On Tue, 2020-06-16 at 17:31 -0600, Simon Glass wrote: > Hi Nicolas, > > On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne > <nsaenzjulienne at suse.de> wrote: > > On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote: > > > Hi Nicolas, > > > > > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne > > > <nsaenzjulienne at suse.de> wrote: > > > > There is no distinction in DT between the PCI controller device and the > > > > root bridge, whereas such distinction exists from dm's perspective. Make > > > > sure the root bridge ofnode is assigned to the controller's platform > > > > device node. > > > > > > > > This permits setups like this to work correctly: > > > > > > > > pcie { > > > > compatible = "..."; > > > > ... > > > > dev { > > > > reg = <0 0 0 0 0>; > > > > ... > > > > }; > > > > }; > > > > > > > > Without this the dev node is assigned to the root bridge and the > > > > actual device search starts one level lower than expected. > > > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de> > > > > --- > > > > drivers/pci/pci-uclass.c | 15 ++++++++++++++- > > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > > > Can you update the tests to handle this case please? > > > > I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give > > me > > some pointers on where/how to test this? > > > > Yes it is at test/dm/pci.c and the device tree is test.dts > > 'make qcheck' to run all tests. To run one test, build for sandbox and > then something like > > u-boot -T -c "ut dm pci_swapcase" > > for example. > > You can perhaps use an existing PCI controller in test.dts but feel > free to add one more if you need it for your test. Make sure that you > don't break other tests. Thanks for the info. Actually adding the tests made me doubleguess myself, and now I'm pretty sure that what I shoudl've done in DT is the following: &pcie0 { pci at 0 { #address-cells = <3>; #size-cells = <2>; ranges; reg = <0 0 0 0 0>; usb at 1,0 { reg = <0x10000 0 0 0 0>; resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>; }; }; }; (with "lspci -tv": [0000:00]---00.0-[01]----00.0 VIA Technologies, Inc. VL805 USB 3.0 Host Controller) With this the patch above isn't needed, which is great. I'll send this to upstream Linux just to get a confirmation this is correct, although if you have any comments it'll be appreciated. Regards, Nicolas -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: This is a digitally signed message part URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200617/bd5cc8c3/attachment.sig>
Hi Nicolas, On Wed, 17 Jun 2020 at 13:15, Nicolas Saenz Julienne <nsaenzjulienne at suse.de> wrote: > > On Tue, 2020-06-16 at 17:31 -0600, Simon Glass wrote: > > Hi Nicolas, > > > > On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne > > <nsaenzjulienne at suse.de> wrote: > > > On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote: > > > > Hi Nicolas, > > > > > > > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne > > > > <nsaenzjulienne at suse.de> wrote: > > > > > There is no distinction in DT between the PCI controller device and the > > > > > root bridge, whereas such distinction exists from dm's perspective. Make > > > > > sure the root bridge ofnode is assigned to the controller's platform > > > > > device node. > > > > > > > > > > This permits setups like this to work correctly: > > > > > > > > > > pcie { > > > > > compatible = "..."; > > > > > ... > > > > > dev { > > > > > reg = <0 0 0 0 0>; > > > > > ... > > > > > }; > > > > > }; > > > > > > > > > > Without this the dev node is assigned to the root bridge and the > > > > > actual device search starts one level lower than expected. > > > > > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de> > > > > > --- > > > > > drivers/pci/pci-uclass.c | 15 ++++++++++++++- > > > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > > > > > Can you update the tests to handle this case please? > > > > > > I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give > > > me > > > some pointers on where/how to test this? > > > > > > > Yes it is at test/dm/pci.c and the device tree is test.dts > > > > 'make qcheck' to run all tests. To run one test, build for sandbox and > > then something like > > > > u-boot -T -c "ut dm pci_swapcase" > > > > for example. > > > > You can perhaps use an existing PCI controller in test.dts but feel > > free to add one more if you need it for your test. Make sure that you > > don't break other tests. > > Thanks for the info. > > Actually adding the tests made me doubleguess myself, and now I'm pretty sure > that what I shoudl've done in DT is the following: > > &pcie0 { > pci at 0 { > #address-cells = <3>; > #size-cells = <2>; > ranges; > > reg = <0 0 0 0 0>; > > usb at 1,0 { > reg = <0x10000 0 0 0 0>; > resets = <&reset RASPBERRYPI_FIRMWARE_RESET_ID_USB>; > }; > }; > }; > > > (with "lspci -tv": [0000:00]---00.0-[01]----00.0 VIA Technologies, Inc. VL805 USB 3.0 Host Controller) > > With this the patch above isn't needed, which is great. > > I'll send this to upstream Linux just to get a confirmation this is correct, > although if you have any comments it'll be appreciated. Yes it looks OK to me. U-Boot allows PCI and USB devices to be represented in the device tree. Regards, Simon
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 9ab3539a49..ea27e78465 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -762,7 +762,20 @@ static int pci_find_and_bind_driver(struct udevice *parent, str = strdup(name); if (!str) return -ENOMEM; - drv = bridge ? "pci_bridge_drv" : "pci_generic_drv"; + + if (bridge) { + drv = "pci_bridge_drv"; + + /* + * If we're dealing with the root bridge pass the parent device + * node, as there isn't a distinction in device tree between + * that and the actual controller platform device. + */ + if (!PCI_MASK_BUS(bdf)) + node = parent->node; + } else { + drv = "pci_generic_drv"; + } ret = device_bind_driver_to_node(parent, drv, str, node, devp); if (ret) {
There is no distinction in DT between the PCI controller device and the root bridge, whereas such distinction exists from dm's perspective. Make sure the root bridge ofnode is assigned to the controller's platform device node. This permits setups like this to work correctly: pcie { compatible = "..."; ... dev { reg = <0 0 0 0 0>; ... }; }; Without this the dev node is assigned to the root bridge and the actual device search starts one level lower than expected. Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne at suse.de> --- drivers/pci/pci-uclass.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)