| Message ID | 1366296086-22394-1-git-send-email-p.aubert@staubli.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Dear Pierre Aubert, In message <1366296086-22394-1-git-send-email-p.aubert@staubli.com> you wrote: > The SabreSD platform is available with i.MX6Q or i.MX6DL. This patch adds the > support of the i.MX6DL. The config file and the board directory are renamed > to remove the reference to the MX6Q. Formal issues: - entry to MAINTAINERS file missing - there are 2 checkpatch warnings ("please, no spaces at the start of a line") that need to be fixed. > int checkboard(void) > { > - puts("Board: MX6Q-SabreSD\n"); > - > +#ifdef CONFIG_MX6Q > + puts("Board: MX6Q-SabreSD\n"); > +#else > + puts("Board: MX6DL-SabreSD\n"); > +#endif Can we please avoid such #ifdef's? Here, we could for example refer to the board name (CONFIG_SYS_BOARD if you like the name, or some custom defined CONFIG_BOARD_NAME like other boards do). Best regards, Wolfgang Denk
Hi Pierre, On Thu, Apr 18, 2013 at 11:41 AM, Pierre Aubert <p.aubert@staubli.com> wrote: > diff --git a/boards.cfg b/boards.cfg > index f785da8..c002cf7 100644 > --- a/boards.cfg > +++ b/boards.cfg > @@ -256,9 +256,10 @@ mx53smd arm armv7 mx53smd freesca > ima3-mx53 arm armv7 ima3-mx53 esg mx5 ima3-mx53:IMX_CONFIG=board/esg/ima3-mx53/imximage.cfg > vision2 arm armv7 vision2 ttcontrol mx5 vision2:IMX_CONFIG=board/ttcontrol/vision2/imximage_hynix.cfg > mx6qarm2 arm armv7 mx6qarm2 freescale mx6 mx6qarm2:IMX_CONFIG=board/freescale/mx6qarm2/imximage.cfg > -mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg > +mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg,MX6Q mx6qsabreauto changes should be done on a different patch. Regards, Fabio Estevam
Hi Fabio, I can split the patch if you wish, but in this case the compilation would be broken for the sabreauto if only one of the two patches is applied. Best regards Pierre Aubert Le 18/04/2013 19:51, Fabio Estevam a écrit : > Hi Pierre, > > On Thu, Apr 18, 2013 at 11:41 AM, Pierre Aubert <p.aubert@staubli.com> wrote: > >> diff --git a/boards.cfg b/boards.cfg >> index f785da8..c002cf7 100644 >> --- a/boards.cfg >> +++ b/boards.cfg >> @@ -256,9 +256,10 @@ mx53smd arm armv7 mx53smd freesca >> ima3-mx53 arm armv7 ima3-mx53 esg mx5 ima3-mx53:IMX_CONFIG=board/esg/ima3-mx53/imximage.cfg >> vision2 arm armv7 vision2 ttcontrol mx5 vision2:IMX_CONFIG=board/ttcontrol/vision2/imximage_hynix.cfg >> mx6qarm2 arm armv7 mx6qarm2 freescale mx6 mx6qarm2:IMX_CONFIG=board/freescale/mx6qarm2/imximage.cfg >> -mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg >> +mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg,MX6Q > mx6qsabreauto changes should be done on a different patch. > > Regards, > > Fabio Estevam
Hello Wolfgang, Le 18/04/2013 19:37, Wolfgang Denk a écrit : > Dear Pierre Aubert, > > In message <1366296086-22394-1-git-send-email-p.aubert@staubli.com> you wrote: >> The SabreSD platform is available with i.MX6Q or i.MX6DL. This patch adds the >> support of the i.MX6DL. The config file and the board directory are renamed >> to remove the reference to the MX6Q. > Formal issues: > > - entry to MAINTAINERS file missing Fabio Estevam is already the maintainer of the SabreSD. I didn't add any new boad, I've just add the support for the i.MX6DL on this board. I will update the MAINTAINERS file. > - there are 2 checkpatch warnings ("please, no spaces at the start of > a line") that need to be fixed. Will be fixed > >> int checkboard(void) >> { >> - puts("Board: MX6Q-SabreSD\n"); >> - >> +#ifdef CONFIG_MX6Q >> + puts("Board: MX6Q-SabreSD\n"); >> +#else >> + puts("Board: MX6DL-SabreSD\n"); >> +#endif > Can we please avoid such #ifdef's? Here, we could for example refer > to the board name (CONFIG_SYS_BOARD if you like the name, or some > custom defined CONFIG_BOARD_NAME like other boards do). > The board name in MX6-SabreSD. The Soc type (dual or quad core) is already printed by print_cpuinfo. > Best regards, > > Wolfgang Denk > Best regards Pierre Aubert
On 18/04/2013 19:37, Wolfgang Denk wrote: > Dear Pierre Aubert, > > In message <1366296086-22394-1-git-send-email-p.aubert@staubli.com> you wrote: >> The SabreSD platform is available with i.MX6Q or i.MX6DL. This patch adds the >> support of the i.MX6DL. The config file and the board directory are renamed >> to remove the reference to the MX6Q. > > Formal issues: > > - entry to MAINTAINERS file missing > - there are 2 checkpatch warnings ("please, no spaces at the start of > a line") that need to be fixed. > >> int checkboard(void) >> { >> - puts("Board: MX6Q-SabreSD\n"); >> - >> +#ifdef CONFIG_MX6Q >> + puts("Board: MX6Q-SabreSD\n"); >> +#else >> + puts("Board: MX6DL-SabreSD\n"); >> +#endif > > Can we please avoid such #ifdef's? Here, we could for example refer > to the board name (CONFIG_SYS_BOARD if you like the name, or some > custom defined CONFIG_BOARD_NAME like other boards do). And who does set CONFIG_MX6Q ? You drop it, but I do not see who sets it. Best regards, Stefano Babic
Le 19/04/2013 10:15, Stefano Babic a écrit : > >>> int checkboard(void) >>> { >>> - puts("Board: MX6Q-SabreSD\n"); >>> - >>> +#ifdef CONFIG_MX6Q >>> + puts("Board: MX6Q-SabreSD\n"); >>> +#else >>> + puts("Board: MX6DL-SabreSD\n"); >>> +#endif >> Can we please avoid such #ifdef's? Here, we could for example refer >> to the board name (CONFIG_SYS_BOARD if you like the name, or some >> custom defined CONFIG_BOARD_NAME like other boards do). > And who does set CONFIG_MX6Q ? You drop it, but I do not see who sets it. It is set in boards.cfg: > -mx6qsabresd arm armv7 mx6qsabresd freescale mx6 mx6qsabresd:IMX_CONFIG=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg > +mx6qsabresd arm armv7 mx6sabresd freescale mx6 mx6sabresd:IMX_CONFIG=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg,MX6Q > > Best regards, > Stefano Babic > Best regards Pierre Aubert
Hi Stefano, Le Fri, 19 Apr 2013 10:15:49 +0200, Stefano Babic <sbabic@denx.de> a écrit : > On 18/04/2013 19:37, Wolfgang Denk wrote: > > Dear Pierre Aubert, > > > > In message <1366296086-22394-1-git-send-email-p.aubert@staubli.com> you wrote: > >> The SabreSD platform is available with i.MX6Q or i.MX6DL. This patch adds the > >> support of the i.MX6DL. The config file and the board directory are renamed > >> to remove the reference to the MX6Q. > > > > Formal issues: > > > > - entry to MAINTAINERS file missing > > - there are 2 checkpatch warnings ("please, no spaces at the start of > > a line") that need to be fixed. > > > >> int checkboard(void) > >> { > >> - puts("Board: MX6Q-SabreSD\n"); > >> - > >> +#ifdef CONFIG_MX6Q > >> + puts("Board: MX6Q-SabreSD\n"); > >> +#else > >> + puts("Board: MX6DL-SabreSD\n"); > >> +#endif > > > > Can we please avoid such #ifdef's? Here, we could for example refer > > to the board name (CONFIG_SYS_BOARD if you like the name, or some > > custom defined CONFIG_BOARD_NAME like other boards do). > > And who does set CONFIG_MX6Q ? You drop it, but I do not see who sets it. > that's done in boards.cfg : +++ b/boards.cfg +mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg,MX6Q Eric
On 19/04/2013 10:27, Eric Bénard wrote: > Hi Stefano, > Hi Eric, >>> Can we please avoid such #ifdef's? Here, we could for example refer >>> to the board name (CONFIG_SYS_BOARD if you like the name, or some >>> custom defined CONFIG_BOARD_NAME like other boards do). >> >> And who does set CONFIG_MX6Q ? You drop it, but I do not see who sets it. >> > that's done in boards.cfg : > +++ b/boards.cfg > +mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg,MX6Q Ah, ok, thanks. Regards, Stefano
diff --git a/board/freescale/mx6qsabresd/Makefile b/board/freescale/mx6sabresd/Makefile similarity index 98% rename from board/freescale/mx6qsabresd/Makefile rename to board/freescale/mx6sabresd/Makefile index 5693772..ff3c94b 100644 --- a/board/freescale/mx6qsabresd/Makefile +++ b/board/freescale/mx6sabresd/Makefile @@ -23,7 +23,7 @@ include $(TOPDIR)/config.mk LIB = $(obj)lib$(BOARD).o -COBJS := mx6qsabresd.o +COBJS := mx6sabresd.o SRCS := $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS)) diff --git a/board/freescale/mx6qsabresd/mx6qsabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c similarity index 98% rename from board/freescale/mx6qsabresd/mx6qsabresd.c rename to board/freescale/mx6sabresd/mx6sabresd.c index 0d7cb9e..dcefdab 100644 --- a/board/freescale/mx6qsabresd/mx6qsabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -17,12 +17,10 @@ * GNU General Public License for more details. */ -#include <common.h> -#include <asm/io.h> #include <asm/arch/clock.h> #include <asm/arch/imx-regs.h> #include <asm/arch/iomux.h> -#include <asm/arch/mx6q_pins.h> +#include <asm/arch/mx6-pins.h> #include <asm/errno.h> #include <asm/gpio.h> #include <asm/imx-common/iomux-v3.h> @@ -291,7 +289,10 @@ int board_late_init(void) int checkboard(void) { - puts("Board: MX6Q-SabreSD\n"); - +#ifdef CONFIG_MX6Q + puts("Board: MX6Q-SabreSD\n"); +#else + puts("Board: MX6DL-SabreSD\n"); +#endif return 0; } diff --git a/boards.cfg b/boards.cfg index f785da8..c002cf7 100644 --- a/boards.cfg +++ b/boards.cfg @@ -256,9 +256,10 @@ mx53smd arm armv7 mx53smd freesca ima3-mx53 arm armv7 ima3-mx53 esg mx5 ima3-mx53:IMX_CONFIG=board/esg/ima3-mx53/imximage.cfg vision2 arm armv7 vision2 ttcontrol mx5 vision2:IMX_CONFIG=board/ttcontrol/vision2/imximage_hynix.cfg mx6qarm2 arm armv7 mx6qarm2 freescale mx6 mx6qarm2:IMX_CONFIG=board/freescale/mx6qarm2/imximage.cfg -mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg +mx6qsabreauto arm armv7 mx6qsabreauto freescale mx6 mx6qsabreauto:IMX_CONFIG=board/freescale/mx6qsabreauto/imximage.cfg,MX6Q mx6qsabrelite arm armv7 mx6qsabrelite freescale mx6 mx6qsabrelite:IMX_CONFIG=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg -mx6qsabresd arm armv7 mx6qsabresd freescale mx6 mx6qsabresd:IMX_CONFIG=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg +mx6qsabresd arm armv7 mx6sabresd freescale mx6 mx6sabresd:IMX_CONFIG=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg,MX6Q +mx6dlsabresd arm armv7 mx6sabresd freescale mx6 mx6sabresd:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL eco5pk arm armv7 eco5pk 8dtech omap3 nitrogen6dl arm armv7 nitrogen6x boundary mx6 nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL,DDR_MB=1024 nitrogen6dl2g arm armv7 nitrogen6x boundary mx6 nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl2g.cfg,MX6DL,DDR_MB=2048 diff --git a/include/configs/mx6qsabreauto.h b/include/configs/mx6qsabreauto.h index 1583c11..099839a 100644 --- a/include/configs/mx6qsabreauto.h +++ b/include/configs/mx6qsabreauto.h @@ -30,7 +30,7 @@ #define CONFIG_MXC_USB_PORTSC (PORT_PTS_UTMI | PORT_PTS_PTW) #define CONFIG_MXC_USB_FLAGS 0 -#include "mx6qsabre_common.h" +#include "mx6sabre_common.h" #define CONFIG_SYS_FSL_USDHC_NUM 2 #if defined(CONFIG_ENV_IS_IN_MMC) diff --git a/include/configs/mx6qsabre_common.h b/include/configs/mx6sabre_common.h similarity index 99% rename from include/configs/mx6qsabre_common.h rename to include/configs/mx6sabre_common.h index f5f115f..203158d 100644 --- a/include/configs/mx6qsabre_common.h +++ b/include/configs/mx6sabre_common.h @@ -18,7 +18,6 @@ #define __MX6QSABRE_COMMON_CONFIG_H #define CONFIG_MX6 -#define CONFIG_MX6Q #include "mx6_common.h" diff --git a/include/configs/mx6qsabresd.h b/include/configs/mx6sabresd.h similarity index 97% rename from include/configs/mx6qsabresd.h rename to include/configs/mx6sabresd.h index 3b8d752..e7facd3 100644 --- a/include/configs/mx6qsabresd.h +++ b/include/configs/mx6sabresd.h @@ -24,7 +24,7 @@ #define CONFIG_DEFAULT_FDT_FILE "imx6q-sabresd.dtb" #define PHYS_SDRAM_SIZE (1u * 1024 * 1024 * 1024) -#include "mx6qsabre_common.h" +#include "mx6sabre_common.h" #define CONFIG_SYS_FSL_USDHC_NUM 3 #if defined(CONFIG_ENV_IS_IN_MMC)
The SabreSD platform is available with i.MX6Q or i.MX6DL. This patch adds the support of the i.MX6DL. The config file and the board directory are renamed to remove the reference to the MX6Q. Signed-off-by: Pierre Aubert <p.aubert@staubli.com> CC: Stefano Babic <sbabic@denx.de> --- .../freescale/{mx6qsabresd => mx6sabresd}/Makefile | 2 +- .../mx6qsabresd.c => mx6sabresd/mx6sabresd.c} | 11 ++++++----- boards.cfg | 5 +++-- include/configs/mx6qsabreauto.h | 2 +- .../{mx6qsabre_common.h => mx6sabre_common.h} | 1 - include/configs/{mx6qsabresd.h => mx6sabresd.h} | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) rename board/freescale/{mx6qsabresd => mx6sabresd}/Makefile (98%) rename board/freescale/{mx6qsabresd/mx6qsabresd.c => mx6sabresd/mx6sabresd.c} (98%) rename include/configs/{mx6qsabre_common.h => mx6sabre_common.h} (99%) rename include/configs/{mx6qsabresd.h => mx6sabresd.h} (97%)