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

[Bin Meng]

Hi Mugunthan,

On Thu, Dec 24, 2015 at 6:38 PM, Mugunthan V N <mugunthanvnm at 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 at 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 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..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