[U-Boot] [PATCH v2] pci: Avoid assigning PCI resources that are below 0x1000

[U-Boot] [PATCH v2] pci: Avoid assigning PCI resources that are below 0x1000

[Bin Meng]

commit b7598a43f2b4 ("[PATCH] Avoid assigning PCI resources from
zero address") only moved the bus lower address to 0x1000 if the
given bus start address is zero. The comment said 0x1000 is a
reasonable starting value, hence we'd better apply the same
adjustment when the given bus start address is below 0x1000.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- avoid changing res->bus_start

 drivers/pci/pci_auto_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci_auto_common.c b/drivers/pci/pci_auto_common.c
index 1837873..84908e6 100644
--- a/drivers/pci/pci_auto_common.c
+++ b/drivers/pci/pci_auto_common.c
@@ -21,9 +21,10 @@ void pciauto_region_init(struct pci_region *res)
 	/*
 	 * Avoid allocating PCI resources from address 0 -- this is illegal
 	 * according to PCI 2.1 and moreover, this is known to cause Linux IDE
-	 * drivers to fail. Use a reasonable starting value of 0x1000 instead.
+	 * drivers to fail. Use a reasonable starting value of 0x1000 instead
+	 * if the bus start address is below 0x1000.
 	 */
-	res->bus_lower = res->bus_start ? res->bus_start : 0x1000;
+	res->bus_lower = res->bus_start < 0x1000 ? 0x1000 : res->bus_start;
 }
 
 void pciauto_region_align(struct pci_region *res, pci_size_t size)
-- 
2.7.4

[U-Boot] [PATCH v2] pci: Avoid assigning PCI resources that are below 0x1000

[Stefan Roese]

On 05.06.19 16:26, Bin Meng wrote:
> commit b7598a43f2b4 ("[PATCH] Avoid assigning PCI resources from
> zero address") only moved the bus lower address to 0x1000 if the
> given bus start address is zero. The comment said 0x1000 is a
> reasonable starting value, hence we'd better apply the same
> adjustment when the given bus start address is below 0x1000.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

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

Though again my question, where you did spot an issue with the
current implementation?

Thanks,
Stefan

[U-Boot] [PATCH v2] pci: Avoid assigning PCI resources that are below 0x1000

[Bin Meng]

Hi Stefan,

On Wed, Jun 5, 2019 at 10:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> commit b7598a43f2b4 ("[PATCH] Avoid assigning PCI resources from
> zero address") only moved the bus lower address to 0x1000 if the
> given bus start address is zero. The comment said 0x1000 is a
> reasonable starting value, hence we'd better apply the same
> adjustment when the given bus start address is below 0x1000.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - avoid changing res->bus_start

Somehow I did not receive your RB in my inbox. However when I look at
the patch in patchwork [1], I found you already gave your RB tag, so
there must have be something wrong with gmail.

Regarding your question, first it was just pure code review before and
at that time I did not spot any actual issue. However I indeed created
a test case to trigger the issue.

If I modified qemu-x86_i440fx.dts pci range to have the PCI IO region
address starts from 0x100, when Linux boots, it reports the following:
[    0.445751] pci 0000:00:03.0: can't claim BAR 1 [io
0x0140-0x017f]: address conflict with 0000:00:01.1 [io  0x0170-0x0177]

With my patch, there is no longer such complaints from the kernel driver.

[1] http://patchwork.ozlabs.org/patch/1110510/

Regards,
Bin

[U-Boot] [PATCH v2] pci: Avoid assigning PCI resources that are below 0x1000

[Tom Rini]

On Wed, Jun 05, 2019 at 07:26:44AM -0700, Bin Meng wrote:

> commit b7598a43f2b4 ("[PATCH] Avoid assigning PCI resources from
> zero address") only moved the bus lower address to 0x1000 if the
> given bus start address is zero. The comment said 0x1000 is a
> reasonable starting value, hence we'd better apply the same
> adjustment when the given bus start address is below 0x1000.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Stefan Roese <sr@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190622/306743a1/attachment.sig>