[U-Boot] [PATCH 1/2] at91: move a/a/c/arm926ejs/at91/clock.c -> a/a/lib/at91

Reinhard Meyer
2011-06-09T10:23:03Z

Dear Andreas Bie?mann,
> Dear Reinhard Meyer,
>
> Am 09.06.2011 11:38, schrieb Reinhard Meyer:
>> Dear Andreas Bie?mann,
>>> The at91/clock.c is copied from linux kernel and has support for both
>>> arm920t and arm926ejs core devices. Therefore this patch moves this
>>> generic at91/clock.c to a new place at arch/arm/lib/at91 to be used from
>>> at91 family devices.
>>>
>>> We build a new libat91-common.o to provide the required symbols to both
>>> cpu types.
>>>
>>> Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
>>> ---
>>> Makefile | 3 +
>>> arch/arm/cpu/arm926ejs/at91/Makefile | 1 -
>>> arch/arm/cpu/arm926ejs/at91/clock.c | 215 ----------------------------------
>>> arch/arm/lib/at91/Makefile | 45 +++++++
>>> arch/arm/lib/at91/clock.c | 215 ++++++++++++++++++++++++++++++++++
>>> 5 files changed, 263 insertions(+), 216 deletions(-)
>>> delete mode 100644 arch/arm/cpu/arm926ejs/at91/clock.c
>>> create mode 100644 arch/arm/lib/at91/Makefile
>>> create mode 100644 arch/arm/lib/at91/clock.c
>>
>> Please use the "git-mv" command to move the file "clock.c".
>
> I did it that way ... but I think there was another trick for
> git-format-patch ... will have a look for that
>
>> On any account, we should discuss about opening a subdir lib scheme first.
>> Currently arch/arm/lib does not have any subdirectories.
>
> Well as described in cover letter this was discussed before. See the
> other discussion on gmane:
> http://mid.gmane.org/BANLkTimn29vmAYGb5csMdCYS-xx6zD_6Zw at mail.gmail.com

Sorry, I had forgotten already about that discussion, it was even
marked "important" in my e-Mail program... and I wanted to comment then
but was too busy with other things ;)

>
> I think this solution was preferred by Albert too. But I'm open for
> another solution, so lets see what possibilities you have.

What other files do you see (in the future) to be in arch/arm/lib/at91?
For just clock.c, it seems like an overkill... Can we (on the long run)
move _all_ files from the at91 subdirs there, like *_devices.c, reset.c,
timer.c, at91sam9_watchdog.c? Then it would make sense to me...

if YES, ignore the following ideas:
{
When I look at the #ifdef's near the end of clock.c I get an uneasy feeling;)

With more Atmel SoCs coming up which must have additional clock domains
(e.g. with 2x EMAC, CAN, etc) I can imagine this file to become more and
more ugly.

I currently have rough ideas to add to the discussion:

1. move the SoC specific clock stuff into <SoC>_devices.c - even if that
means some duplication (the get_*_clk_rate() functions could possibly be
inlined and put into the header file ?)

2. introduce <SoC>_clock.c (if one does not like the clock stuff in "devices")

3. similar to drivers/watchdog we could have drivers/clock and put
at91sam9_clock.c there. On that account, the hardware specific timer
functions of any new timer scheme could be placed there as well.
}

>
>> I _do_ see the problems of the current ARM directory structure which makes
>> the ARM core a more important criteria than SoC family especially in the
>> Atmel context where the SoCs have identical peripheral blocks but can have
>> different ARM cores.
>
> correct

Best Regards,
Reinhard