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;
Date:   Wed, 9 Aug 2023 11:14:36 +0530
From:   Sunil V L <sunilvl@ventanamicro.com>
To:     Marc Zyngier <maz@kernel.org>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        linux-doc@vger.kernel.org, linux-riscv@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Len Brown <lenb@kernel.org>,
        Daniel Scally <djrscally@gmail.com>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Anup Patel <anup@brainfault.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Robert Moore <robert.moore@intel.com>,
        Haibo Xu <haibo1.xu@intel.com>,
        Andrew Jones <ajones@ventanamicro.com>,
        Conor Dooley <conor.dooley@microchip.com>,
        Atish Kumar Patra <atishp@rivosinc.com>,
        Anup Patel <apatel@ventanamicro.com>
Subject: Re: [RFC PATCH v1 11/21] swnode: Add support to create early during
 boot
Message-ID: <ZNMnxAVn6Vy337Eq@sunil-laptop>
References: <20230803175916.3174453-1-sunilvl@ventanamicro.com>
 <20230803175916.3174453-12-sunilvl@ventanamicro.com>
 <ZMyWDDD6Lw8REd1r@smile.fi.intel.com>
 <ZMyymUdV63g4eGaT@sunil-laptop>
 <865y5phdwd.wl-maz@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <865y5phdwd.wl-maz@kernel.org>
Precedence: bulk

On Tue, Aug 08, 2023 at 02:17:22PM +0100, Marc Zyngier wrote:
> On Fri, 04 Aug 2023 09:11:05 +0100,
> Sunil V L <sunilvl@ventanamicro.com> wrote:
> > 
> > Hi Andy,
> > 
> > On Fri, Aug 04, 2023 at 09:09:16AM +0300, Andy Shevchenko wrote:
> > > On Thu, Aug 03, 2023 at 11:29:06PM +0530, Sunil V L wrote:
> > > > From: Anup Patel <apatel@ventanamicro.com>
> > > > 
> > > > swnode framework can be used to create fwnode for interrupt
> > > > controllers.
> > > 
> > > Why? What is this for?
> > > Can you elaborate? This commit message is poorly written...
> > > 
> > > And why firmware node is not enough for ACPI case?
> > > I assume the fwnode in DT case is already provided by OF.
> > > 
> > Thanks a lot for the review!.
> > 
> > You are right, OF provides the fwnode for irqchip drivers. However, for
> > ACPI case, it is typically created using irq_domain_alloc_named_fwnode
> > or irq_domain_alloc_fwnode since these are not ACPI devices in the
> > namespace but from MADT. The fwnode created using
> > irq_domain_alloc_fwnode() is a simple one which doesn't support properties
> > similar to the one created by OF framework or software node framework.
> > Hence, lot of data from the MADT structures need to be cached as
> > separate structures in the drivers and also would need several ifdefs to
> > check for ACPI and some amount of code duplication is also required due
> > to the way DT driver gets the information vs ACPI.
> > 
> > The beauty of software node framework is, it supports adding properties
> > and also is a supported fwnode type in __irq_domain_create().
> 
> There is no beauty here. Only some extra bloat that we do not need.
> 
> DT and ACPI exposes very different attributes. One describe the HW,
> the other one describe an OS abstraction. Pretending that you can
> summon both into the same infrastructure is a fallacy. You'll just end
> up with the cross product of both infrastructure, and pollute the rest
> of the kernel with pointless cruft.
> 
Hi Marc,

Thank you very much for the feedback!. Sure, let me revert this approach
and do as you recommended in next version.

> > So, if we
> > can create the fwnode for these irqchip using software node, we can
> > attach the same properties and the actual irqchip driver which uses the
> > fwnode doesn't need to have any ACPI vs DT checks. Same driver will work
> > seamlessly on both DT and ACPI platforms.  But the challenge is,
> > currently swnode expects to be created with sysfs which won't be
> > available during early boot when irqchip drivers need to be probed. So,
> > adding support to create without dependency on sysfs help us to reuse
> > the same framework for irqchip use case also.
> 
> That's another fallacy.
> 
> Most irqchips *DO NOT* need to be probed early. Only the root
> irqchip. Given that this series is about *secondary* interrupt
> controllers, they absolutely don't need to be probed early.
>
Since we created swnode for root irqchip also in this approach, we had
to support early creation. With your feedback, this is no longer
required.

> To be clear: I do not intend to merge anything that:
> 
> - invents yet another way to "abstract" firmware interfaces
> 
> - adds more "early probe" hacks for non-primary interrupt controllers
> 
> I have already said that in response to Anup's AIA series, and this
> equally applies to this series.
>
In Anup's AIA v7 series, he has made non-primary controller drivers as
platform drivers which are not probed early.

Thanks,
Sunil