| Message ID | 20180911125831.18851-2-marek.vasut+renesas@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot,V3,1/2] pci: Support parsing PCI controller DT subnodes | expand |
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 ...
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:
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(-)