Re: [U-Boot] [PATCH-OMAP3] OMAP3: Convert readl/writel and replace hardcoded values

On Mon, Nov 24, 2008 at 6:47 AM, Wolfgang Denk wd@denx.de wrote:

Dear Dirk,

you should really use structures like this:

   struct ecc_control {
           u32 ecc_config;
           u32 ecc_control;
           u32 ecc_size_config;
           u32 ecc1_result;
           u32 ecc2_result;
           u32 ecc3_result;
           u32 ecc4_result;
           u32 ecc5_result;
           u32 ecc6_result;
           u32 ecc7_result;
           u32 ecc8_result;
           u32 ecc9_result;
   };

so that instead of

   writel(0x000, gpmc_base + OFFS(GPMC_ECC_CONFIG));

you can write

   writel(0x000, ptr->ecc_config);

or similar - and the compiler will actually perform type checking.

Wolfgang,

Do you mean to instance the ecc_control struct at a specific memory location and use writel(0x000, &ptr->ecc_config);? If so, why not simply use ptr->ecc_config = 0x0000?

Example:

The AMD sc520 has 4k of Memory Mapped Control Registers (MMCR) that range in size from 8 to 32 bits located at a fixed memory address (0xfffef000)

I am thinking of replacing the read/write_mmcr_byte/word/long () functions into a struct e.g.

struct sc520_mmcr { /* CPU */ u16 revid; u8 cpuctl; u8 pad_0x003[0xd];

/* SDRAM Controller */ u8 drcctl; <etc>

} __attribute__((packed));

struct struct sc520_mmcr* mmcrptr = (volatile struct sc520_mmcr*)0xfffef000;

and replacing: write_mmcr_byte(SC520_DRCCTL, \ (read_mmcr_byte(SC520_DRCCTL) & 0xcf) | (val<<4));

with: mmcrptr->drcctl = (mmcrptr->drcctl & 0xcf) | (val<<4);

Is this appropriate?

Regards,

Graeme