| Message ID | a5951a033e56dac0b367045f950973bc401ca624.1380539729.git.marcel@ziswiler.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi Marcel, On Mon, 30 Sep 2013 13:26:06 +0200, Marcel Ziswiler <marcel@ziswiler.com> wrote: > The anadig_reg structure started at the wrong offset (fixed by adding > resvA[4]), was missing some reserved field required for alignment > purpose (resvB[3] between pll4_denom and pll6_ctrl) and further > contained too short a reserved field causing further miss-alignment > (resv10[7]). > > Discovered and tested by temporarily putting the following debug > instrumentation into board_init(): > struct anadig_reg *anadig = (struct anadig_reg *)ANADIG_BASE_ADDR; > printf("&anadig->pll3_ctrl=0x%p\n", &anadig->pll3_ctrl); > printf("&anadig->pll5_ctrl=0x%p\n", &anadig->pll5_ctrl); > > Signed-off-by: Marcel Ziswiler <marcel@ziswiler.com> > --- > arch/arm/include/asm/arch-vf610/crm_regs.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/include/asm/arch-vf610/crm_regs.h b/arch/arm/include/asm/arch-vf610/crm_regs.h > index 85f1fda..57a0242 100644 > --- a/arch/arm/include/asm/arch-vf610/crm_regs.h > +++ b/arch/arm/include/asm/arch-vf610/crm_regs.h > @@ -55,6 +55,7 @@ struct ccm_reg { > > /* Analog components control digital interface (ANADIG) */ > struct anadig_reg { > + u32 resvA[4]; Can you name reserved fields based on their byte or register index offset? > u32 pll3_ctrl; > u32 resv0[3]; > u32 pll7_ctrl; > @@ -72,12 +73,13 @@ struct anadig_reg { > u32 pll4_num; > u32 resv7[3]; > u32 pll4_denom; > + u32 resvB[3]; > u32 pll6_ctrl; > u32 resv8[3]; > u32 pll6_num; > u32 resv9[3]; > u32 pll6_denom; > - u32 resv10[3]; > + u32 resv10[7]; > u32 pll5_ctrl; > u32 resv11[3]; > u32 pll3_pfd; Amicalement,
On Thu, 17 Oct 2013 13:20:09 +0200, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Marcel, > > On Mon, 30 Sep 2013 13:26:06 +0200, Marcel Ziswiler > <marcel@ziswiler.com> wrote: > > > The anadig_reg structure started at the wrong offset (fixed by adding > > resvA[4]), was missing some reserved field required for alignment > > purpose (resvB[3] between pll4_denom and pll6_ctrl) and further > > contained too short a reserved field causing further miss-alignment > > (resv10[7]). > > > > Discovered and tested by temporarily putting the following debug > > instrumentation into board_init(): > > struct anadig_reg *anadig = (struct anadig_reg *)ANADIG_BASE_ADDR; > > printf("&anadig->pll3_ctrl=0x%p\n", &anadig->pll3_ctrl); > > printf("&anadig->pll5_ctrl=0x%p\n", &anadig->pll5_ctrl); > > > > Signed-off-by: Marcel Ziswiler <marcel@ziswiler.com> > > --- > > arch/arm/include/asm/arch-vf610/crm_regs.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/include/asm/arch-vf610/crm_regs.h b/arch/arm/include/asm/arch-vf610/crm_regs.h > > index 85f1fda..57a0242 100644 > > --- a/arch/arm/include/asm/arch-vf610/crm_regs.h > > +++ b/arch/arm/include/asm/arch-vf610/crm_regs.h > > @@ -55,6 +55,7 @@ struct ccm_reg { > > > > /* Analog components control digital interface (ANADIG) */ > > struct anadig_reg { > > + u32 resvA[4]; > > Can you name reserved fields based on their byte or register > index offset? Scratch that. :) Instead, just merge 01 and 02 together. Amicalement,
diff --git a/arch/arm/include/asm/arch-vf610/crm_regs.h b/arch/arm/include/asm/arch-vf610/crm_regs.h index 85f1fda..57a0242 100644 --- a/arch/arm/include/asm/arch-vf610/crm_regs.h +++ b/arch/arm/include/asm/arch-vf610/crm_regs.h @@ -55,6 +55,7 @@ struct ccm_reg { /* Analog components control digital interface (ANADIG) */ struct anadig_reg { + u32 resvA[4]; u32 pll3_ctrl; u32 resv0[3]; u32 pll7_ctrl; @@ -72,12 +73,13 @@ struct anadig_reg { u32 pll4_num; u32 resv7[3]; u32 pll4_denom; + u32 resvB[3]; u32 pll6_ctrl; u32 resv8[3]; u32 pll6_num; u32 resv9[3]; u32 pll6_denom; - u32 resv10[3]; + u32 resv10[7]; u32 pll5_ctrl; u32 resv11[3]; u32 pll3_pfd;
The anadig_reg structure started at the wrong offset (fixed by adding resvA[4]), was missing some reserved field required for alignment purpose (resvB[3] between pll4_denom and pll6_ctrl) and further contained too short a reserved field causing further miss-alignment (resv10[7]). Discovered and tested by temporarily putting the following debug instrumentation into board_init(): struct anadig_reg *anadig = (struct anadig_reg *)ANADIG_BASE_ADDR; printf("&anadig->pll3_ctrl=0x%p\n", &anadig->pll3_ctrl); printf("&anadig->pll5_ctrl=0x%p\n", &anadig->pll5_ctrl); Signed-off-by: Marcel Ziswiler <marcel@ziswiler.com> --- arch/arm/include/asm/arch-vf610/crm_regs.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)