From: Marc Zyngier <maz@kernel.org> To: Kuldeep Singh <singh.kuldeep87k@gmail.com> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>, Marc Zyngier <marc.zyngier@arm.com>, Mark Rutland <mark.rutland@arm.com>, Daniel Lezcano <daniel.lezcano@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list Date: Wed, 16 Mar 2022 18:43:15 +0000 [thread overview] Message-ID: <87zglpybzw.wl-maz@kernel.org> (raw) In-Reply-To: <20220316174108.GB21737@9a2d8922b8f1> On Wed, 16 Mar 2022 17:41:08 +0000, Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote: > > On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote: > > On 16/03/2022 10:54, Kuldeep Singh wrote: > > > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using > > > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with > > > arm,armv7-timer which are not currently defined in driver file. Add > > > these entries in arch_timer_of_match list to bring them in use. > > > > > > > This looks wrong (also Marc pointed this out) and rationale is not > > sufficient. Why do you need these compatibles in the driver? > > Hi Krzysztof and Marc, > > I find myself in trouble whenever dealing with compatible entries and > had 2 options when I stumble this issue. > 1. Remove unused compatible That'd be silly. > 2. Add required compatible to binding and driver To the binding, yes. But to the driver? > My past experience and advise from other developer says not to remove an > existing compatible. And also I found "arm,cortex-a15-timer" in binding > which was again not documented and was present in DT. This prompted me > to go for second option and make necessary additions in binding and > driver following current entries. The "arm,cortex-a15-timer" compatible is documentation, and only that. If, one day, we find a bug in this implementation, we could work around it in the driver thanks to the separate compatible (although in this case, we'd have much better way of doing that). > As per your perspective, current configuration isn't apt which means > "arm,cortex-a15-timer" is a stub and is wrongly present in binding. That's not what I said. This compatible string is perfectly fine, and accurately describe the HW. The driver doesn't need to know about the fine details of the implementation, and is perfectly happy with the current state of things. Think of it as an instance of a class. The driver doesn't need to know the instance, only that it is a certain class. > I also observed many other DTs have compatibles which are not present in > driver. What is an ideal idealogy behind such cases? I think I've made myself clear above. Thanks, M. -- Without deviation from the norm, progress is not possible.
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org> To: Kuldeep Singh <singh.kuldeep87k@gmail.com> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>, Marc Zyngier <marc.zyngier@arm.com>, Mark Rutland <mark.rutland@arm.com>, Daniel Lezcano <daniel.lezcano@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer in of_match list Date: Wed, 16 Mar 2022 18:43:15 +0000 [thread overview] Message-ID: <87zglpybzw.wl-maz@kernel.org> (raw) In-Reply-To: <20220316174108.GB21737@9a2d8922b8f1> On Wed, 16 Mar 2022 17:41:08 +0000, Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote: > > On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote: > > On 16/03/2022 10:54, Kuldeep Singh wrote: > > > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using > > > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with > > > arm,armv7-timer which are not currently defined in driver file. Add > > > these entries in arch_timer_of_match list to bring them in use. > > > > > > > This looks wrong (also Marc pointed this out) and rationale is not > > sufficient. Why do you need these compatibles in the driver? > > Hi Krzysztof and Marc, > > I find myself in trouble whenever dealing with compatible entries and > had 2 options when I stumble this issue. > 1. Remove unused compatible That'd be silly. > 2. Add required compatible to binding and driver To the binding, yes. But to the driver? > My past experience and advise from other developer says not to remove an > existing compatible. And also I found "arm,cortex-a15-timer" in binding > which was again not documented and was present in DT. This prompted me > to go for second option and make necessary additions in binding and > driver following current entries. The "arm,cortex-a15-timer" compatible is documentation, and only that. If, one day, we find a bug in this implementation, we could work around it in the driver thanks to the separate compatible (although in this case, we'd have much better way of doing that). > As per your perspective, current configuration isn't apt which means > "arm,cortex-a15-timer" is a stub and is wrongly present in binding. That's not what I said. This compatible string is perfectly fine, and accurately describe the HW. The driver doesn't need to know about the fine details of the implementation, and is perfectly happy with the current state of things. Think of it as an instance of a class. The driver doesn't need to know the instance, only that it is a certain class. > I also observed many other DTs have compatibles which are not present in > driver. What is an ideal idealogy behind such cases? I think I've made myself clear above. Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-03-16 18:43 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-16 9:54 [PATCH 0/3] Fix dtbs warnings for arch timer Kuldeep Singh 2022-03-16 9:54 ` Kuldeep Singh 2022-03-16 9:54 ` [PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of " Kuldeep Singh 2022-03-16 9:54 ` Kuldeep Singh 2022-03-16 16:29 ` Krzysztof Kozlowski 2022-03-16 16:29 ` Krzysztof Kozlowski 2022-03-17 7:02 ` Kuldeep Singh 2022-03-17 7:02 ` Kuldeep Singh 2022-03-16 9:54 ` [PATCH 2/3] dt-bindings: timer: Document arm,cortex-a7-timer for " Kuldeep Singh 2022-03-16 9:54 ` [PATCH 2/3] dt-bindings: timer: Document arm, cortex-a7-timer " Kuldeep Singh 2022-03-16 16:28 ` [PATCH 2/3] dt-bindings: timer: Document arm,cortex-a7-timer " Krzysztof Kozlowski 2022-03-16 16:28 ` [PATCH 2/3] dt-bindings: timer: Document arm, cortex-a7-timer " Krzysztof Kozlowski 2022-03-16 17:43 ` [PATCH 2/3] dt-bindings: timer: Document arm,cortex-a7-timer " Kuldeep Singh 2022-03-16 17:43 ` [PATCH 2/3] dt-bindings: timer: Document arm, cortex-a7-timer " Kuldeep Singh 2022-03-16 9:54 ` [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list Kuldeep Singh 2022-03-16 9:54 ` [PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer " Kuldeep Singh 2022-03-16 10:05 ` [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer " Marc Zyngier 2022-03-16 10:05 ` [PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer " Marc Zyngier 2022-03-16 16:30 ` [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer " Krzysztof Kozlowski 2022-03-16 16:30 ` Krzysztof Kozlowski 2022-03-16 17:41 ` Kuldeep Singh 2022-03-16 17:41 ` Kuldeep Singh 2022-03-16 18:43 ` Marc Zyngier [this message] 2022-03-16 18:43 ` [PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer " Marc Zyngier 2022-03-17 6:59 ` [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer " Kuldeep Singh 2022-03-17 6:59 ` Kuldeep Singh 2022-03-17 7:18 ` Krzysztof Kozlowski 2022-03-17 7:18 ` Krzysztof Kozlowski 2022-03-16 11:27 ` [PATCH 0/3] Fix dtbs warnings for arch timer Marc Zyngier 2022-03-16 11:27 ` Marc Zyngier 2022-03-16 17:20 ` Kuldeep Singh 2022-03-16 17:20 ` Kuldeep Singh 2022-03-16 18:47 ` Marc Zyngier 2022-03-16 18:47 ` Marc Zyngier 2022-03-23 19:38 ` Rob Herring 2022-03-23 19:38 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87zglpybzw.wl-maz@kernel.org \ --to=maz@kernel.org \ --cc=daniel.lezcano@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=krzysztof.kozlowski@canonical.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=mark.rutland@arm.com \ --cc=singh.kuldeep87k@gmail.com \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.