On Monday 04 November 2002 01:59 pm, Wolfgang Denk wrote:

In message 20021104121905.GC21366@pengutronix.de you wrote:

...

here is an updated patch for the Cogent CSB226 board (PXA250). Some details which might require discussion:

arminfrastructure

• The "reset" code didn't work for me in the version which was in the repository. I've added a reset routine which works on the PXA250, but I suspect the old variant might have run on some other XScale processor? If yes, we should sort it out to some processor specific file, if no my code can simply go in.

Let's keep at least a minimum of technical stuff going on...

Any comments about this patch? Should I merge it?

Looks OK to me. This should work for all Xscale.

On a side note Wolfgang. Have you ever considered supporting the

if (machine_is_board_x()) {}

I know you're busy and this kind of thing is low priority. But it does make code, IMHO, more readable and cleaner.

Kyle.

Wolfgang Denk wrote:

2. Many of these #ifdef's are used to conditionammy define variables, or initializers, where an if() cannot be used, so we would have to support BOTH forms, which is actually worse.

Yep, supporting both methods would be very bad.

IMNSHO,

#ifdef MISC_INIT_R misc_init_r(); #endif

is better. But then you have to get all the board maintainers to define MISC_INIT_R properly.

One could just remove all the 'board specific' defines from this and wait for the patches to arrive... ;-)

Personally, I prefer to have all init functions defined for each supported board and skip all the if'def blocks all together. Either by an empty #define or an empty function.

This makes everything *much* more readable.

Yes, there are exceptions, but (in a perfect world) generic code shouldn't have board specific references at all. Let the linker do it's job.

Just my 2/100th's of a USD.

On Monday 04 November 2002 05:35 pm, Wolfgang Denk wrote:

Well, maybe. If you check the #ifdef-mess we have right now, you will find that

1. many #ifdef's come clustered for seral boards, and I don;t see much difference between

   #if defined(CONFIG_CCM) || \ defined(CONFIG_COGENT) || \ defined(CONFIG_CPCI405) || \ defined(CONFIG_EVB64260) || \ defined(CONFIG_HYMOD) || \ defined(CONFIG_LWMON) || \ defined(CONFIG_PCU_E) || \ defined(CONFIG_W7O) || \ defined(CONFIG_MISC_INIT_R) /* miscellaneous platform dependent initialisations */ misc_init_r (); #endif

Yeh, This should really be replaced with #ifdef CONFIG_MISC_INIT_R

and I think most new ports use the common CONFIG_xxxx. Only some of the early boards use board specific defines for common config options.

and

if (machine_is_CCM() || \ machine_is_COGENT() || \ machine_is_CPCI405() || \ machine_is_EVB64260() || \ machine_is_HYMOD() || \ machine_is_LWMON() || \ machine_is_PCU_E() || \ machine_is_W7O() || \ machine_is_MISC_INIT_R() ) { /* miscellaneous platform dependent initialisations */ misc_init_r (); }

2. Many of these #ifdef's are used to conditionammy define variables, or initializers, where an if() cannot be used, so we would have to support BOTH forms, which is actually worse.

Good point.

Thanks for all the hard work (to everyone, including the armboot folks). U-boot is a great project!

Kyle.

In message 200211050844.07322.h.schurig@mn-logistik.de you wrote:

...

Any comments about this patch? Should I merge it?

Yes, you should. In the redboot sources for the IQ30xxx (an XScale I/O processor) I also read that the device can only be reset by timer reset.

Thanks. Done & checked in.

Best regards,

Wolfgang Denk