Message ID | 1577799077-29462-1-git-send-email-sughosh.ganu@linaro.org |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
Series | qemu: arm: Scan the pci bus in board_init | expand |
On Tue, Dec 31, 2019 at 9:31 PM Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > Scan the pci bus in board_init routine before scanning the virtio > devices. This enumerates all the virtio devices, including devices > found on the pci bus. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ > 1 file changed, 7 insertions(+) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
On 12/31/19 2:31 PM, Sughosh Ganu wrote: > Scan the pci bus in board_init routine before scanning the virtio > devices. This enumerates all the virtio devices, including devices > found on the pci bus. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c > index 4e18733..6c5335c 100644 > --- a/board/emulation/qemu-arm/qemu-arm.c > +++ b/board/emulation/qemu-arm/qemu-arm.c > @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; > int board_init(void) > { > /* > + * Scan the pci bus before calling virtio_init. This > + * enumerates all virtio devices, including devices > + * on the pci bus. The last sentence is easy to misunderstand. You surely do not mean that pci_init() would enumerate all virtio devices including mmio virtio devices. I would suggest to rephrase it to "This enumerates all pci devices including virtio devices on the pci bus." With the patch I no longer need to run `virtio scan` on qemu_arm64_defonfig before using the rng command supplied in patch [PATCH 1/1] cmd: add rng command https://lists.denx.de/pipermail/u-boot/2019-December/394539.html Unfortunately you only considered the ARM architecture. We need a solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM). How about eliminating board_init() for ARM and RISC-V and moving virtio_init() to common/board_r.c after initr_pci? Best regards Heinrich > + */ > + pci_init(); > + > + /* > * Make sure virtio bus is enumerated so that peripherals > * on the virtio bus can be discovered by their drivers > */ >
CC Bin Meng On 12/31/19 4:08 PM, Heinrich Schuchardt wrote: > On 12/31/19 2:31 PM, Sughosh Ganu wrote: >> Scan the pci bus in board_init routine before scanning the virtio >> devices. This enumerates all the virtio devices, including devices >> found on the pci bus. >> >> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> >> --- >> board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c >> index 4e18733..6c5335c 100644 >> --- a/board/emulation/qemu-arm/qemu-arm.c >> +++ b/board/emulation/qemu-arm/qemu-arm.c >> @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; >> int board_init(void) >> { >> /* >> + * Scan the pci bus before calling virtio_init. This >> + * enumerates all virtio devices, including devices >> + * on the pci bus. > > The last sentence is easy to misunderstand. You surely do not mean that > pci_init() would enumerate all virtio devices including mmio virtio devices. > > I would suggest to rephrase it to "This enumerates all pci devices > including virtio devices on the pci bus." > > With the patch I no longer need to run `virtio scan` on > qemu_arm64_defonfig before using the rng command supplied in patch > > [PATCH 1/1] cmd: add rng command > https://lists.denx.de/pipermail/u-boot/2019-December/394539.html > > Unfortunately you only considered the ARM architecture. We need a > solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM). > > How about eliminating board_init() for ARM and RISC-V and moving > virtio_init() to common/board_r.c after initr_pci? > > Best regards > > Heinrich > >> + */ >> + pci_init(); >> + >> + /* >> * Make sure virtio bus is enumerated so that peripherals >> * on the virtio bus can be discovered by their drivers >> */ >> > >
On Tue, Dec 31, 2019 at 11:18 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > CC Bin Meng > > On 12/31/19 4:08 PM, Heinrich Schuchardt wrote: > > On 12/31/19 2:31 PM, Sughosh Ganu wrote: > >> Scan the pci bus in board_init routine before scanning the virtio > >> devices. This enumerates all the virtio devices, including devices > >> found on the pci bus. > >> > >> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > >> --- > >> board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c > >> index 4e18733..6c5335c 100644 > >> --- a/board/emulation/qemu-arm/qemu-arm.c > >> +++ b/board/emulation/qemu-arm/qemu-arm.c > >> @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; > >> int board_init(void) > >> { > >> /* > >> + * Scan the pci bus before calling virtio_init. This > >> + * enumerates all virtio devices, including devices > >> + * on the pci bus. > > > > The last sentence is easy to misunderstand. You surely do not mean that > > pci_init() would enumerate all virtio devices including mmio virtio devices. > > > > I would suggest to rephrase it to "This enumerates all pci devices > > including virtio devices on the pci bus." Sounds good. > > > > With the patch I no longer need to run `virtio scan` on > > qemu_arm64_defonfig before using the rng command supplied in patch > > > > [PATCH 1/1] cmd: add rng command > > https://lists.denx.de/pipermail/u-boot/2019-December/394539.html > > > > Unfortunately you only considered the ARM architecture. We need a > > solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM). > > Agreed, sorry I missed that. Actually all the board_init() look similar so we should do the same. > > How about eliminating board_init() for ARM and RISC-V and moving > > virtio_init() to common/board_r.c after initr_pci? > > Or maybe we introduce a new driver model flag so that uclass drivers are automatically probed? Regards, Bin
On Tue, 31 Dec 2019 at 20:38, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 12/31/19 2:31 PM, Sughosh Ganu wrote: > > Scan the pci bus in board_init routine before scanning the virtio > > devices. This enumerates all the virtio devices, including devices > > found on the pci bus. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/board/emulation/qemu-arm/qemu-arm.c > b/board/emulation/qemu-arm/qemu-arm.c > > index 4e18733..6c5335c 100644 > > --- a/board/emulation/qemu-arm/qemu-arm.c > > +++ b/board/emulation/qemu-arm/qemu-arm.c > > @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; > > int board_init(void) > > { > > /* > > + * Scan the pci bus before calling virtio_init. This > > + * enumerates all virtio devices, including devices > > + * on the pci bus. > > The last sentence is easy to misunderstand. You surely do not mean that > pci_init() would enumerate all virtio devices including mmio virtio > devices. > > I would suggest to rephrase it to "This enumerates all pci devices > including virtio devices on the pci bus." > Ok. > > With the patch I no longer need to run `virtio scan` on > qemu_arm64_defonfig before using the rng command supplied in patch > > [PATCH 1/1] cmd: add rng command > https://lists.denx.de/pipermail/u-boot/2019-December/394539.html > > Unfortunately you only considered the ARM architecture. We need a > solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM). > That is fine with me, but currently, only risc-v and arm architectures are calling virtio_init. > > How about eliminating board_init() for ARM and RISC-V and moving > virtio_init() to common/board_r.c after initr_pci? > That can be done, but currently, for some reason, initr_pci function calls pci_init only when CONFIG_DM_PCI is not defined. Not sure why that is the case, although initr_pci gets called after initr_dm. -sughosh
On 12/31/19 4:49 PM, Sughosh Ganu wrote: > > On Tue, 31 Dec 2019 at 20:38, Heinrich Schuchardt <xypron.glpk@gmx.de > <mailto:xypron.glpk@gmx.de>> wrote: > > On 12/31/19 2:31 PM, Sughosh Ganu wrote: > > Scan the pci bus in board_init routine before scanning the virtio > > devices. This enumerates all the virtio devices, including devices > > found on the pci bus. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org > <mailto:sughosh.ganu@linaro.org>> > > --- > > board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/board/emulation/qemu-arm/qemu-arm.c > b/board/emulation/qemu-arm/qemu-arm.c > > index 4e18733..6c5335c 100644 > > --- a/board/emulation/qemu-arm/qemu-arm.c > > +++ b/board/emulation/qemu-arm/qemu-arm.c > > @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; > > int board_init(void) > > { > > /* > > + * Scan the pci bus before calling virtio_init. This > > + * enumerates all virtio devices, including devices > > + * on the pci bus. > > The last sentence is easy to misunderstand. You surely do not mean that > pci_init() would enumerate all virtio devices including mmio virtio > devices. > > I would suggest to rephrase it to "This enumerates all pci devices > including virtio devices on the pci bus." > > > Ok. > > > With the patch I no longer need to run `virtio scan` on > qemu_arm64_defonfig before using the rng command supplied in patch > > [PATCH 1/1] cmd: add rng command > https://lists.denx.de/pipermail/u-boot/2019-December/394539.html > > Unfortunately you only considered the ARM architecture. We need a > solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM). > > > That is fine with me, but currently, only risc-v and arm architectures > are calling virtio_init. > > > How about eliminating board_init() for ARM and RISC-V and moving > virtio_init() to common/board_r.c after initr_pci? > > > That can be done, but currently, for some reason, initr_pci function > calls pci_init only when CONFIG_DM_PCI is not defined. Not sure why that > is the case, although initr_pci gets called after initr_dm. > > -sughosh This dates back to Simon's commit ff3e077bd23c ("dm: pci: Add a uclass for PCI"). I hope that Simon can tell us why he chose not to call pci_init() if DM_PCI is defined. Best regards Heinrich
Hi Heinrich, On Tue, 31 Dec 2019 at 09:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 12/31/19 4:49 PM, Sughosh Ganu wrote: > > > > On Tue, 31 Dec 2019 at 20:38, Heinrich Schuchardt <xypron.glpk@gmx.de > > <mailto:xypron.glpk@gmx.de>> wrote: > > > > On 12/31/19 2:31 PM, Sughosh Ganu wrote: > > > Scan the pci bus in board_init routine before scanning the virtio > > > devices. This enumerates all the virtio devices, including devices > > > found on the pci bus. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org > > <mailto:sughosh.ganu@linaro.org>> > > > --- > > > board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/board/emulation/qemu-arm/qemu-arm.c > > b/board/emulation/qemu-arm/qemu-arm.c > > > index 4e18733..6c5335c 100644 > > > --- a/board/emulation/qemu-arm/qemu-arm.c > > > +++ b/board/emulation/qemu-arm/qemu-arm.c > > > @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; > > > int board_init(void) > > > { > > > /* > > > + * Scan the pci bus before calling virtio_init. This > > > + * enumerates all virtio devices, including devices > > > + * on the pci bus. > > > > The last sentence is easy to misunderstand. You surely do not mean that > > pci_init() would enumerate all virtio devices including mmio virtio > > devices. > > > > I would suggest to rephrase it to "This enumerates all pci devices > > including virtio devices on the pci bus." > > > > > > Ok. > > > > > > With the patch I no longer need to run `virtio scan` on > > qemu_arm64_defonfig before using the rng command supplied in patch > > > > [PATCH 1/1] cmd: add rng command > > https://lists.denx.de/pipermail/u-boot/2019-December/394539.html > > > > Unfortunately you only considered the ARM architecture. We need a > > solution that works on all QEMU architectures (MIPS, RISC-V, X86, ARM). > > > > > > That is fine with me, but currently, only risc-v and arm architectures > > are calling virtio_init. > > > > > > How about eliminating board_init() for ARM and RISC-V and moving > > virtio_init() to common/board_r.c after initr_pci? > > > > > > That can be done, but currently, for some reason, initr_pci function > > calls pci_init only when CONFIG_DM_PCI is not defined. Not sure why that > > is the case, although initr_pci gets called after initr_dm. > > > > -sughosh > > This dates back to Simon's commit ff3e077bd23c ("dm: pci: Add a uclass > for PCI"). > > I hope that Simon can tell us why he chose not to call pci_init() if > DM_PCI is defined. Driver model uses lazy init. We have discussed this before, and considered using a uclass flag to tell U-Boot that the uclass needs to be fully probed (i.e. probe all devices in that uclass, recursively). But I'm not sure we got agreement. If you think it is a good idea you could send a patch. The only thing is that I think some platforms don't need PCI to boot so might not want this? Regards, Simon
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index 4e18733..6c5335c 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -63,6 +63,13 @@ struct mm_region *mem_map = qemu_arm64_mem_map; int board_init(void) { /* + * Scan the pci bus before calling virtio_init. This + * enumerates all virtio devices, including devices + * on the pci bus. + */ + pci_init(); + + /* * Make sure virtio bus is enumerated so that peripherals * on the virtio bus can be discovered by their drivers */
Scan the pci bus in board_init routine before scanning the virtio devices. This enumerates all the virtio devices, including devices found on the pci bus. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- board/emulation/qemu-arm/qemu-arm.c | 7 +++++++ 1 file changed, 7 insertions(+)