[PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Vincent Chen]

When the debug message of driver/base/dd.c is enabled, the following
error messages are present in the boot log:

[    0.207941] platform d000000.aplic: error -EPROBE_DEFER: supplier
28000000.imsics not ready
[    0.208115] platform d000000.aplic: Added to deferred list

The reason for this error message is that the probe of APLIC is executed
earlier than IMSIC. This error also causes all the platform devices
connected to the APLIC to be added to the deferred list. Because both
APLIC and IMSIC are registered by device_initcall, this patch adjusts the
compile order of APLIC and IMSIC to ensure that the probe of IMSIC is
executed earlier than the probe of APLIC.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 drivers/irqchip/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 15635812b2d6..3c09666569d6 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -96,9 +96,9 @@ obj-$(CONFIG_QCOM_MPM)			+= irq-qcom-mpm.o
 obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
 obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
 obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
+obj-$(CONFIG_RISCV_IMSIC)		+= irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
 obj-$(CONFIG_RISCV_APLIC)		+= irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
 obj-$(CONFIG_RISCV_APLIC_MSI)		+= irq-riscv-aplic-msi.o
-obj-$(CONFIG_RISCV_IMSIC)		+= irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
 obj-$(CONFIG_SIFIVE_PLIC)		+= irq-sifive-plic.o
 obj-$(CONFIG_STARFIVE_JH8100_INTC)	+= irq-starfive-jh8100-intc.o
 obj-$(CONFIG_IMX_IRQSTEER)		+= irq-imx-irqsteer.o
-- 
2.34.1

Re: [PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Thomas Gleixner]

On Fri, Aug 02 2024 at 15:57, Vincent Chen wrote:
> When the debug message of driver/base/dd.c is enabled, the following
> error messages are present in the boot log:
>
> [    0.207941] platform d000000.aplic: error -EPROBE_DEFER: supplier
> 28000000.imsics not ready
> [    0.208115] platform d000000.aplic: Added to deferred list

Deferred probing is not an error.

> The reason for this error message is that the probe of APLIC is executed
> earlier than IMSIC. This error also causes all the platform devices
> connected to the APLIC to be added to the deferred list. Because both
> APLIC and IMSIC are registered by device_initcall, this patch adjusts the
> compile order of APLIC and IMSIC to ensure that the probe of IMSIC is
> executed earlier than the probe of APLIC.

And no we are not playing silly ordering games just to suppress a debug
output.

Thanks,

        tglx

Re: [PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Anup Patel]

On Fri, Aug 2, 2024 at 1:27 PM Vincent Chen <vincent.chen@sifive.com> wrote:
>
> When the debug message of driver/base/dd.c is enabled, the following
> error messages are present in the boot log:
>
> [    0.207941] platform d000000.aplic: error -EPROBE_DEFER: supplier
> 28000000.imsics not ready
> [    0.208115] platform d000000.aplic: Added to deferred list

We are relying on fw_devlink implemented by Linux DD core to do the
probe ordering. The above prober defer message implies that the
Linux DD core is doing its job correctly.

>
> The reason for this error message is that the probe of APLIC is executed
> earlier than IMSIC. This error also causes all the platform devices
> connected to the APLIC to be added to the deferred list. Because both
> APLIC and IMSIC are registered by device_initcall, this patch adjusts the
> compile order of APLIC and IMSIC to ensure that the probe of IMSIC is
> executed earlier than the probe of APLIC.
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  drivers/irqchip/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 15635812b2d6..3c09666569d6 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -96,9 +96,9 @@ obj-$(CONFIG_QCOM_MPM)                        += irq-qcom-mpm.o
>  obj-$(CONFIG_CSKY_MPINTC)              += irq-csky-mpintc.o
>  obj-$(CONFIG_CSKY_APB_INTC)            += irq-csky-apb-intc.o
>  obj-$(CONFIG_RISCV_INTC)               += irq-riscv-intc.o
> +obj-$(CONFIG_RISCV_IMSIC)              += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
>  obj-$(CONFIG_RISCV_APLIC)              += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
>  obj-$(CONFIG_RISCV_APLIC_MSI)          += irq-riscv-aplic-msi.o
> -obj-$(CONFIG_RISCV_IMSIC)              += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o

First of all there is no issue here.

Secondly, changing compilation order in Makefile to influence
the probe order will not help in any way.

>  obj-$(CONFIG_SIFIVE_PLIC)              += irq-sifive-plic.o
>  obj-$(CONFIG_STARFIVE_JH8100_INTC)     += irq-starfive-jh8100-intc.o
>  obj-$(CONFIG_IMX_IRQSTEER)             += irq-imx-irqsteer.o
> --
> 2.34.1
>

Regards,
Anup

Re: [PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Vincent Chen]

On Fri, Aug 2, 2024 at 7:03 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Fri, Aug 2, 2024 at 1:27 PM Vincent Chen <vincent.chen@sifive.com> wrote:
> >
> > When the debug message of driver/base/dd.c is enabled, the following
> > error messages are present in the boot log:
> >
> > [    0.207941] platform d000000.aplic: error -EPROBE_DEFER: supplier
> > 28000000.imsics not ready
> > [    0.208115] platform d000000.aplic: Added to deferred list
>
> We are relying on fw_devlink implemented by Linux DD core to do the
> probe ordering. The above prober defer message implies that the
> Linux DD core is doing its job correctly.
>
> >
> > The reason for this error message is that the probe of APLIC is executed
> > earlier than IMSIC. This error also causes all the platform devices
> > connected to the APLIC to be added to the deferred list. Because both
> > APLIC and IMSIC are registered by device_initcall, this patch adjusts the
> > compile order of APLIC and IMSIC to ensure that the probe of IMSIC is
> > executed earlier than the probe of APLIC.
> >
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> >  drivers/irqchip/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 15635812b2d6..3c09666569d6 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -96,9 +96,9 @@ obj-$(CONFIG_QCOM_MPM)                        += irq-qcom-mpm.o
> >  obj-$(CONFIG_CSKY_MPINTC)              += irq-csky-mpintc.o
> >  obj-$(CONFIG_CSKY_APB_INTC)            += irq-csky-apb-intc.o
> >  obj-$(CONFIG_RISCV_INTC)               += irq-riscv-intc.o
> > +obj-$(CONFIG_RISCV_IMSIC)              += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
> >  obj-$(CONFIG_RISCV_APLIC)              += irq-riscv-aplic-main.o irq-riscv-aplic-direct.o
> >  obj-$(CONFIG_RISCV_APLIC_MSI)          += irq-riscv-aplic-msi.o
> > -obj-$(CONFIG_RISCV_IMSIC)              += irq-riscv-imsic-state.o irq-riscv-imsic-early.o irq-riscv-imsic-platform.o
>
> First of all there is no issue here.
>

Hi Anup,
I understood that the defer message implies that the Linux DD core is
doing its job correctly. I also agree with you that there is no issue
here because I still can boot kernel successfully. I just hope this
process becomes smoother by adjusting the probe order because the
APLIC will depend on IMSIC when it is present.

> Secondly, changing compilation order in Makefile to influence
> the probe order will not help in any way.
>
I was confused here. If possible, hope you can help me clarify it.
The following is the backtrace of really_porbe() dumped by GDB.
#0  0xffffffff8092318a in really_probe ()
#1  0xffffffff80923516 in __driver_probe_device.part.0 ()
#2  0xffffffff8057c856 in driver_probe_device ()
#3  0xffffffff8057c9ba in __driver_attach ()
#4  0xffffffff8057aaa4 in bus_for_each_dev ()
#5  0xffffffff8057c3ea in driver_attach ()
#6  0xffffffff8057bc4a in bus_add_driver ()
#7  0xffffffff8057d75a in driver_register ()
#8  0xffffffff8057e83c in __platform_driver_register ()
#9  0xffffffff80a2455e in imsic_platform_driver_init ()
#10 0xffffffff8000212c in do_one_initcall ()
#11 0xffffffff80a01188 in kernel_init_freeable ()
#12 0xffffffff80928d80 in kernel_init ()

According to this result, the source to call really_probe is
do_one_initcall(), regardless of whether it is APLIC or IMSIC. The
do_one_initcall() function follows the placed order of the
initialization functions in the __initcall6 section to invoke them.
The compile order determines the order of the __initcall6 section.
Therefore, I try to adjust the compile order to influence the probe
order between IMSIC and APLIC. Do I misunderstand something?


> >  obj-$(CONFIG_SIFIVE_PLIC)              += irq-sifive-plic.o
> >  obj-$(CONFIG_STARFIVE_JH8100_INTC)     += irq-starfive-jh8100-intc.o
> >  obj-$(CONFIG_IMX_IRQSTEER)             += irq-imx-irqsteer.o
> > --
> > 2.34.1
> >
>
> Regards,
> Anup

Re: [PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Thomas Gleixner]

On Mon, Aug 05 2024 at 10:43, Vincent Chen wrote:
> On Fri, Aug 2, 2024 at 7:03 PM Anup Patel <anup@brainfault.org> wrote:
>> Secondly, changing compilation order in Makefile to influence
>> the probe order will not help in any way.
>>
> I was confused here. If possible, hope you can help me clarify it.
> The following is the backtrace of really_porbe() dumped by GDB.
> #0  0xffffffff8092318a in really_probe ()
> #1  0xffffffff80923516 in __driver_probe_device.part.0 ()
> #2  0xffffffff8057c856 in driver_probe_device ()
> #3  0xffffffff8057c9ba in __driver_attach ()
> #4  0xffffffff8057aaa4 in bus_for_each_dev ()
> #5  0xffffffff8057c3ea in driver_attach ()
> #6  0xffffffff8057bc4a in bus_add_driver ()
> #7  0xffffffff8057d75a in driver_register ()
> #8  0xffffffff8057e83c in __platform_driver_register ()
> #9  0xffffffff80a2455e in imsic_platform_driver_init ()
> #10 0xffffffff8000212c in do_one_initcall ()
> #11 0xffffffff80a01188 in kernel_init_freeable ()
> #12 0xffffffff80928d80 in kernel_init ()
>
> According to this result, the source to call really_probe is
> do_one_initcall(), regardless of whether it is APLIC or IMSIC. The
> do_one_initcall() function follows the placed order of the
> initialization functions in the __initcall6 section to invoke them.
> The compile order determines the order of the __initcall6 section.
> Therefore, I try to adjust the compile order to influence the probe
> order between IMSIC and APLIC. Do I misunderstand something?

There is no guarantee that this order is retained. The linker can freely
reorg the section. That's why we have deferred probing. It's neither a
bug nor a problem, so what are you trying to solve?

Thanks,

        tglx

Re: [PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Vincent Chen]

On Mon, Aug 5, 2024 at 4:08 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Mon, Aug 05 2024 at 10:43, Vincent Chen wrote:
> > On Fri, Aug 2, 2024 at 7:03 PM Anup Patel <anup@brainfault.org> wrote:
> >> Secondly, changing compilation order in Makefile to influence
> >> the probe order will not help in any way.
> >>
> > I was confused here. If possible, hope you can help me clarify it.
> > The following is the backtrace of really_porbe() dumped by GDB.
> > #0  0xffffffff8092318a in really_probe ()
> > #1  0xffffffff80923516 in __driver_probe_device.part.0 ()
> > #2  0xffffffff8057c856 in driver_probe_device ()
> > #3  0xffffffff8057c9ba in __driver_attach ()
> > #4  0xffffffff8057aaa4 in bus_for_each_dev ()
> > #5  0xffffffff8057c3ea in driver_attach ()
> > #6  0xffffffff8057bc4a in bus_add_driver ()
> > #7  0xffffffff8057d75a in driver_register ()
> > #8  0xffffffff8057e83c in __platform_driver_register ()
> > #9  0xffffffff80a2455e in imsic_platform_driver_init ()
> > #10 0xffffffff8000212c in do_one_initcall ()
> > #11 0xffffffff80a01188 in kernel_init_freeable ()
> > #12 0xffffffff80928d80 in kernel_init ()
> >
> > According to this result, the source to call really_probe is
> > do_one_initcall(), regardless of whether it is APLIC or IMSIC. The
> > do_one_initcall() function follows the placed order of the
> > initialization functions in the __initcall6 section to invoke them.
> > The compile order determines the order of the __initcall6 section.
> > Therefore, I try to adjust the compile order to influence the probe
> > order between IMSIC and APLIC. Do I misunderstand something?
>
> There is no guarantee that this order is retained. The linker can freely
> reorg the section. That's why we have deferred probing. It's neither a
> bug nor a problem, so what are you trying to solve?
>

Hi Thomas,
I understand now. I didn't realize that the linker could freely
reorganize this section. This patch won’t actually adjust the probe
order. Thank you very much for the explanation

Regards,
Vincent

> Thanks,
>
>         tglx

Re: [PATCH] irqchip: let the probe of APLIC be earlier than IMSIC

[Jessica Clarke]

On 6 Aug 2024, at 02:56, Vincent Chen <vincent.chen@sifive.com> wrote:
> 
> On Mon, Aug 5, 2024 at 4:08 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> 
>> On Mon, Aug 05 2024 at 10:43, Vincent Chen wrote:
>>> On Fri, Aug 2, 2024 at 7:03 PM Anup Patel <anup@brainfault.org> wrote:
>>>> Secondly, changing compilation order in Makefile to influence
>>>> the probe order will not help in any way.
>>>> 
>>> I was confused here. If possible, hope you can help me clarify it.
>>> The following is the backtrace of really_porbe() dumped by GDB.
>>> #0  0xffffffff8092318a in really_probe ()
>>> #1  0xffffffff80923516 in __driver_probe_device.part.0 ()
>>> #2  0xffffffff8057c856 in driver_probe_device ()
>>> #3  0xffffffff8057c9ba in __driver_attach ()
>>> #4  0xffffffff8057aaa4 in bus_for_each_dev ()
>>> #5  0xffffffff8057c3ea in driver_attach ()
>>> #6  0xffffffff8057bc4a in bus_add_driver ()
>>> #7  0xffffffff8057d75a in driver_register ()
>>> #8  0xffffffff8057e83c in __platform_driver_register ()
>>> #9  0xffffffff80a2455e in imsic_platform_driver_init ()
>>> #10 0xffffffff8000212c in do_one_initcall ()
>>> #11 0xffffffff80a01188 in kernel_init_freeable ()
>>> #12 0xffffffff80928d80 in kernel_init ()
>>> 
>>> According to this result, the source to call really_probe is
>>> do_one_initcall(), regardless of whether it is APLIC or IMSIC. The
>>> do_one_initcall() function follows the placed order of the
>>> initialization functions in the __initcall6 section to invoke them.
>>> The compile order determines the order of the __initcall6 section.
>>> Therefore, I try to adjust the compile order to influence the probe
>>> order between IMSIC and APLIC. Do I misunderstand something?
>> 
>> There is no guarantee that this order is retained. The linker can freely
>> reorg the section. That's why we have deferred probing. It's neither a
>> bug nor a problem, so what are you trying to solve?
>> 
> 
> Hi Thomas,
> I understand now. I didn't realize that the linker could freely
> reorganize this section. This patch won’t actually adjust the probe
> order. Thank you very much for the explanation

Also FYI your patch subject is backwards, which initially confused me.

Jess