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

[Bin Meng]

Hi Mugunthan,

On Sat, Nov 28, 2015 at 7:38 PM, Mugunthan V N <mugunthanvnm at ti.com> wrote:
> On Saturday 28 November 2015 01:56 PM, Bin Meng wrote:
>> Hi Mugunthan,
>>
>> On Fri, Nov 27, 2015 at 4:31 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        | 34 +++++++++++++++++++++++++++++
>>>  lib/time.c                          |  5 +++++
>>>  3 files changed, 82 insertions(+)
>>>  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..58f29f9
>>> --- /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;
>>
>> I believe this is a wrong device tree syntax as timer2 is not a label.
>
> Will fix it in next version.
>
>>
>>> +       };
>>> +
>>> +       timer2 at f00 {
>>> +               compatible = "vendor,some-uart";
>>
>> some-timer
>>
>>> +               reg = <0xf00 0x10>;
>>> +       };
>>> +};
>>> diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
>>> index 12aee5b..78ec989 100644
>>> --- a/drivers/timer/timer-uclass.c
>>> +++ b/drivers/timer/timer-uclass.c
>>> @@ -6,9 +6,13 @@
>>>
>>>  #include <common.h>
>>>  #include <dm.h>
>>> +#include <dm/lists.h>
>>> +#include <dm/device-internal.h>
>>>  #include <errno.h>
>>>  #include <timer.h>
>>>
>>> +DECLARE_GLOBAL_DATA_PTR;
>>> +
>>>  /*
>>>   * Implement a Timer uclass to work with lib/time.c. The timer is usually
>>>   * a 32 bits free-running up counter. The get_rate() method is used to get
>>> @@ -35,6 +39,36 @@ unsigned long timer_get_rate(struct udevice *dev)
>>>         return uc_priv->clock_rate;
>>>  }
>>>
>>> +int timer_init(void)
>>
>> This introduces a timer_init() which won't build for x86 as x86 has a
>> timer_init() already.
>
> You mean init_timer() in arch/x86/lib/tsc_timer.c, once x86 timer is
> converted to driver model the init_timer() should be removed. Similar

x86 timer has already been converted to driver model. Please rebase
your series on top of dm/master and you can see the converted driver
there.

> init_timer() is also present in omap as well, but with patch 01 of this
> series timer driver is removed from build when CONFIG_TIMER is defined
> in defconfig.

Yes, but your patch 01 will break x86. I can prepare a patch for x86
if you like.

>
>>
>>> +{
>>> +       const void *blob = gd->fdt_blob;
>>> +       struct udevice *dev;
>>> +       int node;
>>> +
>>> +       if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
>>
>> Why do we need this if check? I think it can be removed.
>>
>>> +               /* Check for a chosen timer to be used for tick */
>>> +               node = fdtdec_get_chosen_node(blob, "tick-timer");
>>> +               if (node < 0)
>>> +                       return -ENODEV;
>>> +
>>> +               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;
>>> +                       }
>>> +               }
>>> +       }
>>> +
>>> +       gd->timer = dev;
>>> +       return 0;
>>> +}
>>> +
>>>  UCLASS_DRIVER(timer) = {
>>>         .id             = UCLASS_TIMER,
>>>         .name           = "timer",
>>> diff --git a/lib/time.c b/lib/time.c
>>> index b001745..22f7d23 100644
>>> --- a/lib/time.c
>>> +++ b/lib/time.c
>>> @@ -47,6 +47,11 @@ static int notrace dm_timer_init(void)
>>>         int ret;
>>>
>>>         if (!gd->timer) {
>>> +               /* Check if we have a chosen timer */
>>> +               timer_init();
>>
>> timer_init() is already called in board_r.c. Can we avoid duplicate
>> calls? Maybe we should remove the call in board_r.c for DM.
>
> from board_r.c, timer_init is added only for MICROBLAZE, AVR32 and M68K
> architectures. May be those who work on these architectures can initiate
> of this removal during timer dm conversion.

Regards,
Bin