diff mbox

[4/5] pwm: pwm-tipwmss: Update documentation to use empty range property

Message ID 1457380318-15452-5-git-send-email-fcooper@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Franklin Cooper March 7, 2016, 7:51 p.m. UTC
Since the PWMSS and its subdevices (eCAP and ePWM) use the same address
space then the range property should be empty. Update the documentation
to show the correct usage.

Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Comments

Rob Herring (Arm) March 17, 2016, 3:01 p.m. UTC | #1
On Mon, Mar 07, 2016 at 01:51:57PM -0600, Franklin S Cooper Jr wrote:
> Since the PWMSS and its subdevices (eCAP and ePWM) use the same address
> space then the range property should be empty. Update the documentation
> to show the correct usage.

Why does it matter? An empty ranges is generally not preferred.

> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
>  Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
> index f7eae77..672fa71 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
> @@ -7,11 +7,9 @@ Required properties:
>  		  Should set to 1.
>  - size-cells: specify number of u32 entries needed to specify child nodes size
>  		in reg property. Should set to 1.
> -- ranges: describes the address mapping of a memory-mapped bus. Should set to
> -	   physical address map of child's base address, physical address within
> -	   parent's address  space and length of the address map. For am33xx,
> -	   3 set of child register maps present, ECAP register space, EQEP
> -	   register space, EHRPWM register space.
> +- ranges: describes the address mapping of a memory-mapped bus. Its value
> +	  should be empty since no address translation is needed between the
> +	  parent and the child.
>  
>  Also child nodes should also populated under PWMSS DT node.
>  
> @@ -23,9 +21,7 @@ pwmss0: pwmss@48300000 {
>  	#address-cells = <1>;
>  	#size-cells = <1>;
>  	status = "disabled";
> -	ranges = <0x48300100 0x48300100 0x80   /* ECAP */
> -		  0x48300180 0x48300180 0x80   /* EQEP */
> -		  0x48300200 0x48300200 0x80>; /* EHRPWM */
> +	ranges;
>  
>  	/* child nodes go here */
>  };
> -- 
> 2.7.0
> 
--
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
Franklin Cooper March 17, 2016, 4:56 p.m. UTC | #2
On 03/17/2016 10:01 AM, Rob Herring wrote:
> On Mon, Mar 07, 2016 at 01:51:57PM -0600, Franklin S Cooper Jr wrote:
>> Since the PWMSS and its subdevices (eCAP and ePWM) use the same address
>> space then the range property should be empty. Update the documentation
>> to show the correct usage.
> Why does it matter? An empty ranges is generally not preferred.

Someone pointed out that ranges should probably be empty. I
double checked it with what is in the ePAPR doc and based on
the definition it should be set to empty. I also checked
against the am33xx.dtsi and saw that both USB and ethernet
uses an empty value for ranges.

Can you elaborate on why this isn't preferable?

>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>>  Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
>> index f7eae77..672fa71 100644
>> --- a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
>> @@ -7,11 +7,9 @@ Required properties:
>>  		  Should set to 1.
>>  - size-cells: specify number of u32 entries needed to specify child nodes size
>>  		in reg property. Should set to 1.
>> -- ranges: describes the address mapping of a memory-mapped bus. Should set to
>> -	   physical address map of child's base address, physical address within
>> -	   parent's address  space and length of the address map. For am33xx,
>> -	   3 set of child register maps present, ECAP register space, EQEP
>> -	   register space, EHRPWM register space.
>> +- ranges: describes the address mapping of a memory-mapped bus. Its value
>> +	  should be empty since no address translation is needed between the
>> +	  parent and the child.
>>  
>>  Also child nodes should also populated under PWMSS DT node.
>>  
>> @@ -23,9 +21,7 @@ pwmss0: pwmss@48300000 {
>>  	#address-cells = <1>;
>>  	#size-cells = <1>;
>>  	status = "disabled";
>> -	ranges = <0x48300100 0x48300100 0x80   /* ECAP */
>> -		  0x48300180 0x48300180 0x80   /* EQEP */
>> -		  0x48300200 0x48300200 0x80>; /* EHRPWM */
>> +	ranges;
>>  
>>  	/* child nodes go here */
>>  };
>> -- 
>> 2.7.0
>>

--
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
Franklin Cooper March 17, 2016, 9:29 p.m. UTC | #3
On 03/17/2016 11:56 AM, Franklin S Cooper Jr. wrote:
>
> On 03/17/2016 10:01 AM, Rob Herring wrote:
>> On Mon, Mar 07, 2016 at 01:51:57PM -0600, Franklin S Cooper Jr wrote:
>>> Since the PWMSS and its subdevices (eCAP and ePWM) use the same address
>>> space then the range property should be empty. Update the documentation
>>> to show the correct usage.
>> Why does it matter? An empty ranges is generally not preferred.
> Someone pointed out that ranges should probably be empty. I
> double checked it with what is in the ePAPR doc and based on
> the definition it should be set to empty. I also checked
> against the am33xx.dtsi and saw that both USB and ethernet
> uses an empty value for ranges.
>
> Can you elaborate on why this isn't preferable?

I'm going to drop this patch and the previous patch for now
that messes with the ranges property. I can resubmit them
separately later on if that is the route we decide to take.
>
>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt | 12 ++++--------
>>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
>>> index f7eae77..672fa71 100644
>>> --- a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
>>> @@ -7,11 +7,9 @@ Required properties:
>>>  		  Should set to 1.
>>>  - size-cells: specify number of u32 entries needed to specify child nodes size
>>>  		in reg property. Should set to 1.
>>> -- ranges: describes the address mapping of a memory-mapped bus. Should set to
>>> -	   physical address map of child's base address, physical address within
>>> -	   parent's address  space and length of the address map. For am33xx,
>>> -	   3 set of child register maps present, ECAP register space, EQEP
>>> -	   register space, EHRPWM register space.
>>> +- ranges: describes the address mapping of a memory-mapped bus. Its value
>>> +	  should be empty since no address translation is needed between the
>>> +	  parent and the child.
>>>  
>>>  Also child nodes should also populated under PWMSS DT node.
>>>  
>>> @@ -23,9 +21,7 @@ pwmss0: pwmss@48300000 {
>>>  	#address-cells = <1>;
>>>  	#size-cells = <1>;
>>>  	status = "disabled";
>>> -	ranges = <0x48300100 0x48300100 0x80   /* ECAP */
>>> -		  0x48300180 0x48300180 0x80   /* EQEP */
>>> -		  0x48300200 0x48300200 0x80>; /* EHRPWM */
>>> +	ranges;
>>>  
>>>  	/* child nodes go here */
>>>  };
>>> -- 
>>> 2.7.0
>>>
> --
> 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

--
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/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
index f7eae77..672fa71 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
@@ -7,11 +7,9 @@  Required properties:
 		  Should set to 1.
 - size-cells: specify number of u32 entries needed to specify child nodes size
 		in reg property. Should set to 1.
-- ranges: describes the address mapping of a memory-mapped bus. Should set to
-	   physical address map of child's base address, physical address within
-	   parent's address  space and length of the address map. For am33xx,
-	   3 set of child register maps present, ECAP register space, EQEP
-	   register space, EHRPWM register space.
+- ranges: describes the address mapping of a memory-mapped bus. Its value
+	  should be empty since no address translation is needed between the
+	  parent and the child.
 
 Also child nodes should also populated under PWMSS DT node.
 
@@ -23,9 +21,7 @@  pwmss0: pwmss@48300000 {
 	#address-cells = <1>;
 	#size-cells = <1>;
 	status = "disabled";
-	ranges = <0x48300100 0x48300100 0x80   /* ECAP */
-		  0x48300180 0x48300180 0x80   /* EQEP */
-		  0x48300200 0x48300200 0x80>; /* EHRPWM */
+	ranges;
 
 	/* child nodes go here */
 };