Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230613153415.350528-1-apatel@ventanamicro.com>
 <20230613153415.350528-9-apatel@ventanamicro.com> <CAGETcx-hoPNanAwe4++6roqBXwSdc7z6Ei=-r9z6qFG1U7xVXg@mail.gmail.com>
 <CAAhSdy3WH_nrKC-3AC4RHn9=crh6mPzaKO8jmAeo2j8-VJnT5g@mail.gmail.com>
 <CAGETcx_DkuyZ+c50QmA7eu8vUByfVosABugZpd7R8NgR8Wc-FA@mail.gmail.com>
 <CAK9=C2X4+aTs=+WdE0ba1qPDu67TiAKgE2Rx-qqD5+TJwYCKOQ@mail.gmail.com>
 <CAGETcx_KJURkxEuZOgPZR=fA4CaHnAwwRfU_JLJ28absFWAtzg@mail.gmail.com>
 <CAK9=C2XNAXaorxeNAB02o3_mYe3fsDSHS7rj=5+=q=d67Od60A@mail.gmail.com> <86mt0qbaxg.wl-maz@kernel.org>
In-Reply-To: <86mt0qbaxg.wl-maz@kernel.org>
From:   Anup Patel <anup@brainfault.org>
Date:   Fri, 23 Jun 2023 19:22:15 +0530
Message-ID: <CAAhSdy3eW_urQ_716DdBp8SN2zFN3oMMOt695cW4EQbejraVhw@mail.gmail.com>
Subject: Re: [PATCH v4 08/10] irqchip: Add RISC-V advanced PLIC driver
To:     Marc Zyngier <maz@kernel.org>
Cc:     Anup Patel <apatel@ventanamicro.com>,
        Saravana Kannan <saravanak@google.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        Atish Patra <atishp@atishpatra.org>,
        Andrew Jones <ajones@ventanamicro.com>,
        Conor Dooley <conor@kernel.org>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, iommu@lists.linux.dev,
        Android Kernel Team <kernel-team@android.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jun 23, 2023 at 6:19=E2=80=AFPM Marc Zyngier <maz@kernel.org> wrote=
:
>
> [here, let me trim all of this nonsense...]
>
> On Fri, 23 Jun 2023 12:47:00 +0100,
> Anup Patel <apatel@ventanamicro.com> wrote:
> > > No. My previous email asking you to NOT use IRQCHIP_DECLARE() and
> > > instead use IRQCHIP_PLATFORM_DRIVER_BEGIN/END() macros.
> >
> > I tried IRQCHIP_PLATFORM_DRIVER_BEGIN/END() macros but these
> > macros are not suitable for APLIC driver because we need platform devic=
e
> > pointer in the APLIC probe() to create platform MSI device domain (refe=
r,
> > platform_msi_create_device_domain()).
>
> Oh come on. How hard have you tried? Have you even looked at the other
> drivers in the tree to see how they solve this insurmountable problem
> with a *single* line of code?
>
>         pdev =3D of_find_device_by_node(node);
>
> That's it.

Please see the below diff. I tried the same thing but still the APLIC does
not get probed without the FWNODE_FLAG_BEST_EFFORT flag. Please
note that the current APLIC driver works unmodified for both DT and ACPI
but using of_find_device_by_node() here breaks ACPI support.

diff --git a/drivers/irqchip/irq-riscv-aplic.c
b/drivers/irqchip/irq-riscv-aplic.c
index 1e710fdf5608..9ae9e7fb905f 100644
--- a/drivers/irqchip/irq-riscv-aplic.c
+++ b/drivers/irqchip/irq-riscv-aplic.c
@@ -17,6 +17,7 @@
 #include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/msi.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/smp.h>

@@ -730,36 +731,12 @@ static int aplic_probe(struct platform_device *pdev)
     return 0;
 }

-static const struct of_device_id aplic_match[] =3D {
-    { .compatible =3D "riscv,aplic" },
-    {}
-};
-
-static struct platform_driver aplic_driver =3D {
-    .driver =3D {
-        .name        =3D "riscv-aplic",
-        .of_match_table    =3D aplic_match,
-    },
-    .probe =3D aplic_probe,
-};
-builtin_platform_driver(aplic_driver);
-
-static int __init aplic_dt_init(struct device_node *node,
+static int __init aplic_of_init(struct device_node *dn,
                 struct device_node *parent)
 {
-    /*
-     * The APLIC platform driver needs to be probed early
-     * so for device tree:
-     *
-     * 1) Set the FWNODE_FLAG_BEST_EFFORT flag in fwnode which
-     *    provides a hint to the device driver core to probe the
-     *    platform driver early.
-     * 2) Clear the OF_POPULATED flag in device_node because
-     *    of_irq_init() sets it which prevents creation of
-     *    platform device.
-     */
-    node->fwnode.flags |=3D FWNODE_FLAG_BEST_EFFORT;
-    of_node_clear_flag(node, OF_POPULATED);
-    return 0;
+    return aplic_probe(of_find_device_by_node(dn));
 }
-IRQCHIP_DECLARE(riscv_aplic, "riscv,aplic", aplic_dt_init);
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(aplic)
+IRQCHIP_MATCH("riscv,aplic", aplic_of_init)
+IRQCHIP_PLATFORM_DRIVER_END(aplic)

>
> > Further, I tried setting the "suppress_bind_attrs" flag in "struct
> > platform_driver aplic_driver" just like the
> > IRQCHIP_PLATFORM_DRIVER_END() macro but this did not work.
>
> I'm not sure how relevant this is to the conversation.

It's relevant because the only difference in the platform_driver
registered by IRQCHIP_PLATFORM_DRIVER_END() and
"struct platform_driver aplic_driver" is the "suppress_bind_attrs" flag.

Unfortunately, setting the "suppress_bind_attrs" flag does not
help as well.

>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

Regards,
Anup