Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18.04.2012 19:27, Timo Ketola wrote: But if the board configuration file in include/configs is the correct place to include it, I shall then find the obstacle on that approach... Ok, including asm/arch/imx-regs.h in board configuration file *and* asm/types.h in asm/arch/imx-regs.h file seems to make build happy. This would be the right fix then? -- Timo ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18.04.2012 18:05, Stefano Babic wrote: On 18/04/2012 13:05, Timo Ketola wrote: Stefano Babic wrote: Timo Ketola wrote: PPC seems to use a predefined macro from asm/immap_8xxx.h files. Where is that file included? It is a different way. The board configuration file includes the register description file, so for example immap_86xx.h, immap_85xx.h, Where? I don't see an example. For PPC86xx I can see at least: arch/powerpc/cpu/mpc86xx/mpc8641_serdes.c:#include arch/powerpc/cpu/mpc86xx/mpc8610_serdes.c:#include board/freescale/mpc8610hpcd/mpc8610hpcd.c:#include board/freescale/mpc8641hpcn/mpc8641hpcn.c:#include Yes, I saw those but when you said that board configuration file includes those, I thought that you meant the header files in include/configs. But I see them included in common.h. Should there be also imx-regs? Seems to work if I do so. No, this is wrong. ... Then I tried to include imx-regs.h in fsl_esdhc.c and 'MAKEALL -a arm' was happy. Maybe the right fix is to include imx-regs in common.h? No. common.h, as the name suggests, is for all architectures, not only for i.MX. We cannot fix i:MX and break other boards. But why PPC register description files are included there then? For example line 87: #ifdef CONFIG_MPC86xx #include #include #endif Is that deprecated? And how would adding imx file with the same logic break other boards? I mean, putting there: #if defined(CONFIG_MX25) || defined(CONFIG_MX31) || ... #include #endif But if the board configuration file in include/configs is the correct place to include it, I shall then find the obstacle on that approach... -- Timo ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18/04/2012 13:05, Timo Ketola wrote: >> >> fsl_esdhc.c includes config.h. If your board configuration file includes >> imx-regs.h, as most i.MX boards do, the file is automatically included, >> I suppose. > > I tried that but then: > > .../u-boot-imx/build-exe4026/include/asm/arch/imx-regs.h:43:2: error: > expected specifier-qualifier-list before ‘u32’ > > when compiling > > arch/arm/cpu/arm926ejs/cpu.o Well, I have not said that there cannot be other issues. At first glance you must include asm/types.h, in cpu.c or in imx-regs.h. >>> PPC seems to use a predefined macro from asm/immap_8xxx.h files. Where >>> is that file included? >> >> It is a different way. The board configuration file includes the >> register description file, so for example immap_86xx.h, immap_85xx.h, > > Where? I don't see an example. For PPC86xx I can see at least: arch/powerpc/cpu/mpc86xx/mpc8641_serdes.c:#include arch/powerpc/cpu/mpc86xx/mpc8610_serdes.c:#include board/freescale/mpc8610hpcd/mpc8610hpcd.c:#include board/freescale/mpc8641hpcn/mpc8641hpcn.c:#include > But I see them included in common.h. > Should there be also imx-regs? Seems to work if I do so. No, this is wrong. > >> or >> imx-regs.h, and defines CONFIG_SYS_FSL_ESDHC_ADDR using its own specific >> macro, if any, for example: >> >> #define CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_MPC85xx_ESDHC_ADDR >> >> Why is it not enough for you to set in your board configuration file: >> >> #define CONFIG_SYS_FSL_ESDHC_ADDR IMX_MMC_SDHC1_BASE > > I tried also exactly that, but then: > > fsl_esdhc.c:544:20: error: ‘IMX_MMC_SDHC1_BASE’ undeclared (first use in > this function) ...then imx-regs.h was not included... > > fsl_esdhc.c seems not to see imx-regs.h file. > > Then I tried to include imx-regs.h in fsl_esdhc.c and 'MAKEALL -a arm' > was happy. > > Maybe the right fix is to include imx-regs in common.h? No. common.h, as the name suggests, is for all architectures, not only for i.MX. We cannot fix i:MX and break other boards. Best regards, Stefano Babic -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18.04.2012 13:30, Stefano Babic wrote: On 18/04/2012 11:11, Timo Ketola wrote: Ok, I was afraid about something like that and tried first to include it in board configuration but that broke something else (at least arm926ejs didn't compile any more). By the way, why do you need it if you do not use that macro ? I use it in my board (support of which I'm preparing to send) configuration file and I think it is annoying to write a literal constant there which is already defined in imx-regs.h. fsl_esdhc.c includes config.h. If your board configuration file includes imx-regs.h, as most i.MX boards do, the file is automatically included, I suppose. I tried that but then: .../u-boot-imx/build-exe4026/include/asm/arch/imx-regs.h:43:2: error: expected specifier-qualifier-list before ‘u32’ when compiling arch/arm/cpu/arm926ejs/cpu.o PPC seems to use a predefined macro from asm/immap_8xxx.h files. Where is that file included? It is a different way. The board configuration file includes the register description file, so for example immap_86xx.h, immap_85xx.h, Where? I don't see an example. But I see them included in common.h. Should there be also imx-regs? Seems to work if I do so. or imx-regs.h, and defines CONFIG_SYS_FSL_ESDHC_ADDR using its own specific macro, if any, for example: #define CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_MPC85xx_ESDHC_ADDR Why is it not enough for you to set in your board configuration file: #define CONFIG_SYS_FSL_ESDHC_ADDR IMX_MMC_SDHC1_BASE I tried also exactly that, but then: fsl_esdhc.c:544:20: error: ‘IMX_MMC_SDHC1_BASE’ undeclared (first use in this function) fsl_esdhc.c seems not to see imx-regs.h file. Then I tried to include imx-regs.h in fsl_esdhc.c and 'MAKEALL -a arm' was happy. Maybe the right fix is to include imx-regs in common.h? What would be the right expression for #ifdef? -- Timo ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18/04/2012 11:11, Timo Ketola wrote: > > Ok, I was afraid about something like that and tried first to include it > in board configuration but that broke something else (at least arm926ejs > didn't compile any more). > >> By the way, why do you need it if you do not use that macro ? > > I use it in my board (support of which I'm preparing to send) > configuration file and I think it is annoying to write a literal > constant there which is already defined in imx-regs.h. fsl_esdhc.c includes config.h. If your board configuration file includes imx-regs.h, as most i.MX boards do, the file is automatically included, I suppose. > > PPC seems to use a predefined macro from asm/immap_8xxx.h files. Where > is that file included? It is a different way. The board configuration file includes the register description file, so for example immap_86xx.h, immap_85xx.h, or imx-regs.h, and defines CONFIG_SYS_FSL_ESDHC_ADDR using its own specific macro, if any, for example: #define CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_MPC85xx_ESDHC_ADDR Why is it not enough for you to set in your board configuration file: #define CONFIG_SYS_FSL_ESDHC_ADDR IMX_MMC_SDHC1_BASE Best regards, Stefano Babic -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18.04.2012 11:43, Stefano Babic wrote: On 18/04/2012 09:57, Timo Ketola wrote: One might want to define CONFIG_SYS_FSL_ESDHC_ADDR with the macro already define in imx-regs.h, e.g. with IMX_MMC_SDHC1_BASE. Then the header must be included here. ... diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c ... +#include NAK. There is a good reason to avoid it. The fsl_esdhc driver is common to both i.MX and PowerPc architecture, and of course PowerPC have not imx-regs.h. And CONFIG_SYS_FSL_ESDHC_ADDR cannot be set by a macro in imx-regs.h, because it is different on PowerPC. Ok, I was afraid about something like that and tried first to include it in board configuration but that broke something else (at least arm926ejs didn't compile any more). By the way, why do you need it if you do not use that macro ? I use it in my board (support of which I'm preparing to send) configuration file and I think it is annoying to write a literal constant there which is already defined in imx-regs.h. PPC seems to use a predefined macro from asm/immap_8xxx.h files. Where is that file included? -- Timo ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
On 18/04/2012 09:57, Timo Ketola wrote: > One might want to define CONFIG_SYS_FSL_ESDHC_ADDR with the macro already > define in imx-regs.h, e.g. with IMX_MMC_SDHC1_BASE. Then the header must be > included here. > > Signed-off-by: Timo Ketola > --- > drivers/mmc/fsl_esdhc.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c > index a2f35e3..5ada747 100644 > --- a/drivers/mmc/fsl_esdhc.c > +++ b/drivers/mmc/fsl_esdhc.c > @@ -36,6 +36,7 @@ > #include > #include > #include > +#include > NAK. There is a good reason to avoid it. The fsl_esdhc driver is common to both i.MX and PowerPc architecture, and of course PowerPC have not imx-regs.h. And CONFIG_SYS_FSL_ESDHC_ADDR cannot be set by a macro in imx-regs.h, because it is different on PowerPC. By the way, why do you need it if you do not use that macro ? Best regards, Stefano Babic -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
One might want to define CONFIG_SYS_FSL_ESDHC_ADDR with the macro already define in imx-regs.h, e.g. with IMX_MMC_SDHC1_BASE. Then the header must be included here. Signed-off-by: Timo Ketola --- drivers/mmc/fsl_esdhc.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..5ada747 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -36,6 +36,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; -- 1.7.5.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 7/9] imx: esdhc: Needed to use in imx-regs.h defined address
One might want to define CONFIG_SYS_FSL_ESDHC_ADDR with the macro already define in imx-regs.h, e.g. with IMX_MMC_SDHC1_BASE. Then the header must be included here. Signed-off-by: Timo Ketola --- drivers/mmc/fsl_esdhc.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index a2f35e3..5ada747 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -36,6 +36,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; -- 1.7.5.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
