Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

2020-06-25 Thread Simon Glass
Hi Nicolas,

On Wed, 17 Jun 2020 at 13:15, Nicolas Saenz Julienne
 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
> >  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
> > > >  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 
> > > > > ---
> > > > >  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:
>
>  {
>pci@0 {
>#address-cells = <3>;
>#size-cells = <2>;
>ranges;
>
>reg = <0 0 0 0 0>;
>
>usb@1,0 {
>reg = <0x1 0 0 0 0>;
>resets = < 
> RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
>};
>};
> };
>
>
> (with "lspci -tv": [: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


Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

2020-06-17 Thread Nicolas Saenz Julienne
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
>  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
> > >  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 
> > > > ---
> > > >  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:

 {
   pci@0 {
   #address-cells = <3>;
   #size-cells = <2>;
   ranges;

   reg = <0 0 0 0 0>;

   usb@1,0 {
   reg = <0x1 0 0 0 0>;
   resets = < 
RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
   };
   };
};


(with "lspci -tv": [: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



signature.asc
Description: This is a digitally signed message part


Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

2020-06-16 Thread Simon Glass
Hi Nicolas,

On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
 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
> >  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 
> > > ---
> > >  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


Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

2020-06-16 Thread Nicolas Saenz Julienne
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
>  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 
> > ---
> >  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



signature.asc
Description: This is a digitally signed message part


Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

2020-06-16 Thread Simon Glass
Hi Nicolas,

On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
 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 
> ---
>  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


[PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

2020-06-12 Thread Nicolas Saenz Julienne
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 
---
 drivers/pci/pci-uclass.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

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