Hi Chris,
On 17/07/2022 22:58, Chris Healy wrote:
[...]
[...]
+ goto err2;
+ }
+
+ irq_name = of_get_property(node, "interrupt-names", NULL);
+ if (!irq_name)
+ irq_name = "ddr_pmu";
That's not how the "interrupt-names" property works. If you only have
a single interrupt then there's not much need for it to be named in
the DT at all. If you do want to use named interrupts then use
platform_get_irq_byname(), and the name should probably have a bnit
more functional meaning. Either way, please don't abuse the DT like this.
Okay, actually there will be multiple interrupts , but not in current
G12 series.
That's fair enough, so we should try to anticipate it in the design of
the DT binding. If for instance future SoCs are going to move from
having a single combined overflow interrupt to a separate interrupt for
each counter, then the driver can reasonably continue to get them by
index and we'll effectively only need to update maxItems in the binding.
If on the other hand there's still going to be one combined overflow
interrupt, plus some other new interrupt for something completely
different, then it *could* be more appropriate to have names, and thus
to define and use a standard "overflow" name from the beginning even
when it is the only one present, so that we can remain consistent later
once more are added.
My assumption is that the goal of having this "interrupt-names" in DT
is to cover future cases where there is more than one DRAM controller
instance in the SoC and you want to be able to discriminate between
the two instances with this driver's interrupt name. If this is true,
as an alternative, you could do something like this:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/i2c/busses/i2c-mt65xx.c?h=v5.19-rc6&id=7fb9dc8109bf9713ffcda65617249099a1942f0f
This should result in each instance having a unique name that includes
the base address as the prefix to the interrupt name which should be
sufficient for determining which instance is which.
It's ok to introduce interrupt-names in the bindings for newer SoCs,
since it's useless for the current ones, there's no need to introduce it right now.
It's also why it's simpler to introduce a compatible per SoC, so we can add
different attributes in the bindings depending on the compatible.
Neil
[...]
