Hi Wolfgang
2011/3/31 Wolfgang Denk wd@denx.de:
Dear Macpaul Lin,
We should probably split architecture and/or board specific additions like these into separate files in the respectice architecture / board directories. Eventually we add make targets for these, then; for now it's probably sufficient to add some #include to lib/asm-offsets.c
Otherwise lib/asm-offsets.c will quickly become an unreadable mess.
Otherwise this looks OK with me.
There are lots of register offset is widely been used both in lowlevel_init and C environment. If we really want assembly offset of these registers is generated from structures, lots of includes files with OFFSET() marco to generate asm-offsets will be required. For example: in "arch/arm/include/asm/arch-at91/at91sam9_smc.h", there exist the similar problem as ftsmc020.
#ifdef __ASSEMBLY__
#ifndef AT91_SMC_BASE #define AT91_SMC_BASE AT91_SMC0_BASE #endif
#define AT91_ASM_SMC_SETUP0 AT91_SMC_BASE #define AT91_ASM_SMC_PULSE0 (AT91_SMC_BASE + 0x04) #define AT91_ASM_SMC_CYCLE0 (AT91_SMC_BASE + 0x08) #define AT91_ASM_SMC_MODE0 (AT91_SMC_BASE + 0x0C)
#else
typedef struct at91_cs { u32 setup; /* 0x00 SMC Setup Register */ u32 pulse; /* 0x04 SMC Pulse Register */ u32 cycle; /* 0x08 SMC Cycle Register */ u32 mode; /* 0x0C SMC Mode Register */ } at91_cs_t;
typedef struct at91_smc { at91_cs_t cs[8]; } at91_smc_t;
#endif /* __ASSEMBLY__ */
Thus we may need to write some code like OFFSET(AT91_ASM_SMC_SETUP0, at91_cs, setup); some where in the included file then generate the code like. #define AT91_ASM_SMC_SETUP0 (0) /* offsetof(struct at91_cs, setup) */
Even like this, we cannot directly use AT91_ASM_SMC_SETUP0 like the code above in lowlevel_init.c. If there is a code wrote as writel(0x1, AT91_ASM_SMC_SETUP0); originally must be rewrote as writel(0x1, AT91_SMC_BASE + AT91_ASM_SMC_SETUP0);
Hence we really need some scalable rework for the both a split arch/board make-asm-offset directories.
I'm not sure if my thought of this scenario correct?
Keep in mind that I dislike this manual unrolling of the nested structs. It may work in your code, but it is ugly and doesn't scale. Also, it does not allow any kind of looping over the entries which might be needed here and there. I strongly recommend to get rid of these nested declarations.
Ok, since the original structure was commit by other people, I'm still trying to rewrite the original C code.
#define FTSMC020_PAD4 (48) /* offsetof(struct ftsmc020, pad[4]) */ #define FTSMC020_PAD5 (52) /* offsetof(struct ftsmc020, pad[5]) */ #define FTSMC020_PAD6 (56) /* offsetof(struct ftsmc020, pad[6]) */ #define FTSMC020_PAD7 (60) /* offsetof(struct ftsmc020, pad[7]) */ #define FTSMC020_SSR (64) /* offsetof(struct ftsmc020, ssr) */
However, this looks weird. It doesn't look like the other automated generated code.
What exactly looks weird? And what "other automated generated code" do you mean?
I meant, should we make a script that could auto generate asm-offset like a translation from struct ftsmc020 { unsigned bank0_cr; unsigned bank0_tpr; ... } into #define FTSMC020_BANK0_CR 0x00; or #define FTSMC020_BANK0_TPR (4);
without manually reworte the structure in the way as OFFSET(FTSMC020_BANK0_CR, ftsmc020, bank0_cr); and create a new specific header file for make-asm-offset?
Could I move the generated code into ftsmc020.h?
No - what for? This is automatically generated code, that gets used somewhere. No human eye is supposed to have to read it.
You are right, the comments of FTSMC020_BANK0_TPR (4) is really bad for human reading. However, when people implementing lowlevel_init or other assembly files, they still need such kind of reference to write *.S files before the asm-offset has been generated by make.
Thanks.