diff mbox

[U-Boot,05/10] dm: pci: Adjust pci_find_and_bind_driver() to return -EPERM

Message ID 1441756374-6762-6-git-send-email-sjg@chromium.org
State Accepted
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Sept. 8, 2015, 11:52 p.m. UTC
The current code returns 0 even if it failed to find or bind a driver. The
caller then has to check the returned device to see if it is NULL. It is
better to return an error code in this case so that it is clear what
happened.

Adjust the code to return -EPERM, indicating that the device was not bound
because it is not needed for pre-relocation use. Add comments so that the
return value is clear.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/pci/pci-uclass.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

Comments

Bin Meng Sept. 14, 2015, 12:15 p.m. UTC | #1
On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass <sjg@chromium.org> wrote:
> The current code returns 0 even if it failed to find or bind a driver. The
> caller then has to check the returned device to see if it is NULL. It is
> better to return an error code in this case so that it is clear what
> happened.
>
> Adjust the code to return -EPERM, indicating that the device was not bound
> because it is not needed for pre-relocation use. Add comments so that the
> return value is clear.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/pci/pci-uclass.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 7347160..54285ee 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -458,10 +458,17 @@ static bool pci_match_one_id(const struct pci_device_id *id,
>   * pci_find_and_bind_driver() - Find and bind the right PCI driver
>   *
>   * This only looks at certain fields in the descriptor.
> + *
> + * @parent:    Parent bus
> + * @find_id:   Specification of the driver to find
> + * @bdf:       Bus/device/function addreess - see PCI_BDF()
> + * @devp:      Returns a pointer to the device created
> + * @return 0 if OK, -EPERM if the device is not needed before relocation and
> + *        therefore was not created, other -ve value on error
>   */
>  static int pci_find_and_bind_driver(struct udevice *parent,
> -                                   struct pci_device_id *find_id, pci_dev_t bdf,
> -                                   struct udevice **devp)
> +                                   struct pci_device_id *find_id,
> +                                   pci_dev_t bdf, struct udevice **devp)
>  {
>         struct pci_driver_entry *start, *entry;
>         const char *drv;
> @@ -497,7 +504,7 @@ static int pci_find_and_bind_driver(struct udevice *parent,
>                          */
>                         if (!(gd->flags & GD_FLG_RELOC) &&
>                             !(drv->flags & DM_FLAG_PRE_RELOC))
> -                               return 0;
> +                               return -EPERM;
>
>                         /*
>                          * We could pass the descriptor to the driver as
> @@ -525,7 +532,7 @@ static int pci_find_and_bind_driver(struct udevice *parent,
>          * limited (ie: using Cache As RAM).
>          */
>         if (!(gd->flags & GD_FLG_RELOC) && !bridge)
> -               return 0;
> +               return -EPERM;
>
>         /* Bind a generic driver so that the device can be used */
>         sprintf(name, "pci_%x:%x.%x", parent->seq, PCI_DEV(bdf),
> @@ -613,17 +620,17 @@ int pci_bind_bus_devices(struct udevice *bus)
>                         ret = pci_find_and_bind_driver(bus, &find_id, bdf,
>                                                        &dev);
>                 }
> -               if (ret)
> +               if (ret == -EPERM)
> +                       continue;
> +               else if (ret)
>                         return ret;
>
>                 /* Update the platform data */
> -               if (dev) {
> -                       pplat = dev_get_parent_platdata(dev);
> -                       pplat->devfn = PCI_MASK_BUS(bdf);
> -                       pplat->vendor = vendor;
> -                       pplat->device = device;
> -                       pplat->class = class;
> -               }
> +               pplat = dev_get_parent_platdata(dev);
> +               pplat->devfn = PCI_MASK_BUS(bdf);
> +               pplat->vendor = vendor;
> +               pplat->device = device;
> +               pplat->class = class;
>         }
>
>         return 0;
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Simon Glass Oct. 18, 2015, 12:28 p.m. UTC | #2
On 14 September 2015 at 06:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass <sjg@chromium.org> wrote:
>> The current code returns 0 even if it failed to find or bind a driver. The
>> caller then has to check the returned device to see if it is NULL. It is
>> better to return an error code in this case so that it is clear what
>> happened.
>>
>> Adjust the code to return -EPERM, indicating that the device was not bound
>> because it is not needed for pre-relocation use. Add comments so that the
>> return value is clear.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  drivers/pci/pci-uclass.c | 31 +++++++++++++++++++------------
>>  1 file changed, 19 insertions(+), 12 deletions(-)
[snip]
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Applied to u-boot-x86.
diff mbox

Patch

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 7347160..54285ee 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -458,10 +458,17 @@  static bool pci_match_one_id(const struct pci_device_id *id,
  * pci_find_and_bind_driver() - Find and bind the right PCI driver
  *
  * This only looks at certain fields in the descriptor.
+ *
+ * @parent:	Parent bus
+ * @find_id:	Specification of the driver to find
+ * @bdf:	Bus/device/function addreess - see PCI_BDF()
+ * @devp:	Returns a pointer to the device created
+ * @return 0 if OK, -EPERM if the device is not needed before relocation and
+ *	   therefore was not created, other -ve value on error
  */
 static int pci_find_and_bind_driver(struct udevice *parent,
-				    struct pci_device_id *find_id, pci_dev_t bdf,
-				    struct udevice **devp)
+				    struct pci_device_id *find_id,
+				    pci_dev_t bdf, struct udevice **devp)
 {
 	struct pci_driver_entry *start, *entry;
 	const char *drv;
@@ -497,7 +504,7 @@  static int pci_find_and_bind_driver(struct udevice *parent,
 			 */
 			if (!(gd->flags & GD_FLG_RELOC) &&
 			    !(drv->flags & DM_FLAG_PRE_RELOC))
-				return 0;
+				return -EPERM;
 
 			/*
 			 * We could pass the descriptor to the driver as
@@ -525,7 +532,7 @@  static int pci_find_and_bind_driver(struct udevice *parent,
 	 * limited (ie: using Cache As RAM).
 	 */
 	if (!(gd->flags & GD_FLG_RELOC) && !bridge)
-		return 0;
+		return -EPERM;
 
 	/* Bind a generic driver so that the device can be used */
 	sprintf(name, "pci_%x:%x.%x", parent->seq, PCI_DEV(bdf),
@@ -613,17 +620,17 @@  int pci_bind_bus_devices(struct udevice *bus)
 			ret = pci_find_and_bind_driver(bus, &find_id, bdf,
 						       &dev);
 		}
-		if (ret)
+		if (ret == -EPERM)
+			continue;
+		else if (ret)
 			return ret;
 
 		/* Update the platform data */
-		if (dev) {
-			pplat = dev_get_parent_platdata(dev);
-			pplat->devfn = PCI_MASK_BUS(bdf);
-			pplat->vendor = vendor;
-			pplat->device = device;
-			pplat->class = class;
-		}
+		pplat = dev_get_parent_platdata(dev);
+		pplat->devfn = PCI_MASK_BUS(bdf);
+		pplat->vendor = vendor;
+		pplat->device = device;
+		pplat->class = class;
 	}
 
 	return 0;