Messages in this thread |  | | Date | Mon, 18 Jul 2022 10:06:29 +0200 | Subject | Re: [PATCH 1/4] perf/amlogic: Add support for Amlogic meson G12 SoC DDR PMU driver | From | Neil Armstrong <> |
| |
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
[...]
|  |