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

[Andreas Bießmann]

Dear Reinhard Meyer,

Am 09.06.2011 12:23, schrieb Reinhard Meyer:
> Dear Andreas Bießmann,
>> Dear Reinhard Meyer,
>>
>> Am 09.06.2011 11:38, schrieb Reinhard Meyer:
>>> Dear Andreas Bießmann,

<snip patch header>

>>> 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@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 ;)

no problem, we can do it here ...

>>
>> 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?

Well, if I look at the arm920t/at91 I do see following files:

 timer.c
  timer could not be shared between arm920t and arm926ejs at91 SoC
  cause at91rm9200 do not have a 32 bit PITC!
 reset.c
  reset could not be shared cause at91rm9200 has an input only NRST pin.
  We do need the watchdog workaround to do the reset

A future commit will add cpu.c as in arm926ejs but with less features
(just arch_cpu_init() for at91_clock_init()).

If I look at ar926ejs/at91 files I do see additionally these files:

 eflash.c
  new feature in some sam9XE devices, not related to at91rm9200
 led.c
  I do hate this ... we have dozen of led API ... but it is really
  helpful for assembler (lowlevel_init) stage debugging without JTAG,
  could be useful for at91rm9200
 <SoC>_devices.c
  ...

> For just clock.c, it seems like an overkill...

You are right here ..

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

This makes no sense to me in the current structure. If we have some
arch/arm/mach-XXX like the linux kernel it would make sense to make the
effort to unify those files.

The main aim for this patch was to not duplicate the clock.c from
arm926ejs to arm920t (remember, the file is already prepared for
at92rm9200 cause it is a copy from linux source tree).

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

I see ... Therefore it would be better to rewrite the clock.c in
arm926ejs (remove at91rm9200 support) and arm920t (do not add support
for other SoC).

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

That may be true.

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

We should inline the get_*_clk_rate() functions in header in any case.
That is a good idea.

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

But this needs also an rewrite of the clock.c driver (which was copied
from linux kernel).

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

That could be another solution.

The conclusion is that lib/at91 is not the correct way, cause there are
only a few commonalities between arm920t and arm926ejs. Therefore we have
 a) include new clock.c in arm920t
 b) move current clock.c to driver/clock (or similar)

regards

Andreas Bießmann