diff mbox

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

Message ID 1448613098-1579-3-git-send-email-mugunthanvnm@ti.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Mugunthan V N Nov. 27, 2015, 8:31 a.m. UTC
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        | 34 +++++++++++++++++++++++++++++
 lib/time.c                          |  5 +++++
 3 files changed, 82 insertions(+)
 create mode 100644 doc/device-tree-bindings/chosen.txt

Comments

Simon Glass Nov. 27, 2015, 7:40 p.m. UTC | #1
Hi Mugunthan,

On 27 November 2015 at 00:31, 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        | 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@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;
> +       };
> +
> +       timer2@f00 {
> +               compatible = "vendor,some-uart";
> +               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)
> +{
> +       const void *blob = gd->fdt_blob;
> +       struct udevice *dev;
> +       int node;
> +
> +       if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
> +               /* Check for a chosen timer to be used for tick */
> +               node = fdtdec_get_chosen_node(blob, "tick-timer");
> +               if (node < 0)
> +                       return -ENODEV;

Please add a debug() here to help people diagnose problems.

> +
> +               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)

debug()

> +                                       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();
> +               if (gd->timer)
> +                       return 0;
> +
>                 ret = uclass_first_device(UCLASS_TIMER, &dev);
>                 if (ret)
>                         return ret;

This code should move up into the timer_init() function. So the sequence is:

- look for a device with 'chosen'
- find the first device

Regards,
Simon
Mugunthan V N Nov. 28, 2015, 6:13 a.m. UTC | #2
On Saturday 28 November 2015 01:10 AM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 27 November 2015 at 00:31, 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        | 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@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;
>> +       };
>> +
>> +       timer2@f00 {
>> +               compatible = "vendor,some-uart";
>> +               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)
>> +{
>> +       const void *blob = gd->fdt_blob;
>> +       struct udevice *dev;
>> +       int node;
>> +
>> +       if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
>> +               /* Check for a chosen timer to be used for tick */
>> +               node = fdtdec_get_chosen_node(blob, "tick-timer");
>> +               if (node < 0)
>> +                       return -ENODEV;
> 
> Please add a debug() here to help people diagnose problems.

timer_init is called even before we have serial_init in
common/board_f.c. So we don't have console yet to add debug logs.

> 
>> +
>> +               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)
> 
> debug()
> 
>> +                                       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();
>> +               if (gd->timer)
>> +                       return 0;
>> +
>>                 ret = uclass_first_device(UCLASS_TIMER, &dev);
>>                 if (ret)
>>                         return ret;
> 
> This code should move up into the timer_init() function. So the sequence is:
> 
> - look for a device with 'chosen'
> - find the first device
> 

Will do it in v2.

Regards
Mugunthan V N
Bin Meng Nov. 28, 2015, 8:26 a.m. UTC | #3
Hi Mugunthan,

On Fri, Nov 27, 2015 at 4:31 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        | 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@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;

I believe this is a wrong device tree syntax as timer2 is not a label.

> +       };
> +
> +       timer2@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.

> +{
> +       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.

> +               if (gd->timer)
> +                       return 0;
> +
>                 ret = uclass_first_device(UCLASS_TIMER, &dev);
>                 if (ret)
>                         return ret;
> --

Regards,
Bin
Mugunthan V N Nov. 28, 2015, 11:38 a.m. UTC | #4
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@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        | 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@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;
> 
> I believe this is a wrong device tree syntax as timer2 is not a label.

Will fix it in next version.

> 
>> +       };
>> +
>> +       timer2@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
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.

> 
>> +{
>> +       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
Mugunthan V N
Bin Meng Nov. 28, 2015, 11:46 a.m. UTC | #5
Hi Mugunthan,

On Sat, Nov 28, 2015 at 7:38 PM, Mugunthan V N <mugunthanvnm@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@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        | 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@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;
>>
>> I believe this is a wrong device tree syntax as timer2 is not a label.
>
> Will fix it in next version.
>
>>
>>> +       };
>>> +
>>> +       timer2@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
Mugunthan V N Nov. 29, 2015, 1:16 p.m. UTC | #6
On Saturday 28 November 2015 05:16 PM, Bin Meng wrote:
> Yes, but your patch 01 will break x86. I can prepare a patch for x86
> if you like

Can you send the patch so that I can include it on my next series.

Regards
Mugunthan V N
Bin Meng Dec. 1, 2015, 2:39 a.m. UTC | #7
Hi Mugunthan,

On Sun, Nov 29, 2015 at 9:16 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Saturday 28 November 2015 05:16 PM, Bin Meng wrote:
>> Yes, but your patch 01 will break x86. I can prepare a patch for x86
>> if you like
>
> Can you send the patch so that I can include it on my next series.
>

Sure, I will prepare a patch for x86 soon.

> Regards
> Mugunthan V N

Regards,
Bin
Bin Meng Dec. 2, 2015, 9:29 a.m. UTC | #8
Hi Mugunthan,

On Tue, Dec 1, 2015 at 10:39 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Mugunthan,
>
> On Sun, Nov 29, 2015 at 9:16 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> On Saturday 28 November 2015 05:16 PM, Bin Meng wrote:
>>> Yes, but your patch 01 will break x86. I can prepare a patch for x86
>>> if you like
>>
>> Can you send the patch so that I can include it on my next series.
>>
>
> Sure, I will prepare a patch for x86 soon.
>

Please check my x86 patch @ http://patchwork.ozlabs.org/patch/551259/

Feel free to create your v2 on top of mine, or just include mine in your v2.

Regards,
Bin
Mugunthan V N Dec. 2, 2015, 9:50 a.m. UTC | #9
On Wednesday 02 December 2015 02:59 PM, Bin Meng wrote:
> Hi Mugunthan,
> 
> On Tue, Dec 1, 2015 at 10:39 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Mugunthan,
>>
>> On Sun, Nov 29, 2015 at 9:16 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>> On Saturday 28 November 2015 05:16 PM, Bin Meng wrote:
>>>> Yes, but your patch 01 will break x86. I can prepare a patch for x86
>>>> if you like
>>>
>>> Can you send the patch so that I can include it on my next series.
>>>
>>
>> Sure, I will prepare a patch for x86 soon.
>>
> 
> Please check my x86 patch @ http://patchwork.ozlabs.org/patch/551259/
> 
> Feel free to create your v2 on top of mine, or just include mine in your v2.
> 

Thanks, will pick your patch for v2 series.

Regards
Mugunthan V N
Bin Meng Dec. 10, 2015, 1:48 a.m. UTC | #10
Hi Mugunthan,

On Wed, Dec 2, 2015 at 5:50 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Wednesday 02 December 2015 02:59 PM, Bin Meng wrote:
>> Hi Mugunthan,
>>
>> On Tue, Dec 1, 2015 at 10:39 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Mugunthan,
>>>
>>> On Sun, Nov 29, 2015 at 9:16 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> On Saturday 28 November 2015 05:16 PM, Bin Meng wrote:
>>>>> Yes, but your patch 01 will break x86. I can prepare a patch for x86
>>>>> if you like
>>>>
>>>> Can you send the patch so that I can include it on my next series.
>>>>
>>>
>>> Sure, I will prepare a patch for x86 soon.
>>>
>>
>> Please check my x86 patch @ http://patchwork.ozlabs.org/patch/551259/
>>
>> Feel free to create your v2 on top of mine, or just include mine in your v2.
>>
>
> Thanks, will pick your patch for v2 series.
>

Just let you know my patches are in mainline.

Regards,
Bin
diff mbox

Patch

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@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;
+	};
+
+	timer2@f00 {
+		compatible = "vendor,some-uart";
+		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)
+{
+	const void *blob = gd->fdt_blob;
+	struct udevice *dev;
+	int node;
+
+	if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
+		/* 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();
+		if (gd->timer)
+			return 0;
+
 		ret = uclass_first_device(UCLASS_TIMER, &dev);
 		if (ret)
 			return ret;