[U-Boot] [PATCH V3 1/2] pci: Support parsing PCI controller DT subnodes
The PCI controller can have DT subnodes describing extra properties of particular PCI devices, ie. a PHY attached to an EHCI controller on a PCI bus. This patch parses those DT subnodes and assigns a node to the PCI device instance, so that the driver can extract details from that node and ie. configure the PHY using the PHY subsystem.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- V3: Reword the comment V2: Move the whole machinery to pci_bind_bus_devices(), right after the driver instance platform data are updated. This reduces the number of times the DT is traversed and works for both DT nodes with and without compat string. --- drivers/pci/pci-uclass.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index e9671d9b76..ab8818280d 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -733,6 +733,7 @@ int pci_bind_bus_devices(struct udevice *bus) ulong vendor, device; ulong header_type; pci_dev_t bdf, end; + ofnode node; bool found_multi; int ret;
@@ -803,6 +804,20 @@ int pci_bind_bus_devices(struct udevice *bus) pplat->vendor = vendor; pplat->device = device; pplat->class = class; + + /* Associate optional OF node */ + dev_for_each_subnode(node, bus) { + phys_addr_t df, size; + df = ofnode_get_addr_size(node, "reg", &size); + if (df == FDT_ADDR_T_NONE) + continue; + + if (PCI_FUNC(df) == PCI_FUNC(bdf) && + PCI_DEV(df) == PCI_DEV(bdf)) { + dev->node = node; + break; + } + } }
return 0;
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- V3: No change V2: New patch --- doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable +bus, albeit there are justified exceptions. If the compatible string is +present, matching on it takes precedence over PCI IDs and PCI classes. + +Note we must describe PCI devices with the same bus hierarchy as the hardware, otherwise driver model cannot detect the correct parent/children relationship during PCI bus enumeration thus PCI devices won't be bound to their drivers accordingly. A working example like below:
Hi Marex,
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: No change V2: New patch
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
+bus, albeit there are justified exceptions. If the compatible string is +present, matching on it takes precedence over PCI IDs and PCI classes.
+Note we must describe PCI devices with the same bus hierarchy as the hardware, otherwise driver model cannot detect the correct parent/children relationship during PCI bus enumeration thus PCI devices won't be bound to their drivers accordingly. A working example like below: -- 2.18.0
Regards, Simon
On 09/14/2018 06:41 AM, Simon Glass wrote:
Hi Marex,
It's Marek btw ...
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: No change V2: New patch
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
Can you rephrase it better then ? I can paste it into the docs.
+bus, albeit there are justified exceptions. If the compatible string is +present, matching on it takes precedence over PCI IDs and PCI classes.
+Note we must describe PCI devices with the same bus hierarchy as the hardware, otherwise driver model cannot detect the correct parent/children relationship during PCI bus enumeration thus PCI devices won't be bound to their drivers accordingly. A working example like below: -- 2.18.0
Regards, Simon
Hi Marek,
On 18 September 2018 at 05:47, Marek Vasut marek.vasut@gmail.com wrote:
On 09/14/2018 06:41 AM, Simon Glass wrote:
Hi Marex,
It's Marek btw ...
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: No change V2: New patch
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
Well it's more than that, as I mentioned before. Finding a compatible string in the source code is easier, and if we are matching with a DT node anyway, makes more sense IMO. Anyway since DTs likely come from the newly pleasant Linux we'll just end up with what they have there. This mostly applies for things like x86 which don't use DT in Linux.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
Can you rephrase it better then ? I can paste it into the docs.
How about:
The compatible string is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/ PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attache that node to the device so that it is accessible to the driver.
+bus, albeit there are justified exceptions. If the compatible string is +present, matching on it takes precedence over PCI IDs and PCI classes.
+Note we must describe PCI devices with the same bus hierarchy as the hardware, otherwise driver model cannot detect the correct parent/children relationship during PCI bus enumeration thus PCI devices won't be bound to their drivers accordingly. A working example like below: -- 2.18.0
Regards, Simon
Regards, Simon
On 09/18/2018 04:02 PM, Simon Glass wrote:
Hi Marek,
Hi,
On 18 September 2018 at 05:47, Marek Vasut marek.vasut@gmail.com wrote:
On 09/14/2018 06:41 AM, Simon Glass wrote:
Hi Marex,
It's Marek btw ...
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: No change V2: New patch
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
Well it's more than that, as I mentioned before. Finding a compatible string in the source code is easier, and if we are matching with a DT node anyway, makes more sense IMO.
It's about as easy as finding PCI ID.
And PCI is a discoverable bus, so using a compatible string is some obscure edge-case.
Anyway since DTs likely come from the newly pleasant Linux we'll just end up with what they have there. This mostly applies for things like x86 which don't use DT in Linux.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
Can you rephrase it better then ? I can paste it into the docs.
How about:
The compatible string is optional since U_BOOT_PCI_DEVICE() can be used to specific
specify ?
the driver based on conditions like the PCI vendor/ PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver;
This implies the compatible string is preferred, it is not.
assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attache that node to the device so that it is accessible to the driver.
+bus, albeit there are justified exceptions. If the compatible string is +present, matching on it takes precedence over PCI IDs and PCI classes.
+Note we must describe PCI devices with the same bus hierarchy as the hardware, otherwise driver model cannot detect the correct parent/children relationship during PCI bus enumeration thus PCI devices won't be bound to their drivers accordingly. A working example like below: -- 2.18.0
Regards, Simon
Regards, Simon
Hi Marek,
On Wed, Sep 19, 2018 at 9:29 PM Marek Vasut marek.vasut@gmail.com wrote:
On 09/18/2018 04:02 PM, Simon Glass wrote:
Hi Marek,
Hi,
On 18 September 2018 at 05:47, Marek Vasut marek.vasut@gmail.com wrote:
On 09/14/2018 06:41 AM, Simon Glass wrote:
Hi Marex,
It's Marek btw ...
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: No change V2: New patch
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
Well it's more than that, as I mentioned before. Finding a compatible string in the source code is easier, and if we are matching with a DT node anyway, makes more sense IMO.
It's about as easy as finding PCI ID.
And PCI is a discoverable bus, so using a compatible string is some obscure edge-case.
Anyway since DTs likely come from the newly pleasant Linux we'll just end up with what they have there. This mostly applies for things like x86 which don't use DT in Linux.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
Can you rephrase it better then ? I can paste it into the docs.
How about:
The compatible string is optional since U_BOOT_PCI_DEVICE() can be used to specific
specify ?
the driver based on conditions like the PCI vendor/ PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver;
This implies the compatible string is preferred, it is not.
I think Simon was describing the *current* U-Boot implementation, that "compatible" string is looked up first, then U_BOOT_PCI_DEVICE().
assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attache that node to the device so that it is accessible to the driver.
Regards, Bin
On 09/20/2018 03:55 AM, Bin Meng wrote:
Hi Marek,
On Wed, Sep 19, 2018 at 9:29 PM Marek Vasut marek.vasut@gmail.com wrote:
On 09/18/2018 04:02 PM, Simon Glass wrote:
Hi Marek,
Hi,
On 18 September 2018 at 05:47, Marek Vasut marek.vasut@gmail.com wrote:
On 09/14/2018 06:41 AM, Simon Glass wrote:
Hi Marex,
It's Marek btw ...
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
Reword the documentation to make it clear the compatible string is now optional, yet still matching on it takes precedence over PCI IDs and PCI classes.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: No change V2: New patch
doc/driver-model/pci-info.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt index e1701d1fbc..14364c5c75 100644 --- a/doc/driver-model/pci-info.txt +++ b/doc/driver-model/pci-info.txt @@ -34,11 +34,15 @@ under that bus. Note that this is all done on a lazy basis, as needed, so until something is touched on PCI (eg: a call to pci_find_devices()) it will not be probed.
-PCI devices can appear in the flattened device tree. If they do this serves to -specify the driver to use for the device. In this case they will be bound at -first. Each PCI device node must have a compatible string list as well as a -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document -v2.1. Note we must describe PCI devices with the same bus hierarchy as the +PCI devices can appear in the flattened device tree. If they do, their node +often contains extra information which cannot be derived from the PCI IDs or +PCI class of the device. Each PCI device node must have a <reg> property, as +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
Well it's more than that, as I mentioned before. Finding a compatible string in the source code is easier, and if we are matching with a DT node anyway, makes more sense IMO.
It's about as easy as finding PCI ID.
And PCI is a discoverable bus, so using a compatible string is some obscure edge-case.
Anyway since DTs likely come from the newly pleasant Linux we'll just end up with what they have there. This mostly applies for things like x86 which don't use DT in Linux.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
Can you rephrase it better then ? I can paste it into the docs.
How about:
The compatible string is optional since U_BOOT_PCI_DEVICE() can be used to specific
specify ?
the driver based on conditions like the PCI vendor/ PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver;
This implies the compatible string is preferred, it is not.
I think Simon was describing the *current* U-Boot implementation, that "compatible" string is looked up first, then U_BOOT_PCI_DEVICE().
This patch updates the documentation to match reality though.
Hi Marek,
On Thu, Sep 20, 2018 at 6:34 PM Marek Vasut marek.vasut@gmail.com wrote:
On 09/20/2018 03:55 AM, Bin Meng wrote:
Hi Marek,
On Wed, Sep 19, 2018 at 9:29 PM Marek Vasut marek.vasut@gmail.com wrote:
On 09/18/2018 04:02 PM, Simon Glass wrote:
Hi Marek,
Hi,
On 18 September 2018 at 05:47, Marek Vasut marek.vasut@gmail.com wrote:
On 09/14/2018 06:41 AM, Simon Glass wrote:
Hi Marex,
It's Marek btw ...
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote: > Reword the documentation to make it clear the compatible string is now > optional, yet still matching on it takes precedence over PCI IDs and > PCI classes. > > Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com > Cc: Simon Glass sjg@chromium.org > Cc: Tom Rini trini@konsulko.com > --- > V3: No change > V2: New patch > --- > doc/driver-model/pci-info.txt | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt > index e1701d1fbc..14364c5c75 100644 > --- a/doc/driver-model/pci-info.txt > +++ b/doc/driver-model/pci-info.txt > @@ -34,11 +34,15 @@ under that bus. > Note that this is all done on a lazy basis, as needed, so until something is > touched on PCI (eg: a call to pci_find_devices()) it will not be probed. > > -PCI devices can appear in the flattened device tree. If they do this serves to > -specify the driver to use for the device. In this case they will be bound at > -first. Each PCI device node must have a compatible string list as well as a > -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document > -v2.1. Note we must describe PCI devices with the same bus hierarchy as the > +PCI devices can appear in the flattened device tree. If they do, their node > +often contains extra information which cannot be derived from the PCI IDs or > +PCI class of the device. Each PCI device node must have a <reg> property, as > +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible > +string list is optional and generally not needed, since PCI is discoverable
I really don't like 'generally not needed'. How about 'generally not essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
Well it's more than that, as I mentioned before. Finding a compatible string in the source code is easier, and if we are matching with a DT node anyway, makes more sense IMO.
It's about as easy as finding PCI ID.
And PCI is a discoverable bus, so using a compatible string is some obscure edge-case.
Anyway since DTs likely come from the newly pleasant Linux we'll just end up with what they have there. This mostly applies for things like x86 which don't use DT in Linux.
I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be used to specific the driver based on conditions like the PCI vendor/, PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver; assuming it finds one it will then search for the device-tree node whose reg property matches the bus/device/function of the device, and attached that node to the device so that it is accessible to the driver.
Can you rephrase it better then ? I can paste it into the docs.
How about:
The compatible string is optional since U_BOOT_PCI_DEVICE() can be used to specific
specify ?
the driver based on conditions like the PCI vendor/ PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver;
This implies the compatible string is preferred, it is not.
I think Simon was describing the *current* U-Boot implementation, that "compatible" string is looked up first, then U_BOOT_PCI_DEVICE().
This patch updates the documentation to match reality though.
Am I looking at a different pci uclass driver implementation from yours?
Currently in pci_bind_bus_devices(), we have:
/* Find this device in the device tree */ ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev);
This is "compatible" based driver binding, and it goes first.
And we have:
/* If nothing in the device tree, bind a device */ if (ret == -ENODEV) { ... ret = pci_find_and_bind_driver(bus, &find_id, bdf, &dev); ... }
this is the dynamic driver binding based vid/pid, etc.
Your patch does not change the fact that "compatible" comes first than dynamic binding.
Regards, Bin
On 09/21/2018 11:08 AM, Bin Meng wrote:
Hi Marek,
On Thu, Sep 20, 2018 at 6:34 PM Marek Vasut marek.vasut@gmail.com wrote:
On 09/20/2018 03:55 AM, Bin Meng wrote:
Hi Marek,
On Wed, Sep 19, 2018 at 9:29 PM Marek Vasut marek.vasut@gmail.com wrote:
On 09/18/2018 04:02 PM, Simon Glass wrote:
Hi Marek,
Hi,
On 18 September 2018 at 05:47, Marek Vasut marek.vasut@gmail.com wrote:
On 09/14/2018 06:41 AM, Simon Glass wrote: > Hi Marex,
It's Marek btw ...
> On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote: >> Reword the documentation to make it clear the compatible string is now >> optional, yet still matching on it takes precedence over PCI IDs and >> PCI classes. >> >> Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com >> Cc: Simon Glass sjg@chromium.org >> Cc: Tom Rini trini@konsulko.com >> --- >> V3: No change >> V2: New patch >> --- >> doc/driver-model/pci-info.txt | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/doc/driver-model/pci-info.txt b/doc/driver-model/pci-info.txt >> index e1701d1fbc..14364c5c75 100644 >> --- a/doc/driver-model/pci-info.txt >> +++ b/doc/driver-model/pci-info.txt >> @@ -34,11 +34,15 @@ under that bus. >> Note that this is all done on a lazy basis, as needed, so until something is >> touched on PCI (eg: a call to pci_find_devices()) it will not be probed. >> >> -PCI devices can appear in the flattened device tree. If they do this serves to >> -specify the driver to use for the device. In this case they will be bound at >> -first. Each PCI device node must have a compatible string list as well as a >> -<reg> property, as defined by the IEEE Std 1275-1994 PCI bus binding document >> -v2.1. Note we must describe PCI devices with the same bus hierarchy as the >> +PCI devices can appear in the flattened device tree. If they do, their node >> +often contains extra information which cannot be derived from the PCI IDs or >> +PCI class of the device. Each PCI device node must have a <reg> property, as >> +defined by the IEEE Std 1275-1994 PCI bus binding document v2.1. Compatible >> +string list is optional and generally not needed, since PCI is discoverable > > I really don't like 'generally not needed'. How about 'generally not > essential'? Or that you can usually avoid it if desired.
Must be a language nuance, but the compatible string is really not needed. I am starting to understand where this mindset of "compat strings are generally needed" comes from, which is the design of the virtual PCI devices in sandbox, but that's not the usual case.
Well it's more than that, as I mentioned before. Finding a compatible string in the source code is easier, and if we are matching with a DT node anyway, makes more sense IMO.
It's about as easy as finding PCI ID.
And PCI is a discoverable bus, so using a compatible string is some obscure edge-case.
Anyway since DTs likely come from the newly pleasant Linux we'll just end up with what they have there. This mostly applies for things like x86 which don't use DT in Linux.
> I'd like to say that it is optional since U_BOOT_PCI_DEVICE() can be > used to specific the driver based on conditions like the PCI vendor/, > PCI class, etc. If U-Boot does not find a compatible string then it > will search these U_BOOT_PCI_DEVICE() records to find a driver; > assuming it finds one it will then search for the device-tree node > whose reg property matches the bus/device/function of the device, and > attached that node to the device so that it is accessible to the > driver.
Can you rephrase it better then ? I can paste it into the docs.
How about:
The compatible string is optional since U_BOOT_PCI_DEVICE() can be used to specific
specify ?
the driver based on conditions like the PCI vendor/ PCI class, etc. If U-Boot does not find a compatible string then it will search these U_BOOT_PCI_DEVICE() records to find a driver;
This implies the compatible string is preferred, it is not.
I think Simon was describing the *current* U-Boot implementation, that "compatible" string is looked up first, then U_BOOT_PCI_DEVICE().
This patch updates the documentation to match reality though.
Am I looking at a different pci uclass driver implementation from yours?
Currently in pci_bind_bus_devices(), we have:
/* Find this device in the device tree */ ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev);
This is "compatible" based driver binding, and it goes first.
And we have:
/* If nothing in the device tree, bind a device */ if (ret == -ENODEV) { ... ret = pci_find_and_bind_driver(bus, &find_id, bdf, &dev); ... }
this is the dynamic driver binding based vid/pid, etc.
Your patch does not change the fact that "compatible" comes first than dynamic binding.
So how would the test look like ? I am asking because even if you do bind the driver here, the bind() function is called with a lot of missing stuff, like parent_plat_data (so dm_pci_*() calls do not work there), the DT node is not associated yet. Oh, and the swap-case driver is never probe()d, so you cannot perform the test in probe either ...
Hi Marek,
On 11 September 2018 at 14:58, Marek Vasut marek.vasut@gmail.com wrote:
The PCI controller can have DT subnodes describing extra properties of particular PCI devices, ie. a PHY attached to an EHCI controller on a PCI bus. This patch parses those DT subnodes and assigns a node to the PCI device instance, so that the driver can extract details from that node and ie. configure the PHY using the PHY subsystem.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
V3: Reword the comment V2: Move the whole machinery to pci_bind_bus_devices(), right after the driver instance platform data are updated. This reduces the number of times the DT is traversed and works for both DT nodes with and without compat string.
drivers/pci/pci-uclass.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index e9671d9b76..ab8818280d 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -733,6 +733,7 @@ int pci_bind_bus_devices(struct udevice *bus) ulong vendor, device; ulong header_type; pci_dev_t bdf, end;
ofnode node; bool found_multi; int ret;@@ -803,6 +804,20 @@ int pci_bind_bus_devices(struct udevice *bus) pplat->vendor = vendor; pplat->device = device; pplat->class = class;
/* Associate optional OF node */dev_for_each_subnode(node, bus) {phys_addr_t df, size;df = ofnode_get_addr_size(node, "reg", &size);if (df == FDT_ADDR_T_NONE)continue;if (PCI_FUNC(df) == PCI_FUNC(bdf) &&PCI_DEV(df) == PCI_DEV(bdf)) {dev->node = node;break;}} }
You should only need to do this if dev->node is not already valid.
How about:
if (!ofnode_valid(dev->node)) find_ofnode_for_dev(dev);
then put your code in that fnuction...
Regards, Simon
participants (3)
-
Bin Meng -
Marek Vasut -
Simon Glass