From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aneesh V <aneesh@ti.com>
Date: Tue, 21 Jun 2011 14:38:46 +0530
Subject: [U-Boot] [PATCH v2 13/22] omap4: add clock support
In-Reply-To: <20110621082048.2311611EEDB1@gemini.denx.de>
References: <1298893591-17636-1-git-send-email-aneesh@ti.com>
	<1305472900-4004-14-git-send-email-aneesh@ti.com>
	<4E0030F8.6030104@ti.com> <4E003952.30901@ti.com>
	<4E00447B.9070804@ti.com>
	<20110621082048.2311611EEDB1@gemini.denx.de>
Message-ID: <4E005F9E.8050003@ti.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Tuesday 21 June 2011 01:50 PM, Wolfgang Denk wrote:
> Dear Aneesh,
>
> In message<4E00447B.9070804@ti.com>  you wrote:
>>
>> $ gcc main.c
>> main.c:5: error: initializer element is not constant
>> main.c:5: error: (near initialization for ?arr[0]?)
>> main.c:7: error: initializer element is not constant
>> main.c:7: error: (near initialization for ?arr[1]?)
>
> I have to admit that I don't understand either why this error is
> raised here; after all, from our understanding of the code these _are_
> constant addresses.
>
> You may want to ask this in a compiler group...

Yes. I will.

>
>> As a result, I will have to do something like this to populate my
>> array:
>>
>> static unsigned int *const reg_arr[] = {
>> 	&(((struct my_regs_struct *const)OMAP4_PRCM_REG_BASE)->uart_clkctrl),
>> 	&(((struct my_regs_struct *const)OMAP4_PRCM_REG_BASE)->i2c_clkctrl),
>> };
>>
>> Is this acceptable?
>
> No, please don't.
>
> Note that the following code compiles fine:
>
> ----------------------------- snip -----------------------------
> #include<stdio.h>
>
> struct my_regs_struct {
> 	unsigned int reg1;
> 	unsigned int reg2;
> 	unsigned int reg3;
> };
>
> static struct my_regs_struct *const my_regs = (struct my_regs_struct *) 0x1000;
>
> static void print_regs(void)
> {
> 	unsigned int *const reg_arr[] = {
> 		&my_regs->reg1,
> 		&my_regs->reg3,
> 	};
> 	printf("regs %p %p \n", (void *)reg_arr[0], (void *)reg_arr[1]);
> }

In my function I am using 3 such arrays with quite a few entries in
them. Won't it look ugly besides increasing the stack footprint.

Of course, I can try to break them down to different functions, if need
be.

Or, how about using a utility macro and make it look better like this:

#define OMAP4_PRCM_REG_ADDR(reg)\
	(&(((struct my_regs_struct *)OMAP4_PRCM_BASE)->reg))

static unsigned int *const reg_arr[] = {
     OMAP4_PRCM_REG_ADDR(uart_clkctrl),
     OMAP4_PRCM_REG_ADDR(i2c_clkctrl)
};

This one doesn't generate any warning even with 'gcc -Wall -pedantic'.

Are you not comfortable with getting the address in this manner at all?

best regards,
Aneesh