[U-Boot] [PATCH v3 02/19] dm: timer: uclass: add timer init to add timer device

[Mugunthan V N <mugunthanvnm@ti.com>]

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 at f00:115200";
>> +       };
>> +
>> +       serial at 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 at f00";
>> +       };
>> +
>> +       timer2 at 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