Hi Mugunthan,

On Thu, Dec 24, 2015 at 6:38 PM, Mugunthan V N mugunthanvnm@ti.com wrote:

Adding timer_init function to create and initialize the timer device on platforms where u-boot,dm-pre-reloc is not used. Since there will be multiple timer devices in the system, adding a tick-timer node in chosen node to know which timer device to be used as tick timer in u-boot.

Signed-off-by: Mugunthan V N mugunthanvnm@ti.com

doc/device-tree-bindings/chosen.txt | 43 +++++++++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 42 ++++++++++++++++++++++++++++++++++++ lib/time.c | 13 ++--------- 3 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 doc/device-tree-bindings/chosen.txt

diff --git a/doc/device-tree-bindings/chosen.txt b/doc/device-tree-bindings/chosen.txt new file mode 100644 index 0000000..bf9a30a --- /dev/null +++ b/doc/device-tree-bindings/chosen.txt @@ -0,0 +1,43 @@ +The chosen node +--------------- +The chosen node does not represent a real device, but serves as a place +for passing data like which serial device to used to print the logs etc

+stdout-path property +-------------------- +Device trees may specify the device to be used for boot console output +with a stdout-path property under /chosen.

+Example +------- +/ {

•   chosen {
  

•           stdout-path = "/serial@f00:115200";
  

•   };
  

•   serial@f00 {
  

•           compatible = "vendor,some-uart";
  

•           reg = <0xf00 0x10>;
  

•   };
  

+};

+tick-timer property +------------------- +In a system there are multiple timers, specify which timer to be used +as the tick-timer. Earlier it was hardcoded in the timer driver now +since device tree has all the timer nodes. Specify which timer to be +used as tick timer.

+Example +------- +/ {

•   chosen {
  

•           tick-timer = "/timer2@f00";
  

•   };
  

•   timer2@f00 {
  

•           compatible = "vendor,some-timer";
  

•           reg = <0xf00 0x10>;
  

•   };
  

+}; diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index aca421b..db43611 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -6,6 +6,8 @@

#include <common.h> #include <dm.h> +#include <dm/lists.h> +#include <dm/device-internal.h> #include <errno.h> #include <timer.h>

@@ -56,6 +58,46 @@ u64 timer_conv_64(u32 count) return ((u64)gd->timebase_h << 32) | gd->timebase_l; }

+int timer_init(void) +{

•   const void *blob = gd->fdt_blob;
  

•   struct udevice *dev;
  

•   int node;
  

•   int ret;
  

•   /* Check for a chosen timer to be used for tick */
  

•   node = fdtdec_get_chosen_node(blob, "tick-timer");
  

•   if (node < 0)
  

•           return -ENODEV;
  

This changes now require every device tree provide a "tick-timer" under /chosen, which break all the existing dm timer enabled boards, and is not necessary.

We should do:

if (node < 0) { ret = uclass_first_device(UCLASS_TIMER, &dev); if (ret) return ret; if (!dev) return -ENODEV; gd->timer = dev; } else { .... }

•   if (uclass_get_device_by_of_offset(UCLASS_TIMER, node, &dev)) {
  

•           /*
  

•            * If the timer is not marked to be bound before
  

•            * relocation, bind it anyway.
  

•            */
  

•           if (node > 0 &&
  

•               !lists_bind_fdt(gd->dm_root, blob, node, &dev)) {
  

•                   int ret = device_probe(dev);
  

•                   if (ret)
  

•                           return ret;
  

•           }
  

•   }
  

•   if (dev) {
  

•           gd->timer = dev;
  

•           return 0;
  

•   }
  

•   ret = uclass_first_device(UCLASS_TIMER, &dev);
  

•   if (ret)
  

•           return ret;
  

•   if (!dev)
  

•           return -ENODEV;
  

•   gd->timer = dev;
  

•   return 0;
  

+}

UCLASS_DRIVER(timer) = { .id = UCLASS_TIMER, .name = "timer", diff --git a/lib/time.c b/lib/time.c index f37a662..d4060f1 100644 --- a/lib/time.c +++ b/lib/time.c @@ -43,17 +43,8 @@ extern unsigned long __weak timer_read_counter(void); #ifdef CONFIG_TIMER static int notrace dm_timer_init(void) {

•   struct udevice *dev;
  

•   int ret;
  

•   if (!gd->timer) {
  

•           ret = uclass_first_device(UCLASS_TIMER, &dev);
  

•           if (ret)
  

•                   return ret;
  

•           if (!dev)
  

•                   return -ENODEV;
  

•           gd->timer = dev;
  

•   }
  

•   if (!gd->timer)
  

•           return timer_init();
  

Looks timer_init() is only called in timer-uclass.c, should we declare it as static? Otherwise timer_init() will be called in either board_f.c or board_r.c on different architectures. This causes timer devices to be probed twice. We may just simply move all codes in timer_init() into dm_timer_init().

    return 0;

}

Regards, Bin

Adding timer_init function to create and initialize the timer device on platforms where u-boot,dm-pre-reloc is not used. Since there will be multiple timer devices in the system, adding a tick-timer node in chosen node to know which timer device to be used as tick timer in u-boot.

Signed-off-by: Mugunthan V N mugunthanvnm@ti.com ---

Submitting 02/19 only for v3 as there is no change on other patches and to reduce traffic.

This patch is verified on AM437x SK and DRA74 EVM logs [1] and pushed a branch for testing

[1] - http://pastebin.ubuntu.com/14205433/ [2] - git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-timer-v3

Changes from v2->v3: * Fixed issue which is in v2 that if no chosen timer node is present in DT then timer init always fails without trying first available timer.

--- doc/device-tree-bindings/chosen.txt | 43 +++++++++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 41 +++++++++++++++++++++++++++++++++++ lib/time.c | 13 ++--------- 3 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 doc/device-tree-bindings/chosen.txt

diff --git a/doc/device-tree-bindings/chosen.txt b/doc/device-tree-bindings/chosen.txt new file mode 100644 index 0000000..bf9a30a --- /dev/null +++ b/doc/device-tree-bindings/chosen.txt @@ -0,0 +1,43 @@ +The chosen node +--------------- +The chosen node does not represent a real device, but serves as a place +for passing data like which serial device to used to print the logs etc + + +stdout-path property +-------------------- +Device trees may specify the device to be used for boot console output +with a stdout-path property under /chosen. + +Example +------- +/ { + chosen { + stdout-path = "/serial@f00:115200"; + }; + + serial@f00 { + compatible = "vendor,some-uart"; + reg = <0xf00 0x10>; + }; +}; + +tick-timer property +------------------- +In a system there are multiple timers, specify which timer to be used +as the tick-timer. Earlier it was hardcoded in the timer driver now +since device tree has all the timer nodes. Specify which timer to be +used as tick timer. + +Example +------- +/ { + chosen { + tick-timer = "/timer2@f00"; + }; + + timer2@f00 { + compatible = "vendor,some-timer"; + reg = <0xf00 0x10>; + }; +}; diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index aca421b..b6699f2 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -6,6 +6,8 @@

#include <common.h> #include <dm.h> +#include <dm/lists.h> +#include <dm/device-internal.h> #include <errno.h> #include <timer.h>

@@ -56,6 +58,45 @@ u64 timer_conv_64(u32 count) return ((u64)gd->timebase_h << 32) | gd->timebase_l; }

+int timer_init(void) +{ + const void *blob = gd->fdt_blob; + struct udevice *dev = NULL; + int node; + int ret; + + /* Check for a chosen timer to be used for tick */ + node = fdtdec_get_chosen_node(blob, "tick-timer"); + if (node < 0) { + /* No chosen timer, trying first available timer */ + ret = uclass_first_device(UCLASS_TIMER, &dev); + if (ret) + return ret; + if (!dev) + return -ENODEV; + } else { + if (uclass_get_device_by_of_offset(UCLASS_TIMER, node, &dev)) { + /* + * If the timer is not marked to be bound before + * relocation, bind it anyway. + */ + if (node > 0 && + !lists_bind_fdt(gd->dm_root, blob, node, &dev)) { + int ret = device_probe(dev); + if (ret) + return ret; + } + } + } + + if (dev) { + gd->timer = dev; + return 0; + } + + return -ENODEV; +} + UCLASS_DRIVER(timer) = { .id = UCLASS_TIMER, .name = "timer", diff --git a/lib/time.c b/lib/time.c index f37a662..d4060f1 100644 --- a/lib/time.c +++ b/lib/time.c @@ -43,17 +43,8 @@ extern unsigned long __weak timer_read_counter(void); #ifdef CONFIG_TIMER static int notrace dm_timer_init(void) { - struct udevice *dev; - int ret; - - if (!gd->timer) { - ret = uclass_first_device(UCLASS_TIMER, &dev); - if (ret) - return ret; - if (!dev) - return -ENODEV; - gd->timer = dev; - } + if (!gd->timer) + return timer_init();

return 0; }

On Friday 25 December 2015 05:43 PM, Bin Meng wrote:

Hi Mugunthan,

On Fri, Dec 25, 2015 at 6:41 PM, Mugunthan V N mugunthanvnm@ti.com wrote:

...

Adding timer_init function to create and initialize the timer device on platforms where u-boot,dm-pre-reloc is not used. Since there will be multiple timer devices in the system, adding a tick-timer node in chosen node to know which timer device to be used as tick timer in u-boot.

Signed-off-by: Mugunthan V N mugunthanvnm@ti.com

Submitting 02/19 only for v3 as there is no change on other patches and to reduce traffic.

This patch is verified on AM437x SK and DRA74 EVM logs [1] and pushed a branch for testing

[1] - http://pastebin.ubuntu.com/14205433/ [2] - git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-timer-v3

Changes from v2->v3:

• Fixed issue which is in v2 that if no chosen timer node is present in DT then timer init always fails without trying first available timer.

---

doc/device-tree-bindings/chosen.txt | 43 +++++++++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 41 +++++++++++++++++++++++++++++++++++ lib/time.c | 13 ++--------- 3 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 doc/device-tree-bindings/chosen.txt

diff --git a/doc/device-tree-bindings/chosen.txt b/doc/device-tree-bindings/chosen.txt new file mode 100644 index 0000000..bf9a30a --- /dev/null +++ b/doc/device-tree-bindings/chosen.txt @@ -0,0 +1,43 @@ +The chosen node +--------------- +The chosen node does not represent a real device, but serves as a place +for passing data like which serial device to used to print the logs etc

+stdout-path property +-------------------- +Device trees may specify the device to be used for boot console output +with a stdout-path property under /chosen.

+Example +------- +/ {

•   chosen {
  

•           stdout-path = "/serial@f00:115200";
  

•   };
  

•   serial@f00 {
  

•           compatible = "vendor,some-uart";
  

•           reg = <0xf00 0x10>;
  

•   };
  

+};

+tick-timer property +------------------- +In a system there are multiple timers, specify which timer to be used +as the tick-timer. Earlier it was hardcoded in the timer driver now +since device tree has all the timer nodes. Specify which timer to be +used as tick timer.

+Example +------- +/ {

•   chosen {
  

•           tick-timer = "/timer2@f00";
  

•   };
  

•   timer2@f00 {
  

•           compatible = "vendor,some-timer";
  

•           reg = <0xf00 0x10>;
  

•   };
  

+}; diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index aca421b..b6699f2 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -6,6 +6,8 @@

#include <common.h> #include <dm.h> +#include <dm/lists.h> +#include <dm/device-internal.h> #include <errno.h> #include <timer.h>

@@ -56,6 +58,45 @@ u64 timer_conv_64(u32 count) return ((u64)gd->timebase_h << 32) | gd->timebase_l; }

+int timer_init(void) +{

•   const void *blob = gd->fdt_blob;
  

•   struct udevice *dev = NULL;
  

•   int node;
  

•   int ret;
  

•   /* Check for a chosen timer to be used for tick */
  

•   node = fdtdec_get_chosen_node(blob, "tick-timer");
  

•   if (node < 0) {
  

•           /* No chosen timer, trying first available timer */
  

•           ret = uclass_first_device(UCLASS_TIMER, &dev);
  

•           if (ret)
  

•                   return ret;
  

•           if (!dev)
  

•                   return -ENODEV;
  

Thanks for the quick v3!

...

•   } else {
  

•           if (uclass_get_device_by_of_offset(UCLASS_TIMER, node, &dev)) {
  

•                   /*
  

•                    * If the timer is not marked to be bound before
  

•                    * relocation, bind it anyway.
  

•                    */
  

•                   if (node > 0 &&
  

•                       !lists_bind_fdt(gd->dm_root, blob, node, &dev)) {
  

•                           int ret = device_probe(dev);
  

nits: no need to declare another ret. Just "ret = device_probe(dev)".

Hmmm, yep not needed.

...

•                           if (ret)
  

•                                   return ret;
  

•                   }
  

•           }
  

•   }
  

•   if (dev) {
  

•           gd->timer = dev;
  

•           return 0;
  

•   }
  

•   return -ENODEV;
  

+}

UCLASS_DRIVER(timer) = { .id = UCLASS_TIMER, .name = "timer", diff --git a/lib/time.c b/lib/time.c index f37a662..d4060f1 100644 --- a/lib/time.c +++ b/lib/time.c @@ -43,17 +43,8 @@ extern unsigned long __weak timer_read_counter(void); #ifdef CONFIG_TIMER static int notrace dm_timer_init(void) {

•   struct udevice *dev;
  

•   int ret;
  

•   if (!gd->timer) {
  

•           ret = uclass_first_device(UCLASS_TIMER, &dev);
  

•           if (ret)
  

•                   return ret;
  

•           if (!dev)
  

•                   return -ENODEV;
  

•           gd->timer = dev;
  

•   }
  

•   if (!gd->timer)
  

•           return timer_init();
  

What about my comments in v2, that make timer_init() static or just move codes in timer_init() into this dm_timer_init()?

Oops, missed :)

The timer_init() code can be moved to dm_timer_init(), but since it provides timer uclass functionality I kept the code in timer-uclass driver.

Now since dm_timer_init() doesn't have any thing other than calling timer_init, I will move the dm_timer_init() to timer_uclass and move timer_init code to it.

Regards Mugunthan V N

Adding timer init function in timer-uclass driver to create and initialize the timer device on platforms where u-boot,dm-pre-reloc is not used. Since there will be multiple timer devices in the system, adding a tick-timer node in chosen node to know which timer device to be used as tick timer in u-boot.

Signed-off-by: Mugunthan V N mugunthanvnm@ti.com --- doc/device-tree-bindings/chosen.txt | 43 ++++++++++++++++++++++++++++++++++++ drivers/timer/timer-uclass.c | 44 +++++++++++++++++++++++++++++++++++++ include/timer.h | 9 ++++++++ lib/time.c | 17 -------------- 4 files changed, 96 insertions(+), 17 deletions(-) create mode 100644 doc/device-tree-bindings/chosen.txt

Changes from v3: * removed dm_timer_init() in lib/time.c as it doesn't add anything and simply calls uclass driver's init and also renamed timer_init() to dm_timer_init() as many platforms calls timer_init() before dm init. * removed re-declaring of ret in dm_timer_init()

Tested this patch on DRA74x EVM (logs [1]) with sleep 10 with minicom timestamps and pushed a branch [2] for testing.

[1] - http://pastebin.ubuntu.com/14520385/ [2] - git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-timer-v4

---

diff --git a/doc/device-tree-bindings/chosen.txt b/doc/device-tree-bindings/chosen.txt new file mode 100644 index 0000000..bf9a30a --- /dev/null +++ b/doc/device-tree-bindings/chosen.txt @@ -0,0 +1,43 @@ +The chosen node +--------------- +The chosen node does not represent a real device, but serves as a place +for passing data like which serial device to used to print the logs etc + + +stdout-path property +-------------------- +Device trees may specify the device to be used for boot console output +with a stdout-path property under /chosen. + +Example +------- +/ { + chosen { + stdout-path = "/serial@f00:115200"; + }; + + serial@f00 { + compatible = "vendor,some-uart"; + reg = <0xf00 0x10>; + }; +}; + +tick-timer property +------------------- +In a system there are multiple timers, specify which timer to be used +as the tick-timer. Earlier it was hardcoded in the timer driver now +since device tree has all the timer nodes. Specify which timer to be +used as tick timer. + +Example +------- +/ { + chosen { + tick-timer = "/timer2@f00"; + }; + + timer2@f00 { + compatible = "vendor,some-timer"; + reg = <0xf00 0x10>; + }; +}; diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c index aca421b..0480178 100644 --- a/drivers/timer/timer-uclass.c +++ b/drivers/timer/timer-uclass.c @@ -6,6 +6,8 @@

#include <common.h> #include <dm.h> +#include <dm/lists.h> +#include <dm/device-internal.h> #include <errno.h> #include <timer.h>

@@ -56,6 +58,48 @@ u64 timer_conv_64(u32 count) return ((u64)gd->timebase_h << 32) | gd->timebase_l; }

+int notrace dm_timer_init(void) +{ + const void *blob = gd->fdt_blob; + struct udevice *dev = NULL; + int node; + int ret; + + if (gd->timer) + return 0; + + /* Check for a chosen timer to be used for tick */ + node = fdtdec_get_chosen_node(blob, "tick-timer"); + if (node < 0) { + /* No chosen timer, trying first available timer */ + ret = uclass_first_device(UCLASS_TIMER, &dev); + if (ret) + return ret; + if (!dev) + return -ENODEV; + } else { + if (uclass_get_device_by_of_offset(UCLASS_TIMER, node, &dev)) { + /* + * If the timer is not marked to be bound before + * relocation, bind it anyway. + */ + if (node > 0 && + !lists_bind_fdt(gd->dm_root, blob, node, &dev)) { + ret = device_probe(dev); + if (ret) + return ret; + } + } + } + + if (dev) { + gd->timer = dev; + return 0; + } + + return -ENODEV; +} + UCLASS_DRIVER(timer) = { .id = UCLASS_TIMER, .name = "timer", diff --git a/include/timer.h b/include/timer.h index 7fee17e..f14725c 100644 --- a/include/timer.h +++ b/include/timer.h @@ -8,6 +8,15 @@ #define _TIMER_H_

/* + * dm_timer_init - initialize a timer for time keeping. On success + * initializes gd->timer so that lib/timer can use it for future + * referrence. + * + * @return - 0 on success or error number + */ +int dm_timer_init(void); + +/* * timer_conv_64 - convert 32-bit counter value to 64-bit * * @count: 32-bit counter value diff --git a/lib/time.c b/lib/time.c index f37a662..e9f6861 100644 --- a/lib/time.c +++ b/lib/time.c @@ -41,23 +41,6 @@ extern unsigned long __weak timer_read_counter(void); #endif

#ifdef CONFIG_TIMER -static int notrace dm_timer_init(void) -{ - struct udevice *dev; - int ret; - - if (!gd->timer) { - ret = uclass_first_device(UCLASS_TIMER, &dev); - if (ret) - return ret; - if (!dev) - return -ENODEV; - gd->timer = dev; - } - - return 0; -} - ulong notrace get_tbclk(void) { int ret;