[PATCH] pci: ecm generic: use dev_read_() interface

[PATCH] pci: ecm generic: use dev_read_() interface

[Mayuresh Chitale]

Use dev_read_() api instead of the fdtdec API to fetch the host
controller's reg property value. This is similar to the other host
controller drivers such as Sifive, Rockchip etc. Without this change,
enabling CONFIG_OF_LIVE breaks the PCIe enumeration on Qemu Risc-V virt
machine. The issue is described in the link below:

Link: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1
Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/pci/pcie_ecam_generic.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie_ecam_generic.c b/drivers/pci/pcie_ecam_generic.c
index 1a9f9aec2e..f0effe0001 100644
--- a/drivers/pci/pcie_ecam_generic.c
+++ b/drivers/pci/pcie_ecam_generic.c
@@ -132,19 +132,14 @@ static int pci_generic_ecam_write_config(struct udevice *bus, pci_dev_t bdf,
 static int pci_generic_ecam_of_to_plat(struct udevice *dev)
 {
 	struct generic_ecam_pcie *pcie = dev_get_priv(dev);
-	struct fdt_resource reg_res;
-	DECLARE_GLOBAL_DATA_PTR;
-	int err;
-
-	err = fdt_get_resource(gd->fdt_blob, dev_of_offset(dev), "reg",
-			       0, &reg_res);
-	if (err < 0) {
-		pr_err("\"reg\" resource not found\n");
-		return err;
-	}
-
-	pcie->size = fdt_resource_size(&reg_res);
-	pcie->cfg_base = map_physmem(reg_res.start, pcie->size, MAP_NOCACHE);
+	fdt_addr_t addr;
+	fdt_size_t size;
+
+	addr = dev_read_addr_size(dev, "reg", &size);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+	pcie->size = size;
+	pcie->cfg_base = map_physmem(addr, pcie->size, MAP_NOCACHE);
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH] pci: ecm generic: use dev_read_() interface

[Simon Glass]

On Sat, 18 Feb 2023 at 05:25, Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Use dev_read_() api instead of the fdtdec API to fetch the host
> controller's reg property value. This is similar to the other host
> controller drivers such as Sifive, Rockchip etc. Without this change,
> enabling CONFIG_OF_LIVE breaks the PCIe enumeration on Qemu Risc-V virt
> machine. The issue is described in the link below:
>
> Link: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>  drivers/pci/pcie_ecam_generic.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)

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

Re: [PATCH] pci: ecm generic: use dev_read_() interface

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

On Sat, Feb 18, 2023 at 05:55:25PM +0530, Mayuresh Chitale wrote:

> Use dev_read_() api instead of the fdtdec API to fetch the host
> controller's reg property value. This is similar to the other host
> controller drivers such as Sifive, Rockchip etc. Without this change,
> enabling CONFIG_OF_LIVE breaks the PCIe enumeration on Qemu Risc-V virt
> machine. The issue is described in the link below:
> 
> Link: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  drivers/pci/pcie_ecam_generic.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)

This breaks some platforms such as qemu_arm at a minimum.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] pci: ecm generic: use dev_read_() interface

[Mayuresh Chitale]

On Thu, May 4, 2023 at 2:04 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Feb 18, 2023 at 05:55:25PM +0530, Mayuresh Chitale wrote:
>
> > Use dev_read_() api instead of the fdtdec API to fetch the host
> > controller's reg property value. This is similar to the other host
> > controller drivers such as Sifive, Rockchip etc. Without this change,
> > enabling CONFIG_OF_LIVE breaks the PCIe enumeration on Qemu Risc-V virt
> > machine. The issue is described in the link below:
> >
> > Link: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >  drivers/pci/pcie_ecam_generic.c | 21 ++++++++-------------
> >  1 file changed, 8 insertions(+), 13 deletions(-)
>
> This breaks some platforms such as qemu_arm at a minimum.

Ok. How do I reproduce the issue?

>
> --
> Tom

Re: [PATCH] pci: ecm generic: use dev_read_() interface

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

On Mon, May 08, 2023 at 08:30:15PM +0530, Mayuresh Chitale wrote:
> On Thu, May 4, 2023 at 2:04 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Feb 18, 2023 at 05:55:25PM +0530, Mayuresh Chitale wrote:
> >
> > > Use dev_read_() api instead of the fdtdec API to fetch the host
> > > controller's reg property value. This is similar to the other host
> > > controller drivers such as Sifive, Rockchip etc. Without this change,
> > > enabling CONFIG_OF_LIVE breaks the PCIe enumeration on Qemu Risc-V virt
> > > machine. The issue is described in the link below:
> > >
> > > Link: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1
> > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >  drivers/pci/pcie_ecam_generic.c | 21 ++++++++-------------
> > >  1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > This breaks some platforms such as qemu_arm at a minimum.
> 
> Ok. How do I reproduce the issue?

Right, sorry.  So the failure log is:
https://source.denx.de/u-boot/u-boot/-/jobs/622397
And https://u-boot.readthedocs.io/en/latest/develop/py_testing.html
talks about running the pytest suite outside of CI and
https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html about
how to trigger CI yourself.  One thing that's not documented well enough
is that you'll likely want
https://source.denx.de/u-boot/u-boot-test-hooks as well as you can just
use as-is the "travis-ci" directories (legacy name, used by both gitlab
and azure as-is) as the configuration hooks for your system as well and
then the network tests will run easily, locally.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] pci: ecm generic: use dev_read_() interface

[Simon Glass]

Hi,

On Mon, 8 May 2023 at 09:08, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, May 08, 2023 at 08:30:15PM +0530, Mayuresh Chitale wrote:
> > On Thu, May 4, 2023 at 2:04 AM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Feb 18, 2023 at 05:55:25PM +0530, Mayuresh Chitale wrote:
> > >
> > > > Use dev_read_() api instead of the fdtdec API to fetch the host
> > > > controller's reg property value. This is similar to the other host
> > > > controller drivers such as Sifive, Rockchip etc. Without this change,
> > > > enabling CONFIG_OF_LIVE breaks the PCIe enumeration on Qemu Risc-V virt
> > > > machine. The issue is described in the link below:
> > > >
> > > > Link: https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1
> > > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >  drivers/pci/pcie_ecam_generic.c | 21 ++++++++-------------
> > > >  1 file changed, 8 insertions(+), 13 deletions(-)
> > >
> > > This breaks some platforms such as qemu_arm at a minimum.
> >
> > Ok. How do I reproduce the issue?
>
> Right, sorry.  So the failure log is:
> https://source.denx.de/u-boot/u-boot/-/jobs/622397
> And https://u-boot.readthedocs.io/en/latest/develop/py_testing.html
> talks about running the pytest suite outside of CI and
> https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html about
> how to trigger CI yourself.  One thing that's not documented well enough
> is that you'll likely want
> https://source.denx.de/u-boot/u-boot-test-hooks as well as you can just
> use as-is the "travis-ci" directories (legacy name, used by both gitlab
> and azure as-is) as the configuration hooks for your system as well and
> then the network tests will run easily, locally.

Any update on this one? It is:

https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/1

Regards,
Simon