On Thu, Mar 17, 2016 at 2:25 PM, Franklin S Cooper Jr. [off-list ref] wrote:

+Sekhar

On 03/17/2016 01:56 PM, Rob Herring wrote:

quoted

On Thu, Mar 17, 2016 at 1:20 PM, Franklin S Cooper Jr. [off-list ref] wrote:

quoted

On 03/17/2016 01:00 PM, Rob Herring wrote:

quoted

On Thu, Mar 17, 2016 at 11:49 AM, Franklin S Cooper Jr. [off-list ref] wrote:

quoted

On 03/17/2016 10:03 AM, Rob Herring wrote:

quoted

On Mon, Mar 07, 2016 at 01:51:58PM -0600, Franklin S Cooper Jr wrote:

quoted

Now that the node name has been changed from ehrpwm to pwm the document
should show this proper usage. Also change the unit address in the example
from 0 to the proper physical address value that should be used.

Signed-off-by: Franklin S Cooper Jr <redacted>
---
 Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
index 9c100b2..20211ed 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt

@@ -15,14 +15,14 @@ Optional properties:

 Example:

-ehrpwm0: ehrpwm at 0 { /* EHRPWM on am33xx */
+ehrpwm0: pwm at 48300200 { /* EHRPWM on am33xx */
     compatible = "ti,am33xx-ehrpwm";
     #pwm-cells = <3>;
     reg = <0x48300200 0x100>;
     ti,hwmods = "ehrpwm0";
 };

-ehrpwm0: ehrpwm at 0 { /* EHRPWM on da850 */
+ehrpwm0: pwm at 01f00000 { /* EHRPWM on da850 */

No leading 0s, but more importantly the address is wrong.

I will remove the leading 0. However, this value was taken
from the .dtsi and I just double checked and I see the same
value in the datasheet. I believe DA850,OMAP-L138 and AM18x
all have the same memory mapping. I'm looking at
http://www.ti.com/lit/ds/symlink/am1808.pdf page 233 and the
addresses match up what is seen here and in the .dtsi.

Can you point me to which document your looking at that
shows a different value?

Ummm, ...

quoted

quoted

quoted

     compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
     #pwm-cells = <3>;
     reg = <0x300000 0x2000>;

right here.

So I don't know the history but the SOC node specifies a
ranges value of ranges = <0x0 0x01c00000 0x400000>;. It
seems that all child nodes of SOC have a reg property then
is based on an offset of 0x01c00000. So this is true for
UART, rtc, i2c, wdt, mmc, spi etc... So using a base offset
of 0x01c00000 + 0x300000 (reg value of the pwm) equals the
physical address of the ehrpwm0 register 0x1f00000.

For the child nodes within the SOC node, the unit-address is
always based on the physical address not based on the offset
address.

They are all wrong and should be fixed. Unit address should match the
reg property unless the bus has defined something different (e.g.
PCI).

I added Sekhar to hopefully comment if it would be better to
try to get the reg properties to use their physical
addresses or change the unit address to use the offset values.

Change the unit addresses (i.e. use ranges). The main reason ranges
with translation is preferred is it limits the scope of the bus as the
h/w design typically would.

I don't mind shooting a patch that makes this massive switch
for all the various peripherals and documentation. However,
this is a bit beyond this series.  In this series I am not
setting the unit address or reg address I am simply
documenting what is currently being used. This patch set is
a dependency for another patchset to get PWM support for DRA7.

Agreed. I'm not asking for everything to be fixed.

Are you ok with me doing the below?
I submit a v2 that removes the leading 0 in this patch. This
way this patchset and my PWMSS support for DRA7 patchset can
get merged. I can then follow up with another separate
patchset that insures both the unit address and the value in
the reg property are aligned.

The example is an example. It doesn't have to match dts files (if it
always did, then examples would be pointless). So please fix the
documentation now. The dts files can be updated separately.

Rob