Re: [PATCH RFC 02/11] ACPI: Introduce irq_get() for static fwnodes

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

On Mon, Sep 28, 2026 at 04:24:18PM -0700, Ashok Raj wrote:
> 
> On Fri, Sep 25, 2026 at 09:48:01AM +0200, Lorenzo Pieralisi wrote:
> > To describe and map GSIs for firmware nodes created out of ACPI static
> > table entries in a uniform way it is required to define some standard
> > properties and attach them to ACPI static fwnode as secondary nodes.
> > 
> > Define properties names to describe GSIs and their trigger-mode/polarity,
> > and implement an irq_get() callback for static fwnodes so that core code
> > can retrieve and map IRQs for ACPI static fwnodes in standard manner.
> > 
> > An empty stub for property_read_string_array() is also added, so that
> > the fwnode_irq_get_byname() interface falls back (through
> > fwnode_property_read_string_array()) to the secondary
> > fwnode to grab the "interrupt-names" property.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > Cc: Bartosz Golaszewski <brgl@kernel.org>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > ---
> >  drivers/acpi/property.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  include/linux/acpi.h    |  4 +++
> >  2 files changed, 72 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index 8ee5a1f0eb48..c609100c08db 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -1766,7 +1766,74 @@ static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode,
> >  
> >  DECLARE_ACPI_FWNODE_OPS(acpi_device_fwnode_ops);
> >  DECLARE_ACPI_FWNODE_OPS(acpi_data_fwnode_ops);
> > -const struct fwnode_operations acpi_static_fwnode_ops;
> > +
> > +static int acpi_static_fwnode_read_u32_prop_index(const struct fwnode_handle *fwnode,
> > +						  const char *propname,
> > +						  unsigned int index, u32 *value)
> > +{
> > +	u32 *values;
> > +	int ret, count;
> > +
> > +	count = fwnode_property_count_u32(fwnode, propname);
> > +	if (count < 0)
> > +		return count;
> > +
> > +	if (index >= count)
> > +		return -ENOENT;
> > +
> > +	values = kcalloc(count, sizeof(*values), GFP_KERNEL);
> > +	if (!values)
> > +		return -ENOMEM;
> > +
> > +	ret = fwnode_property_read_u32_array(fwnode, propname, values, count);
> > +	if (!ret)
> > +		*value = values[index];
> > +
> 
> You lookup with a propname, and then qualify with and index? is it
> possible to have the different index but same propname? 

What is the question :) ? It is to retrieve a property value at a specific
index.

> Alternately you can send the list to caller and they can use the ones
> they need?
> 
> 
> > +	kfree(values);
> > +	return ret;
> > +}
> > +
> > +static int acpi_static_fwnode_read_string_array(const struct fwnode_handle *fwnode,
> > +						const char *propname,
> > +						const char **val, size_t nval)
> > +{
> > +	/* Route string handling to secondary software nodes */
> > +	return -EINVAL;
> > +}
> > +
> > +static int acpi_static_fwnode_irq_get(const struct fwnode_handle *fwnode,
> > +				      unsigned int index)
> > +{
> > +	u32 gsi, trigger, polarity;
> > +	int ret;
> > +
> > +	if (!fwnode->secondary)
> > +		return -ENODEV;
> > +
> > +	fwnode = fwnode->secondary;
> > +
> > +	ret = acpi_static_fwnode_read_u32_prop_index(fwnode, ACPI_IRQ_PROP_GSI,
> > +						index, &gsi);
> > +	if (ret)
> > +		return ret == -ENOENT ? -ENXIO : ret;
> > +
> > +	ret = acpi_static_fwnode_read_u32_prop_index(fwnode, ACPI_IRQ_PROP_GSI_TRIGGER,
> > +						index, &trigger);
> > +	if (ret)
> > +		return ret == -ENOENT ? -ENXIO : ret;
> > +
> > +	ret = acpi_static_fwnode_read_u32_prop_index(fwnode, ACPI_IRQ_PROP_GSI_POLARITY,
> > +						index, &polarity);
> > +	if (ret)
> > +		return ret == -ENOENT ? -ENXIO : ret;
> 
> Consolidate return to one place? 

Yes it can (and trigger and polarity can be just one property "flags", FWIW),
if we agree that's what will do, which I doubt.

Thanks,
Lorenzo