diff mbox series

[U-Boot,032/126] pci: Show a message if PCI autoconfig fails

Message ID 20190925145750.200592-33-sjg@chromium.org
State Accepted
Delegated to: Bin Meng
Headers show
Series x86: Add initial support for apollolake | expand

Commit Message

Simon Glass Sept. 25, 2019, 2:56 p.m. UTC
At present this fails silently which can be confusing since some devices
on the PCI bus may not work correctly. Show a message in this case

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

 drivers/pci/pci_auto.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Bin Meng Oct. 5, 2019, 1:12 p.m. UTC | #1
On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@chromium.org> wrote:
>
> At present this fails silently which can be confusing since some devices
> on the PCI bus may not work correctly. Show a message in this case

nits: missing . after case

>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/pci/pci_auto.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> index 1a3bf708347..7755ffb6fa2 100644
> --- a/drivers/pci/pci_auto.c
> +++ b/drivers/pci/pci_auto.c
> @@ -39,6 +39,8 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
>
>         for (bar = PCI_BASE_ADDRESS_0;
>              bar < PCI_BASE_ADDRESS_0 + (bars_num * 4); bar += 4) {
> +               int ret = 0;
> +
>                 /* Tickle the BAR and get the response */
>                 if (!enum_only)
>                         dm_pci_write_config32(dev, bar, 0xffffffff);
> @@ -97,9 +99,13 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
>                               (unsigned long long)bar_size);
>                 }
>
> -               if (!enum_only && pciauto_region_allocate(bar_res, bar_size,
> -                                                         &bar_value,
> -                                                         found_mem64) == 0) {
> +               if (!enum_only) {
> +                       ret = pciauto_region_allocate(bar_res, bar_size,
> +                                                     &bar_value, found_mem64);
> +                       if (ret)
> +                               printf("PCI: Failed autoconfig bar %x", bar);

nits: should have a '\n'

> +               }
> +               if (!enum_only && !ret) {
>                         /* Write it out and update our limit */
>                         dm_pci_write_config32(dev, bar, (u32)bar_value);
>
> --

Other than above,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Bin Meng Oct. 6, 2019, 11:19 a.m. UTC | #2
On Sat, Oct 5, 2019 at 9:12 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > At present this fails silently which can be confusing since some devices
> > on the PCI bus may not work correctly. Show a message in this case
>
> nits: missing . after case

Fixed, and

>
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  drivers/pci/pci_auto.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> > index 1a3bf708347..7755ffb6fa2 100644
> > --- a/drivers/pci/pci_auto.c
> > +++ b/drivers/pci/pci_auto.c
> > @@ -39,6 +39,8 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
> >
> >         for (bar = PCI_BASE_ADDRESS_0;
> >              bar < PCI_BASE_ADDRESS_0 + (bars_num * 4); bar += 4) {
> > +               int ret = 0;
> > +
> >                 /* Tickle the BAR and get the response */
> >                 if (!enum_only)
> >                         dm_pci_write_config32(dev, bar, 0xffffffff);
> > @@ -97,9 +99,13 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
> >                               (unsigned long long)bar_size);
> >                 }
> >
> > -               if (!enum_only && pciauto_region_allocate(bar_res, bar_size,
> > -                                                         &bar_value,
> > -                                                         found_mem64) == 0) {
> > +               if (!enum_only) {
> > +                       ret = pciauto_region_allocate(bar_res, bar_size,
> > +                                                     &bar_value, found_mem64);
> > +                       if (ret)
> > +                               printf("PCI: Failed autoconfig bar %x", bar);
>
> nits: should have a '\n'

Added a '\n', and

>
> > +               }
> > +               if (!enum_only && !ret) {
> >                         /* Write it out and update our limit */
> >                         dm_pci_write_config32(dev, bar, (u32)bar_value);
> >
> > --
>
> Other than above,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86/next, thanks!
diff mbox series

Patch

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 1a3bf708347..7755ffb6fa2 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -39,6 +39,8 @@  void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
 
 	for (bar = PCI_BASE_ADDRESS_0;
 	     bar < PCI_BASE_ADDRESS_0 + (bars_num * 4); bar += 4) {
+		int ret = 0;
+
 		/* Tickle the BAR and get the response */
 		if (!enum_only)
 			dm_pci_write_config32(dev, bar, 0xffffffff);
@@ -97,9 +99,13 @@  void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
 			      (unsigned long long)bar_size);
 		}
 
-		if (!enum_only && pciauto_region_allocate(bar_res, bar_size,
-							  &bar_value,
-							  found_mem64) == 0) {
+		if (!enum_only) {
+			ret = pciauto_region_allocate(bar_res, bar_size,
+						      &bar_value, found_mem64);
+			if (ret)
+				printf("PCI: Failed autoconfig bar %x", bar);
+		}
+		if (!enum_only && !ret) {
 			/* Write it out and update our limit */
 			dm_pci_write_config32(dev, bar, (u32)bar_value);