diff mbox series

[2/3] pci: Make auto-config code a little more robust

Message ID 20210103220646.26707-3-phil@nwl.cc
State Accepted
Commit c1b1263b160ece33b2164dc3cd107f0d1dec52fd
Delegated to: Stefan Roese
Headers show
Series A minor DS414 update | expand

Commit Message

Phil Sutter Jan. 3, 2021, 10:06 p.m. UTC
On my DS414, some PCI devices return odd values when probing BAR sizes.
An obvious case is all-ones response, the Linux driver
(drivers/pci/probe.c) catches those explicitly and a comment explains
that either bit 0 or bit 1 must be clear (depending on MEM or IO type).
Other BARs return e.g. 0xfff0000f or 0xfff00004 and thus manage to break
size calculation due to the "middle" zeroes. Mitigate that copying more
or less what Linux does and do a "find least bit set".

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 drivers/pci/pci_auto.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Stefan Roese Jan. 5, 2021, 3:57 p.m. UTC | #1
Adding a few more PCI experts (Simon & Bin) to Cc.

On 03.01.21 23:06, Phil Sutter wrote:
> On my DS414, some PCI devices return odd values when probing BAR sizes.
> An obvious case is all-ones response, the Linux driver
> (drivers/pci/probe.c) catches those explicitly and a comment explains
> that either bit 0 or bit 1 must be clear (depending on MEM or IO type).
> Other BARs return e.g. 0xfff0000f or 0xfff00004 and thus manage to break
> size calculation due to the "middle" zeroes. Mitigate that copying more
> or less what Linux does and do a "find least bit set".
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>   drivers/pci/pci_auto.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
> index 3f46b7697d7ca..ea202b0e0959e 100644
> --- a/drivers/pci/pci_auto.c
> +++ b/drivers/pci/pci_auto.c
> @@ -47,16 +47,17 @@ void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
>   			dm_pci_write_config32(dev, bar, 0xffffffff);
>   		dm_pci_read_config32(dev, bar, &bar_response);
>   
> -		/* If BAR is not implemented go to the next BAR */
> -		if (!bar_response)
> +		/* If BAR is not implemented (or invalid) go to the next BAR */
> +		if (!bar_response || bar_response == 0xffffffff)
>   			continue;
>   
>   		found_mem64 = 0;
>   
>   		/* Check the BAR type and set our address mask */
>   		if (bar_response & PCI_BASE_ADDRESS_SPACE) {
> -			bar_size = ((~(bar_response & PCI_BASE_ADDRESS_IO_MASK))
> -				   & 0xffff) + 1;
> +			bar_size = bar_response & PCI_BASE_ADDRESS_IO_MASK;
> +			bar_size &= ~(bar_size - 1);
> +
>   			if (!enum_only)
>   				bar_res = io;
>   

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan
Simon Glass Jan. 7, 2021, 12:36 p.m. UTC | #2
On Sun, 3 Jan 2021 at 15:05, Phil Sutter <phil@nwl.cc> wrote:
>
> On my DS414, some PCI devices return odd values when probing BAR sizes.
> An obvious case is all-ones response, the Linux driver
> (drivers/pci/probe.c) catches those explicitly and a comment explains
> that either bit 0 or bit 1 must be clear (depending on MEM or IO type).
> Other BARs return e.g. 0xfff0000f or 0xfff00004 and thus manage to break
> size calculation due to the "middle" zeroes. Mitigate that copying more
> or less what Linux does and do a "find least bit set".
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  drivers/pci/pci_auto.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox series

Patch

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 3f46b7697d7ca..ea202b0e0959e 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -47,16 +47,17 @@  void dm_pciauto_setup_device(struct udevice *dev, int bars_num,
 			dm_pci_write_config32(dev, bar, 0xffffffff);
 		dm_pci_read_config32(dev, bar, &bar_response);
 
-		/* If BAR is not implemented go to the next BAR */
-		if (!bar_response)
+		/* If BAR is not implemented (or invalid) go to the next BAR */
+		if (!bar_response || bar_response == 0xffffffff)
 			continue;
 
 		found_mem64 = 0;
 
 		/* Check the BAR type and set our address mask */
 		if (bar_response & PCI_BASE_ADDRESS_SPACE) {
-			bar_size = ((~(bar_response & PCI_BASE_ADDRESS_IO_MASK))
-				   & 0xffff) + 1;
+			bar_size = bar_response & PCI_BASE_ADDRESS_IO_MASK;
+			bar_size &= ~(bar_size - 1);
+
 			if (!enum_only)
 				bar_res = io;