From: Benoit Cousson <b-cousson@ti.com> To: "Mohammed, Afzal" <afzal@ti.com> Cc: Stephen Warren <swarren@wwwdotorg.org>, Jon Hunter <jgchunter@gmail.com>, Russell King <linux@arm.linux.org.uk>, "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>, "devicetree-discuss@lists.ozlabs.org" <devicetree-discuss@lists.ozlabs.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Rob Herring <rob.herring@calxeda.com>, Grant Likely <grant.likely@linaro.org>, Benoit Cousson <benoit.cousson@linaro.org>, "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v2 11/14] Documentation: dt: binding: omap: am43x timer Date: Wed, 29 May 2013 15:35:35 +0200 [thread overview] Message-ID: <51A60427.6010409@ti.com> (raw) In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93EAFB174@DBDE04.ent.ti.com> On 05/29/2013 11:58 AM, Mohammed, Afzal wrote: > Hi Benoit, > > On Wed, May 29, 2013 at 14:09:18, Cousson, Benoit wrote: >> On 05/29/2013 10:06 AM, Mohammed, Afzal wrote: >>> On Wed, May 29, 2013 at 03:35:10, Stephen Warren wrote: >>>> On 05/28/2013 03:25 PM, Jon Hunter wrote: > >>>>> If you are adding more compatibility strings, then this implies that the >>>>> AM43x timers are not 100% compatible with any other device listed (such >>>>> as am335x or any omap device). That's fine but you should state that in >>>>> the changelog. If the AM43x timer registers are 100% compatible with >>>>> existing devices you should not add these. >>>> >>>> I'm not sure that's true; .dts files should always include a compatible >>>> value that describes the most specific model of the HW, plus any >>>> baseline compatible value that the HW is compatible with. This allows >>>> any required quirks/fixes/... to be applied for the specific HW model >>>> later even if nobody knows right now they'll be needed. Hence, defining >>>> new compatible values doesn't necessarily mean incompatible HW. >>> >>> Stephen took words out of my finger ;) >>> >>> Some explanations,I don;t >>> >>> 1. first compatible should be exact device [A], followed by compatible >>> model (if one) >>> 2. Minor effort in getting DT right the first time may help prevent >>> difficult effort later modifying it (if a necessity comes), considering >>> the fact that DT sources has to move out of Kernel at some point of >>> time. And DT is not supposed to be modified, which may cause difficulty >>> for the users (I had been a minor victim of this during rebase). >>> >>> As we both were in GPMC land earlier, an example, >>> >>> If my memory is right, GPMC IP in am335x is rev 6, and IP has 8 chip >>> select, but one is not pinned out. Now assume that same IP is integrated >>> in another SoC (probably OMAP4 has rev 6). Here if we use same compatible >>> for both, driver cannot handle it properly (w/o knowledge about platform). >>> But if exact compatible is mentioned, without modifying DT (which should >>> be considered as a firmware) just by modifying Kernel, deciding based on >>> compatible would help achieve what is required. >> >> That's true for the DTS itself, but here your are changing the binding >> documentation which is supposed to reflect the driver "interface" in the >> Device Tree model description. >> >> Since the driver does not support any new compatible string, you should >> not update the binding. > > I have a different opinion: Binding documentation is to be considered an > entity that is not a part of the Kernel, but currently is a part of the > Kernel for want of a better place. And binding is to be considered OS > agnostic being ignorant of any piece of software (driver here). OK, that part is correct, but instead of driver I should have said HW device. The binding is supposed to identify a specific HW device revision or family if some HW registers are there to identify the version. > Binding has the authority to allow its usage in DT sources. And in this case, you do not introduce any new revision. There is no point to update the binding each time we add a new SoC variant that will contain the exact same IP. I think it will mainly confuse the user that will wonder what is different in that version compare to the previous one, moreover we can end up with hundred of entries for the exact same IP for nothing. The real problem is due to the introduction of the SoC name in the device compatible name. That does introduced a SoC level information that is mostly irrelevant at device level. I can understand why it was done for practical aspect when the IP version is not well identified, but that can lead to this proliferation of new pointless bindings. >> The driver and the binding will have to be changed the day you will have >> to update the driver to handle a bug / feature specific to that revision >> (ti,am4372-timer). >> >> Since this series does not seems to update the driver, you should not >> update the bindings. > > I believe that updating binding is a prerequisite for making a new > DTS change, hence binding was updated first, then DTS. I don't think so, as soon as you still use at least one documented compatible binding in the DTS description. It is not anyway really armless, it just adds much more confusion that real useful information for my point of view. Regards, Benoit
WARNING: multiple messages have this Message-ID (diff)
From: b-cousson@ti.com (Benoit Cousson) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 11/14] Documentation: dt: binding: omap: am43x timer Date: Wed, 29 May 2013 15:35:35 +0200 [thread overview] Message-ID: <51A60427.6010409@ti.com> (raw) In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93EAFB174@DBDE04.ent.ti.com> On 05/29/2013 11:58 AM, Mohammed, Afzal wrote: > Hi Benoit, > > On Wed, May 29, 2013 at 14:09:18, Cousson, Benoit wrote: >> On 05/29/2013 10:06 AM, Mohammed, Afzal wrote: >>> On Wed, May 29, 2013 at 03:35:10, Stephen Warren wrote: >>>> On 05/28/2013 03:25 PM, Jon Hunter wrote: > >>>>> If you are adding more compatibility strings, then this implies that the >>>>> AM43x timers are not 100% compatible with any other device listed (such >>>>> as am335x or any omap device). That's fine but you should state that in >>>>> the changelog. If the AM43x timer registers are 100% compatible with >>>>> existing devices you should not add these. >>>> >>>> I'm not sure that's true; .dts files should always include a compatible >>>> value that describes the most specific model of the HW, plus any >>>> baseline compatible value that the HW is compatible with. This allows >>>> any required quirks/fixes/... to be applied for the specific HW model >>>> later even if nobody knows right now they'll be needed. Hence, defining >>>> new compatible values doesn't necessarily mean incompatible HW. >>> >>> Stephen took words out of my finger ;) >>> >>> Some explanations,I don;t >>> >>> 1. first compatible should be exact device [A], followed by compatible >>> model (if one) >>> 2. Minor effort in getting DT right the first time may help prevent >>> difficult effort later modifying it (if a necessity comes), considering >>> the fact that DT sources has to move out of Kernel at some point of >>> time. And DT is not supposed to be modified, which may cause difficulty >>> for the users (I had been a minor victim of this during rebase). >>> >>> As we both were in GPMC land earlier, an example, >>> >>> If my memory is right, GPMC IP in am335x is rev 6, and IP has 8 chip >>> select, but one is not pinned out. Now assume that same IP is integrated >>> in another SoC (probably OMAP4 has rev 6). Here if we use same compatible >>> for both, driver cannot handle it properly (w/o knowledge about platform). >>> But if exact compatible is mentioned, without modifying DT (which should >>> be considered as a firmware) just by modifying Kernel, deciding based on >>> compatible would help achieve what is required. >> >> That's true for the DTS itself, but here your are changing the binding >> documentation which is supposed to reflect the driver "interface" in the >> Device Tree model description. >> >> Since the driver does not support any new compatible string, you should >> not update the binding. > > I have a different opinion: Binding documentation is to be considered an > entity that is not a part of the Kernel, but currently is a part of the > Kernel for want of a better place. And binding is to be considered OS > agnostic being ignorant of any piece of software (driver here). OK, that part is correct, but instead of driver I should have said HW device. The binding is supposed to identify a specific HW device revision or family if some HW registers are there to identify the version. > Binding has the authority to allow its usage in DT sources. And in this case, you do not introduce any new revision. There is no point to update the binding each time we add a new SoC variant that will contain the exact same IP. I think it will mainly confuse the user that will wonder what is different in that version compare to the previous one, moreover we can end up with hundred of entries for the exact same IP for nothing. The real problem is due to the introduction of the SoC name in the device compatible name. That does introduced a SoC level information that is mostly irrelevant at device level. I can understand why it was done for practical aspect when the IP version is not well identified, but that can lead to this proliferation of new pointless bindings. >> The driver and the binding will have to be changed the day you will have >> to update the driver to handle a bug / feature specific to that revision >> (ti,am4372-timer). >> >> Since this series does not seems to update the driver, you should not >> update the bindings. > > I believe that updating binding is a prerequisite for making a new > DTS change, hence binding was updated first, then DTS. I don't think so, as soon as you still use at least one documented compatible binding in the DTS description. It is not anyway really armless, it just adds much more confusion that real useful information for my point of view. Regards, Benoit
next prev parent reply other threads:[~2013-05-29 13:36 UTC|newest] Thread overview: 109+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-05-27 14:33 [PATCH v2 00/14] ARM: OMAP2+: AM43x initial support Afzal Mohammed 2013-05-27 14:33 ` Afzal Mohammed 2013-05-27 14:33 ` Afzal Mohammed 2013-05-27 14:35 ` [PATCH v2 01/14] ARM: OMAP2+: separate out OMAP4 restart Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:35 ` [PATCH v2 02/14] ARM: OMAP2+: AM43x: Kconfig Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-06-12 17:12 ` Tony Lindgren 2013-06-12 17:12 ` Tony Lindgren 2013-06-13 6:58 ` Mohammed, Afzal 2013-06-13 6:58 ` Mohammed, Afzal 2013-06-13 6:58 ` Mohammed, Afzal 2013-06-13 9:54 ` Tony Lindgren 2013-06-13 9:54 ` Tony Lindgren 2013-06-13 9:54 ` Tony Lindgren 2013-06-13 10:02 ` Mohammed, Afzal 2013-06-13 10:02 ` Mohammed, Afzal 2013-06-13 10:02 ` Mohammed, Afzal 2013-06-13 10:17 ` Tony Lindgren 2013-06-13 10:17 ` Tony Lindgren 2013-06-13 10:17 ` Tony Lindgren 2013-05-27 14:35 ` [PATCH v2 03/14] ARM: OMAP2+: AM43x: kbuild Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:35 ` [PATCH v2 04/14] ARM: OMAP2+: AM43x: soc_is support Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:35 ` Afzal Mohammed 2013-05-27 14:36 ` [PATCH v2 05/14] ARM: OMAP2+: AM437x: SoC revision detection Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` [PATCH v2 06/14] ARM: OMAP2+: AM43x: static mapping Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` [PATCH v2 07/14] ARM: OMAP2+: AM43x: early init Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-06-12 17:14 ` Tony Lindgren 2013-06-12 17:14 ` Tony Lindgren 2013-05-27 14:36 ` [PATCH v2 08/14] ARM: OMAP2+: AM43x: GP or HS ? Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` [PATCH v2 09/14] ARM: OMAP2+: AM43x: SRAM base and size Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` [PATCH v2 10/14] ARM: OMAP2+: AM43x: basic dt support Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:36 ` Afzal Mohammed 2013-05-27 14:37 ` [PATCH v2 11/14] Documentation: dt: binding: omap: am43x timer Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-28 21:25 ` Jon Hunter 2013-05-28 21:25 ` Jon Hunter 2013-05-28 22:05 ` Stephen Warren 2013-05-28 22:05 ` Stephen Warren 2013-05-29 8:06 ` Mohammed, Afzal 2013-05-29 8:06 ` Mohammed, Afzal 2013-05-29 8:06 ` Mohammed, Afzal 2013-05-29 8:39 ` Benoit Cousson 2013-05-29 8:39 ` Benoit Cousson 2013-05-29 8:39 ` Benoit Cousson 2013-05-29 9:58 ` Mohammed, Afzal 2013-05-29 9:58 ` Mohammed, Afzal 2013-05-29 9:58 ` Mohammed, Afzal 2013-05-29 13:35 ` Benoit Cousson [this message] 2013-05-29 13:35 ` Benoit Cousson 2013-05-29 13:35 ` Benoit Cousson 2013-06-03 7:49 ` Mohammed, Afzal 2013-06-03 7:49 ` Mohammed, Afzal 2013-06-03 7:49 ` Mohammed, Afzal 2013-06-03 9:53 ` Benoit Cousson 2013-06-03 9:53 ` Benoit Cousson 2013-06-03 9:53 ` Benoit Cousson 2013-05-29 15:27 ` Stephen Warren 2013-05-29 15:27 ` Stephen Warren 2013-05-29 15:27 ` Stephen Warren 2013-05-30 11:44 ` Benoit Cousson 2013-05-30 11:44 ` Benoit Cousson 2013-05-30 11:44 ` Benoit Cousson 2013-05-29 20:17 ` Jon Hunter 2013-05-29 20:17 ` Jon Hunter 2013-05-27 14:37 ` [PATCH v2 12/14] Documentation: dt: binding: omap: am43x counter Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-28 21:26 ` Jon Hunter 2013-05-28 21:26 ` Jon Hunter 2013-05-28 21:26 ` Jon Hunter 2013-05-29 8:08 ` Mohammed, Afzal 2013-05-29 8:08 ` Mohammed, Afzal 2013-05-29 8:08 ` Mohammed, Afzal 2013-05-27 14:37 ` [PATCH v2 13/14] Documentation: dt: binding: serial: omap: am43x Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-27 14:37 ` [PATCH v2 14/14] ARM: dts: AM43x: initial support Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-27 14:37 ` Afzal Mohammed 2013-05-29 8:53 ` Benoit Cousson 2013-05-29 8:53 ` Benoit Cousson 2013-05-29 8:53 ` Benoit Cousson 2013-05-29 8:57 ` Florian Vaussard 2013-05-29 8:57 ` Florian Vaussard 2013-05-29 8:57 ` Florian Vaussard 2013-05-29 9:05 ` Benoit Cousson 2013-05-29 9:05 ` Benoit Cousson 2013-05-29 9:05 ` Benoit Cousson 2013-06-04 11:25 ` [PATCH v2 00/14] ARM: OMAP2+: AM43x " Mohammed, Afzal 2013-06-04 11:25 ` Mohammed, Afzal 2013-06-04 11:25 ` Mohammed, Afzal
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=51A60427.6010409@ti.com \ --to=b-cousson@ti.com \ --cc=afzal@ti.com \ --cc=benoit.cousson@linaro.org \ --cc=devicetree-discuss@lists.ozlabs.org \ --cc=grant.likely@linaro.org \ --cc=jgchunter@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-doc@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=rob.herring@calxeda.com \ --cc=swarren@wwwdotorg.org \ /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.