[v4,4/5] dm: pci: Assign controller device node to root bridge
diff mbox series

Message ID 20200612164632.25648-5-nsaenzjulienne@suse.de
State New, archived
Headers show
Series
  • usb: xhci: Load Raspberry Pi 4 VL805's firmware
Related show

Commit Message

Nicolas Saenz Julienne June 12, 2020, 4:46 p.m. UTC
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@suse.de>
---
 drivers/pci/pci-uclass.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Simon Glass June 16, 2020, 1:43 p.m. UTC | #1
Hi Nicolas,

On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
<nsaenzjulienne@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@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
Nicolas Saenz Julienne June 16, 2020, 2:09 p.m. UTC | #2
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@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@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
Simon Glass June 16, 2020, 11:31 p.m. UTC | #3
Hi Nicolas,

On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
<nsaenzjulienne@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@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@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
Nicolas Saenz Julienne June 17, 2020, 7:15 p.m. UTC | #4
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@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@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@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@0 {
		       #address-cells = <3>;
		       #size-cells = <2>;
		       ranges;

		       reg = <0 0 0 0 0>;

		       usb@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
Simon Glass June 26, 2020, 1:12 a.m. UTC | #5
Hi Nicolas,

On Wed, 17 Jun 2020 at 13:15, Nicolas Saenz Julienne
<nsaenzjulienne@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@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@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@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@0 {
>                        #address-cells = <3>;
>                        #size-cells = <2>;
>                        ranges;
>
>                        reg = <0 0 0 0 0>;
>
>                        usb@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

Patch
diff mbox series

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) {