On Mon, Mar 21, 2022 at 11:52:27AM +0000, Robin Murphy wrote: > On 2022-03-20 18:47, Rob Herring wrote: > > On Fri, Mar 18, 2022 at 02:55:08AM +0530, Kuldeep Singh wrote: > > > On Thu, Mar 17, 2022 at 08:25:12PM +0000, Robin Murphy wrote: > > > > On 2022-03-17 19:15, Kuldeep Singh wrote: > > > > > Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in > > > > > conjugation with "arm,armv7-timer". Since, initial entry is not > > > > > documented, it start raising dtbs_check warnings. > > > > > > > > > > ['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long > > > > > 'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer'] > > > > > 'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer'] > > > > > > > > > > Document this compatible to address it. The motivation to add this > > > > > change is taken from an already existing entry "arm,cortex-a15-timer". > > > > > Please note, this will not hurt any arch timer users. > > > > > > > > Eh, if it's never been documented or supported, I say just get rid of it. > > > > The arch timer interface is by definition part of a CPU, and we can tell > > > > what the CPU is by reading its ID registers. Indeed that's how the driver > > > > handles the non-zero number of CPU-specific errata that already exist - we > > > > don't need compatibles for that. > > > > > > > > In some ways it might have been nice to have *SoC-specific* compatibles > > > > given the difficulty some integrators seem to have had in wiring up a stable > > > > count *to* the interface, but it's not like they could be magically added to > > > > already-deployed DTs after a bug is discovered, and nor could we have > > > > mandated them from day 1 just in case and subsequently maintained a binding > > > > that is just an ever-growing list of every SoC. Oh well. > > > > > > Robin, A similar discussion was already done on v1 thread. Please see > > > below for details: > > > https://lore.kernel.org/linux-devicetree/20220317065925.GA9158@9a2d8922b8f1/ > > > https://lore.kernel.org/linux-devicetree/726bde76-d792-febf-d364-6eedeb748c3b@xxxxxxxxxxxxx/ > > > > > > And final outcome of discussion turns out to add this compatible string. > > > > I agree with Robin on dropping. More specific here is not useful. If > > we're going to add some cores, then we should add every core > > implementation. Sure Rob, I will drop A7/15-timer entry from compatibles. This means only two entries i.e arm,armv7/8-timer will be there under compatibles now. I actually added A7-timer because A15-timer was already present in binding. Since, it was added by you that's why I added this one. I will update compatibles accordingly as you said above. > > Yeah, what I was trying to convey is that a compatible like > "arm,cortex-a76-timer" has the problem of being both too specific *and* not > specific enough to be genuinely useful *for the particular case of the arch > timer*. It's an architectural interface, where the actual functional > features are described through the interface itself, so the purpose of the > DT entry is really just to indicate that the standard interface is present > and describe how its externally-routed interrupts are wired up. > > However, it's also true that implementations of standard functionality > sometimes have bugs that software needs to know about, but in order for > specific DT compatibles to be useful in that respect they really need to > identify the *exact* implementation, e.g. to know that > "arm,cortex-a76-r0p0-timer" has a bug which needs working around, but > "arm,cortex-a76-r4p0-timer" does not. There might be cases where every known > version of a CPU is equally affected (e.g. Cortex-A73), but it doesn't hold > as a general assumption. Furthermore as mentioned, the other class of bugs > which affect this interface are not in the CPU's implementation of the > interface at all, but in the external SoC logic that provides the counter > value, and therefore it can be identification of the overall SoC that > matters regardless of which CPU IP(s) may be present. > > If we'd had the benefit of 10 years worth of hindsight 10 years ago, we > probably wouldn't have defined "arm,cortex-a15-timer" either. However the > fact that we can't erase the legacy of that decision doesn't make an > argument for repeating it now. > > > If one has a big.LITTLE system with A15/A7 what would be the right > > compatible value? > > > > > > > > I see people with different set of perspective in regard to whether keep > > > compatible string or not. We should have some sort of evidences to > > > support claims so that next time when similar situation arises, we'll be > > > aware beforehand how to proceed. > > > > Every situation tends to be different. > > Indeed, I certainly don't have a personal perspective of "delete all the > bindings!" in general - only when they're truly redundant (functionally, any > driver that can touch the arch timer registers can also read the CPU ID > registers, but even in the DT there should already be compatibles for the > CPUs themselves). Thanks Robin for providing inputs. I agree with your opinion of having soc specific compatibles which is also mentioned under dos and dont's of bindings and other cases will require investigation though. https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html - Kuldeep
