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

On Mon, 28 Sep 2026 18:15:37 +0200
Lorenzo Pieralisi [off-list ref] wrote:

On Fri, Sep 25, 2026 at 01:54:26PM +0300, Andy Shevchenko wrote:

quoted

On Fri, Sep 25, 2026 at 12:30:07PM +0200, Lorenzo Pieralisi wrote:  

quoted

On Fri, Sep 25, 2026 at 12:49:54PM +0300, Andy Shevchenko wrote:  

quoted

On Fri, Sep 25, 2026 at 09:48:01AM +0200, Lorenzo Pieralisi wrote:  

...
  

quoted

quoted

quoted

+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];  

Use standard pattern, id est

	if (ret)
		...
  

quoted

+	kfree(values);  

You want to use __free()
  

quoted

+	return ret;
+}  

I believe the whole approach is suboptimal, if you wish get indexed value (but why?)  

Why what (that's what the irq_get() interface requires ?) I agree it is
suboptimal - the whole point of the series is an RFC on using properties
to store GSI number/flags, then how to read them we will optimize it
when/if we agree that's the approach to be taken.  

Why to have indexed APIs. The callers usually do not want a single item from an
array, they want all of them or a big pile (exception is the array of strings,
but we have matching functions for that).

As per approach, this patch is against device property and I don't think we are
going to agree on the approach taken in *this* patch.  

Coming back to this, what is needed here is a way to stash the GSI number,
flags and name in stardard storage (like OF nodes IRQ properties, ACPI objects
_CRS, etc).

To give you some background, I thought about adding an array of:

GSI [number, flags, name]

to the ACPI static fwnode itself. Then basically the irq_get() API has got
all the information it needs from the primary fwnode; while doing that I
thought that this might have been a bit like reinventing software nodes
(granted, software node properties are ways more general that what I suggest
above and I don't expect any other property to be ever added to static fwnodes)
that's why I put together the series as it is, precisely to understand what's
best.

When you say "against device property", what do you mean precisely ?

I suppose you mean that this approach goes against software node properties
usage guidelines ?

For what it's worth I like it.  Clean (ish) solution for a messy problem.
May well be some details that can be done differently though.

Just to provide a counter point to Andy :)

Jonathan

Anyway, again, it is just to provide some context, we are talking about
a bunch of platform devices, it is nice to have a generic solution but
it has to be simple enough too given its scope, I understand very well
your concern with this approach as-is - it is there to find a way
forward together and show what the problem is.

Thanks,
Lorenzo

quoted

quoted

quoted

it needs to be retrieved as that in the guts of ACPI. Allocating memory for the whole
array to retrieve a single element is simply wrong.  

...
  

quoted

quoted

quoted

+#define ACPI_IRQ_PROP_GSI		"linux,acpi-gsi"
+#define ACPI_IRQ_PROP_GSI_TRIGGER	"linux,acpi-gsi-trigger"
+#define ACPI_IRQ_PROP_GSI_POLARITY	"linux,acpi-gsi-polarity"  

Oh... This sounds like a big ugly hack.  

That does not help much I am afraid.

What's a ugly hack ? Property names ? Using properties for this purpose ?  

Yes, properties started with "linux," for some core functionality.
Yes, using properties for this also doesn't sound right. I think the problem
here is in the table specifications or somewhere that deep. That's why we
ended up in this series.
  

quoted

Again, it is an RFC for this specific reason and I mentioned that in the
cover letter, thank you for your inputs.  

And here we have a discussion started :-)

P.S. Looks like I was too quick to jump into this thread. I will wait for
others to share their view on all this.

-- 
With Best Regards,
Andy Shevchenko