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:
/* make sure there are interrupts defined in the node */ - if (!of_property_present(np, "interrupts")) + if (of_irq_parse_one(...)) return; 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.
Regards, Arend
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
