Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Stefan Roese

adding my 0.02$ to this discussion...

On 26.03.2014 22:31, Eric Bénard wrote:

this board is produced by Embest/Element 14 and is based on i.MX6 Dual
The following features are tested :
- UART2 (console)
- eMMC
- uSDCard
- Ethernet
- USB Host (through 2 ports hub)
- HDMI output
- I2C 1/2
- SPI NOR Flash

Boot on SPI NOR and through USB loader are tested.

For more informations on this board :
http://www.embest-tech.com/shop/star/marsboard.html

As this board shares a lot with RiOTboard, both boards are supported by
the same code base which is renamed embest/mx6boards

Signed-off-by: Eric Bénard e...@eukrea.com
---
  board/embest/{riotboard = mx6boards}/Makefile |  2 +-
  .../riotboard.c = mx6boards/mx6boards.c}  | 49 +-
  boards.cfg |  3 +-
  include/configs/{riotboard.h = embestmx6boards.h} | 58 ++
  4 files changed, 108 insertions(+), 4 deletions(-)
  rename board/embest/{riotboard = mx6boards}/Makefile (87%)
  rename board/embest/{riotboard/riotboard.c = mx6boards/mx6boards.c} (91%)
  rename include/configs/{riotboard.h = embestmx6boards.h} (84%)

diff --git a/board/embest/riotboard/Makefile b/board/embest/mx6boards/Makefile
similarity index 87%
rename from board/embest/riotboard/Makefile
rename to board/embest/mx6boards/Makefile
index 5f978c0..467fb50 100644
--- a/board/embest/riotboard/Makefile
+++ b/board/embest/mx6boards/Makefile
@@ -6,4 +6,4 @@
  # SPDX-License-Identifier:GPL-2.0+
  #

-obj-y  := riotboard.o
+obj-y  := mx6boards.o
diff --git a/board/embest/riotboard/riotboard.c 
b/board/embest/mx6boards/mx6boards.c
similarity index 91%
rename from board/embest/riotboard/riotboard.c
rename to board/embest/mx6boards/mx6boards.c
index 15eaa1e..374c2ec 100644
--- a/board/embest/riotboard/riotboard.c
+++ b/board/embest/mx6boards/mx6boards.c
@@ -60,6 +60,9 @@ DECLARE_GLOBAL_DATA_PTR;
PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | PAD_CTL_HYS |   \
PAD_CTL_ODE | PAD_CTL_SRE_FAST)

+#define SPI_PAD_CTRL (PAD_CTL_HYS | PAD_CTL_SPEED_MED | \
+ PAD_CTL_DSE_40ohm | PAD_CTL_SRE_FAST)
+
  int dram_init(void)
  {
gd-ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
@@ -153,8 +156,10 @@ iomux_v3_cfg_t const usdhc3_pads[] = {
MX6_PAD_SD3_DAT1__SD3_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX6_PAD_SD3_DAT2__SD3_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX6_PAD_SD3_DAT3__SD3_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+#ifdef CONFIG_RIOTBOARD
MX6_PAD_SD3_DAT4__GPIO7_IO01| MUX_PAD_CTRL(NO_PAD_CTRL), /* WP */
MX6_PAD_SD3_DAT5__GPIO7_IO00| MUX_PAD_CTRL(NO_PAD_CTRL), /* CD */
+#endif
  };

  iomux_v3_cfg_t const usdhc4_pads[] = {
@@ -187,7 +192,12 @@ int board_mmc_getcd(struct mmc *mmc)
ret = !gpio_get_value(USDHC2_CD_GPIO);
break;
case USDHC3_BASE_ADDR:
+#ifdef CONFIG_RIOTBOARD
ret = !gpio_get_value(USDHC3_CD_GPIO);
+#endif
+#ifdef CONFIG_MARSBOARD
+   ret = 1; /* eMMC/uSDHC3 is always present */
+#endif


Yes, I agree. #ifdef's are ugly. But code duplication is also a problem 
as Wolfgang has mentioned.


Isn't there a way to detect the board type at runtime somehow (via CPU 
type or GPIO input, ...)? And then dynamically configure the board 
either for RiOT or MarS? This would make the code a bit more complex 
of course. But there would be no #ifdef's and no code duplication. And 
you would only have to maintain one U-Boot binary / version for both boards.


So, what do you think? Is this possible?

Thanks,
Stefan

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Eric Bénard
Hi Stefan,

Le Thu, 27 Mar 2014 08:05:10 +0100,
Stefan Roese s...@denx.de a écrit :
 Yes, I agree. #ifdef's are ugly. But code duplication is also a problem 
 as Wolfgang has mentioned.
 
 Isn't there a way to detect the board type at runtime somehow (via CPU 
 type or GPIO input, ...)? And then dynamically configure the board 
 either for RiOT or MarS? This would make the code a bit more complex 
 of course. But there would be no #ifdef's and no code duplication. And 
 you would only have to maintain one U-Boot binary / version for both boards.
 
 So, what do you think? Is this possible?
 
Thanks for the idea, that's possible by detecting i.MX6Solo
vs i.MX6Dual.
I'll try to implement that.

Eric
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Fabio Estevam
On Thu, Mar 27, 2014 at 2:36 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Fabio Estevam,

 In message 
 caomzo5d2n1zvo5ogeam1_ewt-0kkamzgvrfpeekh5zymqkl...@mail.gmail.com you 
 wrote:

 Me too. I also think the ifdefs may easily cause confusion.

 So you suggest we remove all conditional code and use duplication
 everywhere?  You must be joking...

Not everywhere. We are just talking about board files here, which are
coming obsolete these days with device tree...


 For example: if someone sends a patch for Riotboard, he/she may break
 Marsboard without knowing.

 Usually the developer has only one board and is not able to test on both 
 boards.

 Yes, of course.  If someone touches common code he make breake things
 for boards he did not test.  That has always been the case,

So in theory yes, we could have a single board file for all the mx6
boards out there and make it full of ifdefs all over it to handle all
the hardware variations.

Not so good for the users though if they want to make a simple change
and then they are forced to read all the schematics of several
unrelated boards.

 everywehre.  But that has never been a reason to duplicate code,
 or to accept code duplication.   On contrary, we permanently struggle
 to reduce such duplication.

 Otherwise we would have to strictly split ARM and PowerPC code, and
 all other architectures.  We would need code copies for each SoC.
 Actually each board would need his own version of the whole U-Boot
 code.  Sorry, but this is just bizarre...

Looks like you are extrapolating too much here. Our discussion is only
about board files, not common driver code or architectural code.

Regards,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Otavio Salvador
Em 27/03/2014 09:44, Fabio Estevam feste...@gmail.com escreveu:

 On Thu, Mar 27, 2014 at 2:36 AM, Wolfgang Denk w...@denx.de wrote:
  Dear Fabio Estevam,
 
  In message 
caomzo5d2n1zvo5ogeam1_ewt-0kkamzgvrfpeekh5zymqkl...@mail.gmail.com you
wrote:
 
  Me too. I also think the ifdefs may easily cause confusion.
 
  So you suggest we remove all conditional code and use duplication
  everywhere?  You must be joking...

 Not everywhere. We are just talking about board files here, which are
 coming obsolete these days with device tree...

And this does work, as can be seen in Barebox which has it working for
quite some time.

You can see equivalent patch in

http://lists.infradead.org/pipermail/barebox/2014-March/018408.html

It seems U-Boot is behind !

Regards,
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Eric Bénard
Hi Fabio,

Le Thu, 27 Mar 2014 09:44:02 -0300,
Fabio Estevam feste...@gmail.com a écrit :
 So in theory yes, we could have a single board file for all the mx6
 boards out there and make it full of ifdefs all over it to handle all
 the hardware variations.
 
here we talk of 2 boards from the same manufacturer which are very
similar. What Wolfgang asked is very relevant in that case.

Eric
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Eric Bénard
Hi Otavio,

Le Thu, 27 Mar 2014 09:50:10 -0300,
Otavio Salvador ota...@ossystems.com.br a écrit :
 And this does work, as can be seen in Barebox which has it working for
 quite some time.
 
 You can see equivalent patch in
 
 http://lists.infradead.org/pipermail/barebox/2014-March/018408.html
 
 It seems U-Boot is behind !
 
I don't know what you are trying to demonstrate here but in the present
case, I'll also submit a v2 of these patches on barebox'ML to factorize
the code and the device tree.

Eric
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Fabio Estevam
On Thu, Mar 27, 2014 at 12:59 PM, Eric Bénard e...@eukrea.com wrote:
 Hi Fabio,

 Le Thu, 27 Mar 2014 09:44:02 -0300,
 Fabio Estevam feste...@gmail.com a écrit :
 So in theory yes, we could have a single board file for all the mx6
 boards out there and make it full of ifdefs all over it to handle all
 the hardware variations.

 here we talk of 2 boards from the same manufacturer which are very
 similar. What Wolfgang asked is very relevant in that case.

Agreed now. I wasn't aware they were from the same manufacturer.

Regards,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-27 Thread Wolfgang Denk
Dear Fabio Estevam,

In message caomzo5bf0e20ers4kelh+jkgkqutgn5eci-w_9qtrndbcdk...@mail.gmail.com 
you wrote:

 So in theory yes, we could have a single board file for all the mx6
 boards out there and make it full of ifdefs all over it to handle all
 the hardware variations.

Well, actually I do think there is some common code for iMX6 that
could / should be factored out.  And that does not even need any
ifdefs.

  Otherwise we would have to strictly split ARM and PowerPC code, and
  all other architectures.  We would need code copies for each SoC.
  Actually each board would need his own version of the whole U-Boot
  code.  Sorry, but this is just bizarre...
 
 Looks like you are extrapolating too much here. Our discussion is only

Not more than you, just in the other direction ;-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Es ist nicht genug zu wissen, man muß auch anwenden; es ist nicht ge-
nug zu wollen, man muß auch tun.   -- Goethe, Maximen und Reflexionen
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-26 Thread Eric Bénard
this board is produced by Embest/Element 14 and is based on i.MX6 Dual
The following features are tested :
- UART2 (console)
- eMMC
- uSDCard
- Ethernet
- USB Host (through 2 ports hub)
- HDMI output
- I2C 1/2
- SPI NOR Flash

Boot on SPI NOR and through USB loader are tested.

For more informations on this board :
http://www.embest-tech.com/shop/star/marsboard.html

As this board shares a lot with RiOTboard, both boards are supported by
the same code base which is renamed embest/mx6boards

Signed-off-by: Eric Bénard e...@eukrea.com
---
 board/embest/{riotboard = mx6boards}/Makefile |  2 +-
 .../riotboard.c = mx6boards/mx6boards.c}  | 49 +-
 boards.cfg |  3 +-
 include/configs/{riotboard.h = embestmx6boards.h} | 58 ++
 4 files changed, 108 insertions(+), 4 deletions(-)
 rename board/embest/{riotboard = mx6boards}/Makefile (87%)
 rename board/embest/{riotboard/riotboard.c = mx6boards/mx6boards.c} (91%)
 rename include/configs/{riotboard.h = embestmx6boards.h} (84%)

diff --git a/board/embest/riotboard/Makefile b/board/embest/mx6boards/Makefile
similarity index 87%
rename from board/embest/riotboard/Makefile
rename to board/embest/mx6boards/Makefile
index 5f978c0..467fb50 100644
--- a/board/embest/riotboard/Makefile
+++ b/board/embest/mx6boards/Makefile
@@ -6,4 +6,4 @@
 # SPDX-License-Identifier: GPL-2.0+
 #
 
-obj-y  := riotboard.o
+obj-y  := mx6boards.o
diff --git a/board/embest/riotboard/riotboard.c 
b/board/embest/mx6boards/mx6boards.c
similarity index 91%
rename from board/embest/riotboard/riotboard.c
rename to board/embest/mx6boards/mx6boards.c
index 15eaa1e..374c2ec 100644
--- a/board/embest/riotboard/riotboard.c
+++ b/board/embest/mx6boards/mx6boards.c
@@ -60,6 +60,9 @@ DECLARE_GLOBAL_DATA_PTR;
PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | PAD_CTL_HYS |   \
PAD_CTL_ODE | PAD_CTL_SRE_FAST)
 
+#define SPI_PAD_CTRL (PAD_CTL_HYS | PAD_CTL_SPEED_MED | \
+ PAD_CTL_DSE_40ohm | PAD_CTL_SRE_FAST)
+
 int dram_init(void)
 {
gd-ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
@@ -153,8 +156,10 @@ iomux_v3_cfg_t const usdhc3_pads[] = {
MX6_PAD_SD3_DAT1__SD3_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX6_PAD_SD3_DAT2__SD3_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
MX6_PAD_SD3_DAT3__SD3_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+#ifdef CONFIG_RIOTBOARD
MX6_PAD_SD3_DAT4__GPIO7_IO01| MUX_PAD_CTRL(NO_PAD_CTRL), /* WP */
MX6_PAD_SD3_DAT5__GPIO7_IO00| MUX_PAD_CTRL(NO_PAD_CTRL), /* CD */
+#endif
 };
 
 iomux_v3_cfg_t const usdhc4_pads[] = {
@@ -187,7 +192,12 @@ int board_mmc_getcd(struct mmc *mmc)
ret = !gpio_get_value(USDHC2_CD_GPIO);
break;
case USDHC3_BASE_ADDR:
+#ifdef CONFIG_RIOTBOARD
ret = !gpio_get_value(USDHC3_CD_GPIO);
+#endif
+#ifdef CONFIG_MARSBOARD
+   ret = 1; /* eMMC/uSDHC3 is always present */
+#endif
break;
case USDHC4_BASE_ADDR:
ret = 1; /* eMMC/uSDHC4 is always present */
@@ -205,9 +215,13 @@ int board_mmc_init(bd_t *bis)
/*
 * According to the board_mmc_init() the following map is done:
 * (U-boot device node)(Physical Port)
+* ** RiOTboard :
 * mmc0SDCard slot (bottom)
 * mmc1uSDCard slot (top)
 * mmc2eMMC
+* ** MarSBoard :
+* mmc0uSDCard slot (bottom)
+* mmc1eMMC
 */
for (i = 0; i  CONFIG_SYS_FSL_USDHC_NUM; i++) {
switch (i) {
@@ -224,6 +238,11 @@ int board_mmc_init(bd_t *bis)
gpio_direction_input(USDHC3_CD_GPIO);
usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
usdhc_cfg[1].max_bus_width = 4;
+#ifdef CONFIG_MARSBOARD
+   gpio_direction_output(IMX_GPIO_NR(7, 8) , 0);
+   udelay(250);
+   gpio_set_value(IMX_GPIO_NR(7, 8), 1);
+#endif
break;
case 2:
imx_iomux_v3_setup_multiple_pads(
@@ -248,6 +267,20 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#ifdef CONFIG_MXC_SPI
+iomux_v3_cfg_t const ecspi1_pads[] = {
+   MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
+   MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL),
+   MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL),
+   MX6_PAD_EIM_EB2__GPIO2_IO30 | MUX_PAD_CTRL(NO_PAD_CTRL),
+};
+
+static void setup_spi(void)
+{
+   imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads));
+}
+#endif
+
 struct i2c_pads_info i2c_pad_info1 = {
.scl = {
.i2c_mode = MX6_PAD_CSI0_DAT9__I2C1_SCL | 
MUX_PAD_CTRL(I2C_PAD_CTRL),
@@ -458,21 +491,28 @@ int board_init(void)
 {
/* address of boot 

Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-26 Thread Otavio Salvador
On Wed, Mar 26, 2014 at 6:31 PM, Eric Bénard e...@eukrea.com wrote:
 this board is produced by Embest/Element 14 and is based on i.MX6 Dual
 The following features are tested :
 - UART2 (console)
 - eMMC
 - uSDCard
 - Ethernet
 - USB Host (through 2 ports hub)
 - HDMI output
 - I2C 1/2
 - SPI NOR Flash

 Boot on SPI NOR and through USB loader are tested.

 For more informations on this board :
 http://www.embest-tech.com/shop/star/marsboard.html

 As this board shares a lot with RiOTboard, both boards are supported by
 the same code base which is renamed embest/mx6boards

 Signed-off-by: Eric Bénard e...@eukrea.com

I understand both boards share a lot of code but I think the set of
ifdef makes harder for people to understand the board code. Personally
I prefer v1.

-- 
Otavio Salvador O.S. Systems
http://www.ossystems.com.brhttp://code.ossystems.com.br
Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-26 Thread Wolfgang Denk
Dear Otavio,

In message CAP9ODKrj9as+R7gb-STCnqpoSGFdKg_diJ=p+vg6htoka5d...@mail.gmail.com 
you wrote:

 I understand both boards share a lot of code but I think the set of
 ifdef makes harder for people to understand the board code. Personally
 I prefer v1.

Duplication of so much code is really bad.  It's always been a
maintenance nightmare.  The new version is much better.  [Of course
there are also ways to avoid ifdefs, but there are not so many here,
and they are actually pretty easy to follow.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Commitment, n.:  Commitment can be illustrated by a breakfast
of ham and eggs. The chicken was involved, the pig was committed.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/2] MarSBoard: add new board

2014-03-26 Thread Wolfgang Denk
Dear Fabio Estevam,

In message caomzo5d2n1zvo5ogeam1_ewt-0kkamzgvrfpeekh5zymqkl...@mail.gmail.com 
you wrote:

 Me too. I also think the ifdefs may easily cause confusion.

So you suggest we remove all conditional code and use duplication
everywhere?  You must be joking...

 For example: if someone sends a patch for Riotboard, he/she may break
 Marsboard without knowing.
 
 Usually the developer has only one board and is not able to test on both 
 boards.

Yes, of course.  If someone touches common code he make breake things
for boards he did not test.  That has always been the case,
everywehre.  But that has never been a reason to duplicate code,
or to accept code duplication.   On contrary, we permanently struggle
to reduce such duplication.

Otherwise we would have to strictly split ARM and PowerPC code, and
all other architectures.  We would need code copies for each SoC.
Actually each board would need his own version of the whole U-Boot
code.  Sorry, but this is just bizarre...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I don't mind criticism. You know me. I've  never  been  one  to  take
offence  at  criticism. No one could say I'm the sort to take offence
at criticism -- Not twice, anyway. Not without blowing bubbles.
  - Terry Pratchett, _Witches Abroad_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot