linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
@ 2012-05-15 23:35 Jon Hunter
  2012-05-16  9:28 ` Cousson, Benoit
  2012-05-16 14:37 ` Santosh Shilimkar
  0 siblings, 2 replies; 13+ messages in thread
From: Jon Hunter @ 2012-05-15 23:35 UTC (permalink / raw)
  To: linux-omap; +Cc: Tony Lindgren, Benoit Cousson, Jon Hunter

From: Jon Hunter <jon-hunter@ti.com>

In order to migrate the dmtimer driver to support device-tree I found that it
was first necessary to clean-up the timer platform data. The goal of this
series is to simplify the timer platform data structure from ...

struct dmtimer_platform_data {
	int (*set_timer_src)(struct platform_device *pdev, int source);
	int timer_ip_version;
	u32 needs_manual_reset:1;
	bool reserved;
	bool loses_context;
	int (*get_context_loss_count)(struct device *dev);
};

to ...

struct dmtimer_platform_data {
	int (*set_timer_src)(struct platform_device *pdev, int source);
	u32 timer_capability;
};

... where timer_capability is a bit mask that indicates the timer features
supported and uses the HWMOD timer capabilities flags described in
plat/dmtimer.h. For OMAP2+ devices this allows us to read the timer
capabilities from the HWMOD data and for OMAP1 devices the flags are simply
populated by the timer initialisation code. Eventually, the aim is to read the
timer capabilities from the device tree blob.

This series includes some fixes as well as clean-up. If it is preferred to split
the series into fixes and clean-up I can do that, but wanted to get some
feedback on this approach.

This series is based upon the current linux-omap master branch. I have built
both omap1 and omap2plus configurations as well as booted the respective kernels
on the omap5912 OSK (omap1), omap2430 SDP, OMAP3430 Beagle and OMAP4460 PANDA.

Jon Hunter (9):
  ARM: OMAP: Remove unnecessary clk structure
  ARM: OMAP2+: Remove unused max number of timers definition
  ARM: OMAP2+: Add dmtimer platform function to reserve systimers
  ARM: OMAP: Represent timer features using HWMOD flags
  ARM: OMAP2+: HWMOD: Correct timer device attributes
  ARM: OMAP2+: Fix external clock support for dmtimers
  ARM: OMAP: Remove loses_context variable from timer platform data
  ARM: OMAP: Remove timer function pointer for context loss counter
  ARM: OMAP: Add flag to indicate if a timer needs a manual reset

 arch/arm/mach-omap1/timer.c                        |    3 +-
 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |   24 ++++++----
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   10 +----
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 --
 arch/arm/mach-omap2/timer.c                        |   22 ++-------
 arch/arm/plat-omap/dmtimer.c                       |   49 ++++++++++++--------
 arch/arm/plat-omap/include/plat/dmtimer.h          |   21 ++------
 7 files changed, 57 insertions(+), 78 deletions(-)

-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-15 23:35 [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree Jon Hunter
@ 2012-05-16  9:28 ` Cousson, Benoit
  2012-05-16 13:34   ` Jon Hunter
                     ` (2 more replies)
  2012-05-16 14:37 ` Santosh Shilimkar
  1 sibling, 3 replies; 13+ messages in thread
From: Cousson, Benoit @ 2012-05-16  9:28 UTC (permalink / raw)
  To: Jon Hunter; +Cc: linux-omap, Tony Lindgren

Hi Jon,

On 5/16/2012 1:35 AM, Jon Hunter wrote:
> From: Jon Hunter<jon-hunter@ti.com>
>
> In order to migrate the dmtimer driver to support device-tree I found that it
> was first necessary to clean-up the timer platform data. The goal of this
> series is to simplify the timer platform data structure from ...
>
> struct dmtimer_platform_data {
> 	int (*set_timer_src)(struct platform_device *pdev, int source);
> 	int timer_ip_version;
> 	u32 needs_manual_reset:1;
> 	bool reserved;
> 	bool loses_context;
> 	int (*get_context_loss_count)(struct device *dev);
> };
>
> to ...
>
> struct dmtimer_platform_data {
> 	int (*set_timer_src)(struct platform_device *pdev, int source);

I guess that custom set_timer_src should not be there at all anymore. 
Well at least for OMAP2+.
We should just use the regular clock API to change the parent. I do not 
see why we should add that wrapper on top of the clock API and thus 
store some internal clock name inside the timer device init code.


Regards,
Benoit


> 	u32 timer_capability;
> };
>
> ... where timer_capability is a bit mask that indicates the timer features
> supported and uses the HWMOD timer capabilities flags described in
> plat/dmtimer.h. For OMAP2+ devices this allows us to read the timer
> capabilities from the HWMOD data and for OMAP1 devices the flags are simply
> populated by the timer initialisation code. Eventually, the aim is to read the
> timer capabilities from the device tree blob.
>
> This series includes some fixes as well as clean-up. If it is preferred to split
> the series into fixes and clean-up I can do that, but wanted to get some
> feedback on this approach.
>
> This series is based upon the current linux-omap master branch. I have built
> both omap1 and omap2plus configurations as well as booted the respective kernels
> on the omap5912 OSK (omap1), omap2430 SDP, OMAP3430 Beagle and OMAP4460 PANDA.
>
> Jon Hunter (9):
>    ARM: OMAP: Remove unnecessary clk structure
>    ARM: OMAP2+: Remove unused max number of timers definition
>    ARM: OMAP2+: Add dmtimer platform function to reserve systimers
>    ARM: OMAP: Represent timer features using HWMOD flags
>    ARM: OMAP2+: HWMOD: Correct timer device attributes
>    ARM: OMAP2+: Fix external clock support for dmtimers
>    ARM: OMAP: Remove loses_context variable from timer platform data
>    ARM: OMAP: Remove timer function pointer for context loss counter
>    ARM: OMAP: Add flag to indicate if a timer needs a manual reset
>
>   arch/arm/mach-omap1/timer.c                        |    3 +-
>   arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |   24 ++++++----
>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   10 +----
>   arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 --
>   arch/arm/mach-omap2/timer.c                        |   22 ++-------
>   arch/arm/plat-omap/dmtimer.c                       |   49 ++++++++++++--------
>   arch/arm/plat-omap/include/plat/dmtimer.h          |   21 ++------
>   7 files changed, 57 insertions(+), 78 deletions(-)
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16  9:28 ` Cousson, Benoit
@ 2012-05-16 13:34   ` Jon Hunter
  2012-05-17 10:29     ` Cousson, Benoit
  2012-05-16 20:14   ` Jon Hunter
  2012-06-04 14:11   ` Jon Hunter
  2 siblings, 1 reply; 13+ messages in thread
From: Jon Hunter @ 2012-05-16 13:34 UTC (permalink / raw)
  To: Cousson, Benoit; +Cc: linux-omap, Tony Lindgren

Hi Benoit,

On 05/16/2012 04:28 AM, Cousson, Benoit wrote:
> Hi Jon,
> 
> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>> From: Jon Hunter<jon-hunter@ti.com>
>>
>> In order to migrate the dmtimer driver to support device-tree I found
>> that it
>> was first necessary to clean-up the timer platform data. The goal of this
>> series is to simplify the timer platform data structure from ...
>>
>> struct dmtimer_platform_data {
>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>     int timer_ip_version;
>>     u32 needs_manual_reset:1;
>>     bool reserved;
>>     bool loses_context;
>>     int (*get_context_loss_count)(struct device *dev);
>> };
>>
>> to ...
>>
>> struct dmtimer_platform_data {
>>     int (*set_timer_src)(struct platform_device *pdev, int source);
> 
> I guess that custom set_timer_src should not be there at all anymore.
> Well at least for OMAP2+.
> We should just use the regular clock API to change the parent. I do not
> see why we should add that wrapper on top of the clock API and thus
> store some internal clock name inside the timer device init code.

Yes I really wanted to eliminate that function pointer too, but it was a
little trickier. The OMAP2+ code does use the clock framework to switch
the parent already, but the problem is that the OMAP1 code does not. So
we cannot have a common function for OMAP1 and OMAP2+.

One option would be to move the OMAP2+ function into the dmtimer because
it already uses the clock framework and then only populate the function
pointer for OMAP1. However, I admit this is ugly.

Let me look at this some more to see what I can do. I can at least test
on my old omap1 board now for prototyping :-)

Did you look at the rest of the series? Let me know if you are ok with
the approach and have any concerns on my hwmod changes/fix-ups ;-)

Cheers
Jon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-15 23:35 [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree Jon Hunter
  2012-05-16  9:28 ` Cousson, Benoit
@ 2012-05-16 14:37 ` Santosh Shilimkar
  1 sibling, 0 replies; 13+ messages in thread
From: Santosh Shilimkar @ 2012-05-16 14:37 UTC (permalink / raw)
  To: Jon Hunter
  Cc: linux-omap, Tony Lindgren, Benoit Cousson, Tarun Kanti DebBarma

+ Tarun for any comments

On Wednesday 16 May 2012 05:05 AM, Jon Hunter wrote:
> From: Jon Hunter <jon-hunter@ti.com>
> 
> In order to migrate the dmtimer driver to support device-tree I found that it
> was first necessary to clean-up the timer platform data. The goal of this
> series is to simplify the timer platform data structure from ...
> 
> struct dmtimer_platform_data {
> 	int (*set_timer_src)(struct platform_device *pdev, int source);
> 	int timer_ip_version;
> 	u32 needs_manual_reset:1;
> 	bool reserved;
> 	bool loses_context;
> 	int (*get_context_loss_count)(struct device *dev);
> };
> 
> to ...
> 
> struct dmtimer_platform_data {
> 	int (*set_timer_src)(struct platform_device *pdev, int source);
> 	u32 timer_capability;
> };
> 
> ... where timer_capability is a bit mask that indicates the timer features
> supported and uses the HWMOD timer capabilities flags described in
> plat/dmtimer.h. For OMAP2+ devices this allows us to read the timer
> capabilities from the HWMOD data and for OMAP1 devices the flags are simply
> populated by the timer initialisation code. Eventually, the aim is to read the
> timer capabilities from the device tree blob.
> 
> This series includes some fixes as well as clean-up. If it is preferred to split
> the series into fixes and clean-up I can do that, but wanted to get some
> feedback on this approach.
> 
> This series is based upon the current linux-omap master branch. I have built
> both omap1 and omap2plus configurations as well as booted the respective kernels
> on the omap5912 OSK (omap1), omap2430 SDP, OMAP3430 Beagle and OMAP4460 PANDA.
> 
> Jon Hunter (9):
>   ARM: OMAP: Remove unnecessary clk structure
>   ARM: OMAP2+: Remove unused max number of timers definition
>   ARM: OMAP2+: Add dmtimer platform function to reserve systimers
>   ARM: OMAP: Represent timer features using HWMOD flags
>   ARM: OMAP2+: HWMOD: Correct timer device attributes
>   ARM: OMAP2+: Fix external clock support for dmtimers
>   ARM: OMAP: Remove loses_context variable from timer platform data
>   ARM: OMAP: Remove timer function pointer for context loss counter
>   ARM: OMAP: Add flag to indicate if a timer needs a manual reset
> 
>  arch/arm/mach-omap1/timer.c                        |    3 +-
>  arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |   24 ++++++----
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   10 +----
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 --
>  arch/arm/mach-omap2/timer.c                        |   22 ++-------
>  arch/arm/plat-omap/dmtimer.c                       |   49 ++++++++++++--------
>  arch/arm/plat-omap/include/plat/dmtimer.h          |   21 ++------
>  7 files changed, 57 insertions(+), 78 deletions(-)
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16  9:28 ` Cousson, Benoit
  2012-05-16 13:34   ` Jon Hunter
@ 2012-05-16 20:14   ` Jon Hunter
  2012-05-16 23:30     ` Paul Walmsley
  2012-05-17  5:07     ` DebBarma, Tarun Kanti
  2012-06-04 14:11   ` Jon Hunter
  2 siblings, 2 replies; 13+ messages in thread
From: Jon Hunter @ 2012-05-16 20:14 UTC (permalink / raw)
  To: Cousson, Benoit, Paul Walmsley
  Cc: linux-omap, Tony Lindgren, DebBarma, Tarun Kanti

Hi Benoit,

On 05/16/2012 04:28 AM, Cousson, Benoit wrote:
> Hi Jon,
> 
> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>> From: Jon Hunter<jon-hunter@ti.com>
>>
>> In order to migrate the dmtimer driver to support device-tree I found
>> that it
>> was first necessary to clean-up the timer platform data. The goal of this
>> series is to simplify the timer platform data structure from ...
>>
>> struct dmtimer_platform_data {
>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>     int timer_ip_version;
>>     u32 needs_manual_reset:1;
>>     bool reserved;
>>     bool loses_context;
>>     int (*get_context_loss_count)(struct device *dev);
>> };
>>
>> to ...
>>
>> struct dmtimer_platform_data {
>>     int (*set_timer_src)(struct platform_device *pdev, int source);
> 
> I guess that custom set_timer_src should not be there at all anymore.
> Well at least for OMAP2+.
> We should just use the regular clock API to change the parent. I do not
> see why we should add that wrapper on top of the clock API and thus
> store some internal clock name inside the timer device init code.

I have been looking into this and in order to get rid for the above
function pointer we would need to move at a minimum the following
functions from omap-mach2/clkt_clksel.c into the platform code.

_get_clksel_by_parent()
_get_div_and_fieldval()
_write_clksel_reg()
omap2_init_clksel_parent()
omap2_clksel_set_parent()

However, it may be simpler just to move the clkt_clksel.c file
completely. I have tested the above functions on omap1 and they are
working well. However, before doing this we would need to get Paul's
buy-in that this is the right thing to do.

Paul, do you have any thoughts on this? We were trying to see if we
could eliminate the dmtimer function pointer for setting the timer clock
source.

Also, the only other minor issue I see is that for omap1 devices instead
of having "sys_ck" as the name the clock name is "armxor_ck". We cannot
rename armxor_ck as it is used by many peripherals but we could use a
#define to workaround this or add a dummy clock node.

Cheers
Jon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16 20:14   ` Jon Hunter
@ 2012-05-16 23:30     ` Paul Walmsley
  2012-05-17 15:56       ` Jon Hunter
  2012-05-17  5:07     ` DebBarma, Tarun Kanti
  1 sibling, 1 reply; 13+ messages in thread
From: Paul Walmsley @ 2012-05-16 23:30 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Cousson, Benoit, linux-omap, Tony Lindgren, DebBarma, Tarun Kanti

Hello Jon,

On Wed, 16 May 2012, Jon Hunter wrote:

> I have been looking into this and in order to get rid for the above
> function pointer we would need to move at a minimum the following
> functions from omap-mach2/clkt_clksel.c into the platform code.

By platform code, do you mean arch/arm/plat-omap?

> _get_clksel_by_parent()
> _get_div_and_fieldval()
> _write_clksel_reg()
> omap2_init_clksel_parent()
> omap2_clksel_set_parent()
> 
> However, it may be simpler just to move the clkt_clksel.c file
> completely. I have tested the above functions on omap1 and they are
> working well. However, before doing this we would need to get Paul's
> buy-in that this is the right thing to do.
> 
> Paul, do you have any thoughts on this? We were trying to see if we
> could eliminate the dmtimer function pointer for setting the timer clock
> source.

So, just to see if I'm understanding you correctly, you are planning to 
implement clksel clocks in the clock framework for OMAP1, and then use 
clk_set_parent() in the DMTIMER driver ?

If so, then yes, that sounds like the right thing to do.

Eventually both OMAP2+ and OMAP1 will presumably switch to the common 
clock framework, once things settle down a bit and can be tested.

> Also, the only other minor issue I see is that for omap1 devices instead
> of having "sys_ck" as the name the clock name is "armxor_ck". We cannot
> rename armxor_ck as it is used by many peripherals but we could use a
> #define to workaround this or add a dummy clock node.

OMAP1 clockfw uses clkdev just like OMAP2+, so you should be able to 
define as many clock aliases as you want in mach-omap1/clock_data.c.


- Paul

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16 20:14   ` Jon Hunter
  2012-05-16 23:30     ` Paul Walmsley
@ 2012-05-17  5:07     ` DebBarma, Tarun Kanti
  2012-05-17 16:00       ` Jon Hunter
  1 sibling, 1 reply; 13+ messages in thread
From: DebBarma, Tarun Kanti @ 2012-05-17  5:07 UTC (permalink / raw)
  To: Jon Hunter; +Cc: Cousson, Benoit, Paul Walmsley, linux-omap, Tony Lindgren

On Thu, May 17, 2012 at 1:44 AM, Jon Hunter <jon-hunter@ti.com> wrote:
> Hi Benoit,
>
> On 05/16/2012 04:28 AM, Cousson, Benoit wrote:
>> Hi Jon,
>>
>> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>>> From: Jon Hunter<jon-hunter@ti.com>
>>>
>>> In order to migrate the dmtimer driver to support device-tree I found
>>> that it
>>> was first necessary to clean-up the timer platform data. The goal of this
>>> series is to simplify the timer platform data structure from ...
>>>
>>> struct dmtimer_platform_data {
>>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>>     int timer_ip_version;
>>>     u32 needs_manual_reset:1;
>>>     bool reserved;
>>>     bool loses_context;
>>>     int (*get_context_loss_count)(struct device *dev);
>>> };
>>>
>>> to ...
>>>
>>> struct dmtimer_platform_data {
>>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>
>> I guess that custom set_timer_src should not be there at all anymore.
>> Well at least for OMAP2+.
>> We should just use the regular clock API to change the parent. I do not
>> see why we should add that wrapper on top of the clock API and thus
>> store some internal clock name inside the timer device init code.
Whatever is done within omap2_dm_timer_set_src() in mach-omap2/timer.c
can be implemented inside omap_dm_timer_set_source() in plat-omap/dmtimer.c
directly whereby we continue to use the generic clock APIs provided in
include/linux/clk.h.

>
> I have been looking into this and in order to get rid for the above
> function pointer we would need to move at a minimum the following
> functions from omap-mach2/clkt_clksel.c into the platform code.
>
> _get_clksel_by_parent()
> _get_div_and_fieldval()
> _write_clksel_reg()
> omap2_init_clksel_parent()
> omap2_clksel_set_parent()
>
> However, it may be simpler just to move the clkt_clksel.c file
> completely. I have tested the above functions on omap1 and they are
> working well. However, before doing this we would need to get Paul's
> buy-in that this is the right thing to do.
I am not sure if this is really needed though.
--
Tarun
>
> Paul, do you have any thoughts on this? We were trying to see if we
> could eliminate the dmtimer function pointer for setting the timer clock
> source.
>
> Also, the only other minor issue I see is that for omap1 devices instead
> of having "sys_ck" as the name the clock name is "armxor_ck". We cannot
> rename armxor_ck as it is used by many peripherals but we could use a
> #define to workaround this or add a dummy clock node.
>
> Cheers
> Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16 13:34   ` Jon Hunter
@ 2012-05-17 10:29     ` Cousson, Benoit
  0 siblings, 0 replies; 13+ messages in thread
From: Cousson, Benoit @ 2012-05-17 10:29 UTC (permalink / raw)
  To: Jon Hunter; +Cc: linux-omap, Tony Lindgren

On 5/16/2012 3:34 PM, Jon Hunter wrote:
> Hi Benoit,
>
> On 05/16/2012 04:28 AM, Cousson, Benoit wrote:
>> Hi Jon,
>>
>> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>>> From: Jon Hunter<jon-hunter@ti.com>
>>>
>>> In order to migrate the dmtimer driver to support device-tree I found
>>> that it
>>> was first necessary to clean-up the timer platform data. The goal of this
>>> series is to simplify the timer platform data structure from ...
>>>
>>> struct dmtimer_platform_data {
>>>      int (*set_timer_src)(struct platform_device *pdev, int source);
>>>      int timer_ip_version;
>>>      u32 needs_manual_reset:1;
>>>      bool reserved;
>>>      bool loses_context;
>>>      int (*get_context_loss_count)(struct device *dev);
>>> };
>>>
>>> to ...
>>>
>>> struct dmtimer_platform_data {
>>>      int (*set_timer_src)(struct platform_device *pdev, int source);
>>
>> I guess that custom set_timer_src should not be there at all anymore.
>> Well at least for OMAP2+.
>> We should just use the regular clock API to change the parent. I do not
>> see why we should add that wrapper on top of the clock API and thus
>> store some internal clock name inside the timer device init code.
>
> Yes I really wanted to eliminate that function pointer too, but it was a
> little trickier. The OMAP2+ code does use the clock framework to switch
> the parent already, but the problem is that the OMAP1 code does not. So
> we cannot have a common function for OMAP1 and OMAP2+.
>
> One option would be to move the OMAP2+ function into the dmtimer because
> it already uses the clock framework and then only populate the function
> pointer for OMAP1. However, I admit this is ugly.
>
> Let me look at this some more to see what I can do. I can at least test
> on my old omap1 board now for prototyping :-)
>
> Did you look at the rest of the series? Let me know if you are ok with
> the approach and have any concerns on my hwmod changes/fix-ups ;-)

No, I'll keep that for next week, after my vacation :-)

Regards,
Benoit

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16 23:30     ` Paul Walmsley
@ 2012-05-17 15:56       ` Jon Hunter
  2012-05-17 16:48         ` Paul Walmsley
  0 siblings, 1 reply; 13+ messages in thread
From: Jon Hunter @ 2012-05-17 15:56 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Cousson, Benoit, linux-omap, Tony Lindgren, DebBarma, Tarun Kanti

Hi Paul,

On 05/16/2012 06:30 PM, Paul Walmsley wrote:
> Hello Jon,
> 
> On Wed, 16 May 2012, Jon Hunter wrote:
> 
>> I have been looking into this and in order to get rid for the above
>> function pointer we would need to move at a minimum the following
>> functions from omap-mach2/clkt_clksel.c into the platform code.
> 
> By platform code, do you mean arch/arm/plat-omap?

Yes, exactly.

>> _get_clksel_by_parent()
>> _get_div_and_fieldval()
>> _write_clksel_reg()
>> omap2_init_clksel_parent()
>> omap2_clksel_set_parent()
>>
>> However, it may be simpler just to move the clkt_clksel.c file
>> completely. I have tested the above functions on omap1 and they are
>> working well. However, before doing this we would need to get Paul's
>> buy-in that this is the right thing to do.
>>
>> Paul, do you have any thoughts on this? We were trying to see if we
>> could eliminate the dmtimer function pointer for setting the timer clock
>> source.
> 
> So, just to see if I'm understanding you correctly, you are planning to 
> implement clksel clocks in the clock framework for OMAP1, and then use 
> clk_set_parent() in the DMTIMER driver ?
> 
> If so, then yes, that sounds like the right thing to do.

Yes that's right. What is your preference here, the options are ...

1. Move the clkt_clksel.c file to arch/arm/plat-omap and change the
omap2_xxx API names to omap_xxx.
2. Add the functions in clkt_clksel.c to arch/arm/plat-omap/clock.c and
get rid of clkt_clksel.c altogether.

> Eventually both OMAP2+ and OMAP1 will presumably switch to the common 
> clock framework, once things settle down a bit and can be tested.
> 
>> Also, the only other minor issue I see is that for omap1 devices instead
>> of having "sys_ck" as the name the clock name is "armxor_ck". We cannot
>> rename armxor_ck as it is used by many peripherals but we could use a
>> #define to workaround this or add a dummy clock node.
> 
> OMAP1 clockfw uses clkdev just like OMAP2+, so you should be able to 
> define as many clock aliases as you want in mach-omap1/clock_data.c.

Yes, I saw that I have been adding the timer clocks to the OMAP1
clock_data.c. So it is working well, I just need a little guidance on
the appropriate way to move the clksel functions.

Thanks!
Jon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-17  5:07     ` DebBarma, Tarun Kanti
@ 2012-05-17 16:00       ` Jon Hunter
  0 siblings, 0 replies; 13+ messages in thread
From: Jon Hunter @ 2012-05-17 16:00 UTC (permalink / raw)
  To: DebBarma, Tarun Kanti
  Cc: Cousson, Benoit, Paul Walmsley, linux-omap, Tony Lindgren

Hi Tarun,

On 05/17/2012 12:07 AM, DebBarma, Tarun Kanti wrote:
> On Thu, May 17, 2012 at 1:44 AM, Jon Hunter <jon-hunter@ti.com> wrote:
>> Hi Benoit,
>>
>> On 05/16/2012 04:28 AM, Cousson, Benoit wrote:
>>> Hi Jon,
>>>
>>> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>>>> From: Jon Hunter<jon-hunter@ti.com>
>>>>
>>>> In order to migrate the dmtimer driver to support device-tree I found
>>>> that it
>>>> was first necessary to clean-up the timer platform data. The goal of this
>>>> series is to simplify the timer platform data structure from ...
>>>>
>>>> struct dmtimer_platform_data {
>>>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>>>     int timer_ip_version;
>>>>     u32 needs_manual_reset:1;
>>>>     bool reserved;
>>>>     bool loses_context;
>>>>     int (*get_context_loss_count)(struct device *dev);
>>>> };
>>>>
>>>> to ...
>>>>
>>>> struct dmtimer_platform_data {
>>>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>>
>>> I guess that custom set_timer_src should not be there at all anymore.
>>> Well at least for OMAP2+.
>>> We should just use the regular clock API to change the parent. I do not
>>> see why we should add that wrapper on top of the clock API and thus
>>> store some internal clock name inside the timer device init code.
> Whatever is done within omap2_dm_timer_set_src() in mach-omap2/timer.c
> can be implemented inside omap_dm_timer_set_source() in plat-omap/dmtimer.c
> directly whereby we continue to use the generic clock APIs provided in
> include/linux/clk.h.

Have you looked at the OMAP1 code for this?

Today it is not using the clock framework at all. So first we need to
change the OMAP1 code to use the clock framework for dmtimers and then
we can move the function.

>> I have been looking into this and in order to get rid for the above
>> function pointer we would need to move at a minimum the following
>> functions from omap-mach2/clkt_clksel.c into the platform code.
>>
>> _get_clksel_by_parent()
>> _get_div_and_fieldval()
>> _write_clksel_reg()
>> omap2_init_clksel_parent()
>> omap2_clksel_set_parent()
>>
>> However, it may be simpler just to move the clkt_clksel.c file
>> completely. I have tested the above functions on omap1 and they are
>> working well. However, before doing this we would need to get Paul's
>> buy-in that this is the right thing to do.
> I am not sure if this is really needed though.

It is needed so that OMAP1 can use the clksel functions and so we can
migrate OMAP1 and OMAP2+ to use a common function for changing the
parent clock.

Cheers
Jon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-17 15:56       ` Jon Hunter
@ 2012-05-17 16:48         ` Paul Walmsley
  2012-05-22 20:33           ` Jon Hunter
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Walmsley @ 2012-05-17 16:48 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Cousson, Benoit, linux-omap, Tony Lindgren, DebBarma, Tarun Kanti

On Thu, 17 May 2012, Jon Hunter wrote:

> Yes that's right. What is your preference here, the options are ...
> 
> 1. Move the clkt_clksel.c file to arch/arm/plat-omap and change the
> omap2_xxx API names to omap_xxx.
> 2. Add the functions in clkt_clksel.c to arch/arm/plat-omap/clock.c and
> get rid of clkt_clksel.c altogether.

#1.


- Paul

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-17 16:48         ` Paul Walmsley
@ 2012-05-22 20:33           ` Jon Hunter
  0 siblings, 0 replies; 13+ messages in thread
From: Jon Hunter @ 2012-05-22 20:33 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Cousson, Benoit, linux-omap, Tony Lindgren, DebBarma, Tarun Kanti

Hi Tony, Paul,

On 05/17/2012 11:48 AM, Paul Walmsley wrote:
> On Thu, 17 May 2012, Jon Hunter wrote:
> 
>> Yes that's right. What is your preference here, the options are ...
>>
>> 1. Move the clkt_clksel.c file to arch/arm/plat-omap and change the
>> omap2_xxx API names to omap_xxx.
>> 2. Add the functions in clkt_clksel.c to arch/arm/plat-omap/clock.c and
>> get rid of clkt_clksel.c altogether.
> 
> #1.

I have posted a new series here [1] to fix omap1 dmtimers and commonise
the code to set dmtimer source clock between omap1 and omap2.

My plan is to rebase this series on top of that, if you are ok with
those changes.

[1] http://marc.info/?l=linux-omap&m=133771799505501&w=2

Cheers
Jon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree
  2012-05-16  9:28 ` Cousson, Benoit
  2012-05-16 13:34   ` Jon Hunter
  2012-05-16 20:14   ` Jon Hunter
@ 2012-06-04 14:11   ` Jon Hunter
  2 siblings, 0 replies; 13+ messages in thread
From: Jon Hunter @ 2012-06-04 14:11 UTC (permalink / raw)
  To: Cousson, Benoit; +Cc: linux-omap, Tony Lindgren

Hi Benoit,

On 05/16/2012 04:28 AM, Cousson, Benoit wrote:
> Hi Jon,
> 
> On 5/16/2012 1:35 AM, Jon Hunter wrote:
>> From: Jon Hunter<jon-hunter@ti.com>
>>
>> In order to migrate the dmtimer driver to support device-tree I found
>> that it
>> was first necessary to clean-up the timer platform data. The goal of this
>> series is to simplify the timer platform data structure from ...
>>
>> struct dmtimer_platform_data {
>>     int (*set_timer_src)(struct platform_device *pdev, int source);
>>     int timer_ip_version;
>>     u32 needs_manual_reset:1;
>>     bool reserved;
>>     bool loses_context;
>>     int (*get_context_loss_count)(struct device *dev);
>> };
>>
>> to ...
>>
>> struct dmtimer_platform_data {
>>     int (*set_timer_src)(struct platform_device *pdev, int source);
> 
> I guess that custom set_timer_src should not be there at all anymore.
> Well at least for OMAP2+.
> We should just use the regular clock API to change the parent. I do not
> see why we should add that wrapper on top of the clock API and thus
> store some internal clock name inside the timer device init code.

I was trying to remove the set_timer_src for all omap devices, but my
series to fix this for omap1, is conflicting with Rajendra's omap2
common clock framework. So for now, I will workaround this by only using
this function pointer for omap1 devices. Then eventually if we can move
omap1 to the common clock framework we can remove it completely.
Otherwise it may take a long timer to get DT dmtimer support added for
OMAP2.

Cheers
Jon

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-06-04 14:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-15 23:35 [PATCH 0/9] ARM: OMAP: DMTIMER clean-up in preparation for device-tree Jon Hunter
2012-05-16  9:28 ` Cousson, Benoit
2012-05-16 13:34   ` Jon Hunter
2012-05-17 10:29     ` Cousson, Benoit
2012-05-16 20:14   ` Jon Hunter
2012-05-16 23:30     ` Paul Walmsley
2012-05-17 15:56       ` Jon Hunter
2012-05-17 16:48         ` Paul Walmsley
2012-05-22 20:33           ` Jon Hunter
2012-05-17  5:07     ` DebBarma, Tarun Kanti
2012-05-17 16:00       ` Jon Hunter
2012-06-04 14:11   ` Jon Hunter
2012-05-16 14:37 ` Santosh Shilimkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).