Am 22.06.24 um 19:46 schrieb Arend Van Spriel:
On June 22, 2024 4:07:40 PM Alex Bee <knaerzche@xxxxxxxxx> wrote:
Am 22.06.24 um 15:49 schrieb Arend Van Spriel:
On June 22, 2024 3:38:32 PM Arend Van Spriel
<arend.vanspriel@xxxxxxxxxxxx> wrote:
On June 22, 2024 12:56:02 AM Alex Bee <knaerzche@xxxxxxxxx> wrote:
This "new" version of defining external interrupts is around for a
very
long time now and supported and preferred by irq_of_parse_and_map
respectively of_irq_parse_one.
Support it in brcmfmac as well by checking if either "interrupts" or
"interrupts-extended" property exists as indication if
irq_of_parse_and_map
should be called.
All very interesting, but why should we add code for something that
is not
specified in the bindings documentation?
NAK (for now). Feel free to update the bindings document.
Sure, if you insist on it, I can update the bindings document. So far
not
many (no) users did that, as it is clearly specified as an
alternative in
devicetree/bindings/interrupt-controller/interrupts.txt (sure, it's
not yet
converted to yaml yet).
Right. So in my opinion that should be handled by the interrupt
subsystem. Hence I dived into irq_of_parse_and_map(). I would suggest
to open code that:
And as you can see: It's already handled by the interrupt subsystem - all
what prevents it from working in it's intended behavior is this strange
of_property_present check.
/* make sure there are interrupts defined in the node */
- if (!of_property_present(np, "interrupts"))
+ if (of_irq_parse_one(...))
return;
Agreed. That's even better - I also didn't fully understand why this
of_property_present was chosen in the first place. Actually I wanted to
send something similar first with only calling of_irq_parse_one and return
early if it fails, but the result doesn't allow to differentiate between
"no interrupt defined" and "interrupt mapping failed" - so open coding it
seems required, unfortunately..
irq = irq_create_of_mapping(...);
Which is a much nicer form, imho.
And by the way for instance
arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts already uses it that
way
and the interrupt will currently not picked up (at least not by this
driver).
I expected the "nicer" argument would be thrown in at some point.
Esthetics are never a technical argument, but let's not debate that
;-) Hopefully you can agree with my suggestion.
I wouldn't want the 'nicer'-argument to be an argument, as I don't like
that either: so sorry for that. My point was: Why wouldn’t we support it
in brcmfmac also?
So will resend with you suggestion applied: Remove !of_property_present
check completely and do the two steps of_irq_parse_one and
irq_create_of_mapping separately.
Alex
Regards,
Arend
