Re: [PATCH v3 04/26] platform: Add firmware-agnostic irq and affinity retrieval interface

[Marc Zyngier <maz@kernel.org>]

On Thu, 09 Oct 2025 18:03:51 +0100,
Jonathan Cameron <jonathan.cameron@huawei.com> wrote:
> 
> On Mon, 22 Sep 2025 09:28:11 +0100
> Marc Zyngier <maz@kernel.org> wrote:
> 
> > Expand platform_get_irq_optional() to also return an affinity if
> > available, renaming it to platform_get_irq_affinity() in the
> > process.
> > 
> > platform_get_irq_optional() is preserved with its current semantics
> > by calling into the new helper with a NULL affinity pointer.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> Maybe a breadcrumb of a comment for those of us who can't be bothered
> to figure out why this needs the ifndef CONFIG_SPARC?

The main issue is that SPARC, despite using OpenFirmware, does not use
the OF infrastructure (which is basically DT only). This means that
SPARC has its own firmware interface and parses interrupts its own
way, storing them as archdata in the device. Sad state of things,
unfortunately.

> Otherwise a question on whether it's worth spinning a fwnode.h handler
> to hide away the fwnode type in get_irq_affinity.
> I think not given the complexity already there for the platform device
> irq stuff, but thought I'd mention it.

I don't think it'd be worth the hassle at this stage. The platform
code is already a weird mix of DT and ACPI, without any clear
delineation.

If we wanted to do something useful, we'd split that into generic code
on one side (the actual Linux platform device code), and the firmware
specific backend. The main problem is to find a common abstraction,
and ISTR that people found that rather hard, hence the current state.

> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Thanks for that.

> > ---
> >  drivers/base/platform.c         | 60 +++++++++++++++++++++++++++------
> >  include/linux/platform_device.h |  2 ++
> >  2 files changed, 52 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 09450349cf323..3a058f63ef0d3 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -150,25 +150,37 @@ devm_platform_ioremap_resource_byname(struct platform_device *pdev,
> >  EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname);
> >  #endif /* CONFIG_HAS_IOMEM */
> >  
> > +static const struct cpumask *get_irq_affinity(struct platform_device *dev,
> > +					      unsigned int num)
> > +{
> > +	const struct cpumask *mask = NULL;
> > +#ifndef CONFIG_SPARC
> > +	struct fwnode_handle *fwnode = dev_fwnode(&dev->dev);
> > +
> > +	if (is_of_node(fwnode))
> > +		mask = of_irq_get_affinity(to_of_node(fwnode), num);
> > +	else if (is_acpi_device_node(fwnode))
> > +		mask = acpi_irq_get_affinity(ACPI_HANDLE_FWNODE(fwnode), num);
> 
> Not sure how useful it will be more generally, but maybe use fwnode.h and
> appropriate callback rather than opencoding here?
> 
> Mind you the extra handling in existing platform_get_irq_optional()
> for corner cases doesn't really fit with that model.

Indeed, and I find that fwnode.h is currently completely
FW-independent.  I'd rather keep it that way and not expose these
shenanigans outside of the support code *unless* we have a good reason
to do so.

Cheers,

	M.

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