diff mbox

[v2,11/14] Documentation: dt: binding: omap: am43x timer

Message ID 223d90dc9eeab13d8496690d336cdf0f7d27cd22.1369658705.git.afzal@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Afzal Mohammed May 27, 2013, 2:37 p.m. UTC
AM43x timer bindings.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 Documentation/devicetree/bindings/arm/omap/timer.txt | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jon Hunter May 28, 2013, 9:25 p.m. UTC | #1
On 27/05/13 15:37, Afzal Mohammed wrote:
> AM43x timer bindings.
> 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---
>  Documentation/devicetree/bindings/arm/omap/timer.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/timer.txt b/Documentation/devicetree/bindings/arm/omap/timer.txt
> index d02e27c..70cb398 100644
> --- a/Documentation/devicetree/bindings/arm/omap/timer.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/timer.txt
> @@ -14,6 +14,8 @@ Required properties:
>  			ti,omap5430-timer (applicable to OMAP543x devices)
>  			ti,am335x-timer	(applicable to AM335x devices)
>  			ti,am335x-timer-1ms (applicable to AM335x devices)
> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
>  
>  - reg:			Contains timer register address range (base address and
>  			length).

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.

Jon

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren May 28, 2013, 10:05 p.m. UTC | #2
On 05/28/2013 03:25 PM, Jon Hunter wrote:
> 
> On 27/05/13 15:37, Afzal Mohammed wrote:
>> AM43x timer bindings.
>>
>> Signed-off-by: Afzal Mohammed <afzal@ti.com>
>> ---
>>  Documentation/devicetree/bindings/arm/omap/timer.txt | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/timer.txt b/Documentation/devicetree/bindings/arm/omap/timer.txt
>> index d02e27c..70cb398 100644
>> --- a/Documentation/devicetree/bindings/arm/omap/timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/omap/timer.txt
>> @@ -14,6 +14,8 @@ Required properties:
>>  			ti,omap5430-timer (applicable to OMAP543x devices)
>>  			ti,am335x-timer	(applicable to AM335x devices)
>>  			ti,am335x-timer-1ms (applicable to AM335x devices)
>> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
>> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
>>  
>>  - reg:			Contains timer register address range (base address and
>>  			length).
> 
> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Afzal Mohammed May 29, 2013, 8:06 a.m. UTC | #3
Hi Jon,

On Wed, May 29, 2013 at 03:35:10, Stephen Warren wrote:
> On 05/28/2013 03:25 PM, Jon Hunter wrote:


> >>  			ti,am335x-timer	(applicable to AM335x devices)

> >>  			ti,am335x-timer-1ms (applicable to AM335x devices)

> >> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer

> >> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one


> > 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,

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.

Regards
Afzal

[A] http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
Benoit Cousson May 29, 2013, 8:39 a.m. UTC | #4
Hi Afzal,

On 05/29/2013 10:06 AM, Mohammed, Afzal wrote:
> Hi Jon,
> 
> On Wed, May 29, 2013 at 03:35:10, Stephen Warren wrote:
>> On 05/28/2013 03:25 PM, Jon Hunter wrote:
> 
>>>>  			ti,am335x-timer	(applicable to AM335x devices)
>>>>  			ti,am335x-timer-1ms (applicable to AM335x devices)
>>>> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
>>>> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
> 
>>> 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.

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.

Regards,
Benoit


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Afzal Mohammed May 29, 2013, 9:58 a.m. UTC | #5
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). Binding has
the authority to allow its usage in DT sources.

> 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. 

Regards
Afzal
Benoit Cousson May 29, 2013, 1:35 p.m. UTC | #6
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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren May 29, 2013, 3:27 p.m. UTC | #7
On 05/29/2013 02:39 AM, Benoit Cousson wrote:
> Hi Afzal,
> 
> On 05/29/2013 10:06 AM, Mohammed, Afzal wrote:
>> Hi Jon,
>>
>> On Wed, May 29, 2013 at 03:35:10, Stephen Warren wrote:
>>> On 05/28/2013 03:25 PM, Jon Hunter wrote:
>>
>>>>>  			ti,am335x-timer	(applicable to AM335x devices)
>>>>>  			ti,am335x-timer-1ms (applicable to AM335x devices)
>>>>> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
>>>>> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
>>
>>>> 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 don't agree here; the DT binding should define all the required and/or
allowed values that must/should/can be present in the DT - the entire
legal schema. The set of all compatible values is included in that,
irrespective of whether a particular value actually (currently) defines
a different HW interface or not.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter May 29, 2013, 8:17 p.m. UTC | #8
On 28/05/13 23:05, Stephen Warren wrote:
> On 05/28/2013 03:25 PM, Jon Hunter wrote:
>>
>> On 27/05/13 15:37, Afzal Mohammed wrote:
>>> AM43x timer bindings.
>>>
>>> Signed-off-by: Afzal Mohammed <afzal@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/arm/omap/timer.txt | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/omap/timer.txt b/Documentation/devicetree/bindings/arm/omap/timer.txt
>>> index d02e27c..70cb398 100644
>>> --- a/Documentation/devicetree/bindings/arm/omap/timer.txt
>>> +++ b/Documentation/devicetree/bindings/arm/omap/timer.txt
>>> @@ -14,6 +14,8 @@ Required properties:
>>>  			ti,omap5430-timer (applicable to OMAP543x devices)
>>>  			ti,am335x-timer	(applicable to AM335x devices)
>>>  			ti,am335x-timer-1ms (applicable to AM335x devices)
>>> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
>>> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
>>>  
>>>  - reg:			Contains timer register address range (base address and
>>>  			length).
>>
>> 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.

Yes that does seem to agree with what Grant had told me in the past [1].
So this is ok then. However, I still think that the changelog needs to
be vastly improved and more explicit about the changes.

Cheers
Jon

[1] http://thread.gmane.org/gmane.linux.ports.arm.omap/90551/focus=26447

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson May 30, 2013, 11:44 a.m. UTC | #9
Hi Stephen,

On 05/29/2013 05:27 PM, Stephen Warren wrote:
> On 05/29/2013 02:39 AM, Benoit Cousson wrote:
>> Hi Afzal,
>>
>> On 05/29/2013 10:06 AM, Mohammed, Afzal wrote:
>>> Hi Jon,
>>>
>>> On Wed, May 29, 2013 at 03:35:10, Stephen Warren wrote:
>>>> On 05/28/2013 03:25 PM, Jon Hunter wrote:
>>>
>>>>>>  			ti,am335x-timer	(applicable to AM335x devices)
>>>>>>  			ti,am335x-timer-1ms (applicable to AM335x devices)
>>>>>> +			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
>>>>>> +			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
>>>
>>>>> 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 don't agree here; the DT binding should define all the required and/or
> allowed values that must/should/can be present in the DT - the entire
> legal schema. The set of all compatible values is included in that,
> irrespective of whether a particular value actually (currently) defines
> a different HW interface or not.

Well, I tend to agree on the principle, but so far it was never really
done like that. That's not necessarily a good excuse, but if we start
adding new bindings for the huge number of OMAP|AM variants TI has been
introduced for 10 years, I'd rather use a wildcard than a exhaustive
list of all the devices.
Something like ti,[omap|am]*-timer for example .

Regards,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Afzal Mohammed June 3, 2013, 7:49 a.m. UTC | #10
SGkgQmVub2l0LA0KDQpPbiBXZWQsIE1heSAyOSwgMjAxMyBhdCAxOTowNTozNSwgQ291c3Nvbiwg
QmVub2l0IHdyb3RlOg0KDQo+IEFuZCBpbiB0aGlzIGNhc2UsIHlvdSBkbyBub3QgaW50cm9kdWNl
IGFueSBuZXcgcmV2aXNpb24uDQo+IA0KPiBUaGVyZSBpcyBubyBwb2ludCB0byB1cGRhdGUgdGhl
IGJpbmRpbmcgZWFjaCB0aW1lIHdlIGFkZCBhIG5ldyBTb0MNCj4gdmFyaWFudCB0aGF0IHdpbGwg
Y29udGFpbiB0aGUgZXhhY3Qgc2FtZSBJUC4NCj4gDQo+IEkgdGhpbmsgaXQgd2lsbCBtYWlubHkg
Y29uZnVzZSB0aGUgdXNlciB0aGF0IHdpbGwgd29uZGVyIHdoYXQgaXMNCj4gZGlmZmVyZW50IGlu
IHRoYXQgdmVyc2lvbiBjb21wYXJlIHRvIHRoZSBwcmV2aW91cyBvbmUsIG1vcmVvdmVyIHdlIGNh
bg0KPiBlbmQgdXAgd2l0aCBodW5kcmVkIG9mIGVudHJpZXMgZm9yIHRoZSBleGFjdCBzYW1lIElQ
IGZvciBub3RoaW5nLg0KPiANCj4gVGhlIHJlYWwgcHJvYmxlbSBpcyBkdWUgdG8gdGhlIGludHJv
ZHVjdGlvbiBvZiB0aGUgU29DIG5hbWUgaW4gdGhlDQo+IGRldmljZSBjb21wYXRpYmxlIG5hbWUu
IFRoYXQgZG9lcyBpbnRyb2R1Y2VkIGEgU29DIGxldmVsIGluZm9ybWF0aW9uDQo+IHRoYXQgaXMg
bW9zdGx5IGlycmVsZXZhbnQgYXQgZGV2aWNlIGxldmVsLiBJIGNhbiB1bmRlcnN0YW5kIHdoeSBp
dCB3YXMNCj4gZG9uZSBmb3IgcHJhY3RpY2FsIGFzcGVjdCB3aGVuIHRoZSBJUCB2ZXJzaW9uIGlz
IG5vdCB3ZWxsIGlkZW50aWZpZWQsDQo+IGJ1dCB0aGF0IGNhbiBsZWFkIHRvIHRoaXMgcHJvbGlm
ZXJhdGlvbiBvZiBuZXcgcG9pbnRsZXNzIGJpbmRpbmdzLg0KDQpBcyBvcGluaW9ucyBvbiAkc3Vi
amVjdCBzZWVtcyBub3QgeWV0IHRvIGJlIGNvbmNsdXNpdmUsIEkgcGxhbiB0bw0KcmViYXNlIERU
UyBwYXRjaCAoMTQvMTQpIG92ZXIgeW91ciAnZm9yXzMuMTEvZHRzJyBicmFuY2ggKHRoYXQgbWFr
ZXMNCnVzZSBvZiBDIHByZXByb2Nlc3NvciBvbiBPTUFQIERUUykgYW5kIHBvc3Qgc2VwYXJhdGVs
eSBkcm9wcGluZw0KMTEtMTQgcGF0Y2hlcywgaXMgdGhhdCBva2F5ID8NCg0KUmVnYXJkcw0KQWZ6
YWwNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson June 3, 2013, 9:53 a.m. UTC | #11
Hi Afzal,

On 06/03/2013 09:49 AM, Mohammed, Afzal wrote:
> Hi Benoit,
> 
> On Wed, May 29, 2013 at 19:05:35, Cousson, Benoit wrote:
> 
>> 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.
> 
> As opinions on $subject seems not yet to be conclusive, I plan to
> rebase DTS patch (14/14) over your 'for_3.11/dts' branch (that makes
> use of C preprocessor on OMAP DTS) and post separately dropping
> 11-14 patches, is that okay ?

Yes, that sounds good to me.

Thanks,
Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/arm/omap/timer.txt b/Documentation/devicetree/bindings/arm/omap/timer.txt
index d02e27c..70cb398 100644
--- a/Documentation/devicetree/bindings/arm/omap/timer.txt
+++ b/Documentation/devicetree/bindings/arm/omap/timer.txt
@@ -14,6 +14,8 @@  Required properties:
 			ti,omap5430-timer (applicable to OMAP543x devices)
 			ti,am335x-timer	(applicable to AM335x devices)
 			ti,am335x-timer-1ms (applicable to AM335x devices)
+			"ti,am4372-timer-1ms", "ti,am335x-timer-1ms" for AM43x 1ms timer
+			"ti,am4372-timer", "ti,am335x-timer" for AM43x timers other than 1ms one
 
 - reg:			Contains timer register address range (base address and
 			length).