> You've missed a bunch of people/lists. Use get_maintainers.pl.
Sorry, indeed, did not think about about PCI...
> Can you provide some details on what these nodes look like. The
> interrupt provider to an SoC device is a PCI device? That's weird...
The DTO looks like this:
watchdog {
...
reg = <0x00002064 0x00000028>;
...
interrupt-parent = <&gpio_17_0>;
interrupts = <4 0x8>; // 8 -> IRQ_TYPE_LEVEL_LOW
...
};
And the base DT:
ecam0: pci@878000000000 {
...
#size-cells = <2>;
#address-cells = <3>;
...
gpio_17_0: gpio0@17,0 {
...
reg = <0x8800 0 0 0 0>; /* DEVFN = 0x88 (17:0) */
...
};
...
};
I completely agree it's a bit sketchy, but it's not me who came up with
this ;-) Nevertheless I think other people might run into this issue of
mismatching address sizes as well when no interrupt mapping was intended.
> Note that of_irq_parse_raw() was refactored in 6.10, so your patch is
> not going to apply.
I'm aware of that and have adapted the patch accordingly.
> That's not the right information to parse the address correctly. You
> would need the device's #address-cells. However, in most cases we
> don't really need to parse the address. The address is not really used
> except in cases where interrupt routing matches the bus and so there
> is only one size. That's effectively only PCI devices today. In that
> case, the address size would always be equal, and the code implicitly
> assumes that. It would be better if we could just detect when to use
> the address or not. I think we'd have to look at 'interrupt-map-mask'
> first to see whether or not to read the address cells. Or maybe we
> could detect when the interrupt parent is the device's parent node.
> Either way, this code is tricky and hard to change without breaking
> something.
Thanks for confirming that it's PCI only and no address size mismatch
should occur. I also was thinking in the direction of checking first if
interrupt mapping is intended and return early otherwise, but was
worried to break something along the way...
> A simpler way to fix this is just always pass in an address buffer of
> 3 cells to of_irq_parse_raw. You would just need to copy the cells in
> of_irq_parse_one() to a temporary buffer.
That indeed sounds like the easiest solution to me; I'll send a new patch
shortly. However I don't understand how you came up with an address
buffer size of 3 - shouldn't it be MAX_PHANDLE_ARGS
(addrsize = MAX_PHANDLE_ARGS and intsize = 0 in the worst case)?
> Please don't send new versions right after the prior version. Give
> people a chance to review.
Sorry about that, I wanted to fix checkpatch because I thought
patch-applied failed due to that, but that seems to be another
issue...
Kind regards,
Stefan
