Re: [RFC PATCH v1 11/21] swnode: Add support to create early during boot

[Sunil V L <sunilvl@ventanamicro.com>]

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