[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

There are two revisions of wandboard: version B1 and C1.

Add the revision detection support, so that the correct dtb file can
be automatically loaded.

Based on the patch from Richard Hu <hakahu@gmail.com>.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:

- Fixed commit log to 'automatically loaded'.

 board/wandboard/wandboard.c | 26 +++++++++++++++++++++++++-
 include/configs/wandboard.h |  8 ++++++--
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c
index 90625ab..0af63d2 100644
--- a/board/wandboard/wandboard.c
+++ b/board/wandboard/wandboard.c
@@ -50,6 +50,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define USDHC1_CD_GPIO		IMX_GPIO_NR(1, 2)
 #define USDHC3_CD_GPIO		IMX_GPIO_NR(3, 9)
 #define ETH_PHY_RESET		IMX_GPIO_NR(3, 29)
+#define REV_DETECTION		IMX_GPIO_NR(2, 28)
 
 int dram_init(void)
 {
@@ -105,6 +106,10 @@ static iomux_v3_cfg_t const enet_pads[] = {
 	IOMUX_PADS(PAD_EIM_D29__GPIO3_IO29    | MUX_PAD_CTRL(NO_PAD_CTRL)),
 };
 
+static iomux_v3_cfg_t const rev_detection_pad[] = {
+	IOMUX_PADS(PAD_EIM_EB0__GPIO2_IO28  | MUX_PAD_CTRL(NO_PAD_CTRL)),
+};
+
 static void setup_iomux_uart(void)
 {
 	SETUP_IOMUX_PADS(uart1_pads);
@@ -393,6 +398,17 @@ static const struct boot_mode board_boot_modes[] = {
 };
 #endif
 
+static bool is_revc1(void)
+{
+	SETUP_IOMUX_PADS(rev_detection_pad);
+	gpio_direction_input(REV_DETECTION);
+
+	if (gpio_get_value(REV_DETECTION))
+		return true;
+	else
+		return false;
+}
+
 int board_late_init(void)
 {
 #ifdef CONFIG_CMD_BMODE
@@ -404,6 +420,11 @@ int board_late_init(void)
 		setenv("board_rev", "MX6Q");
 	else
 		setenv("board_rev", "MX6DL");
+
+	if (is_revc1())
+		setenv("board_name", "C1");
+	else
+		setenv("board_name", "B1");
 #endif
 	return 0;
 }
@@ -424,7 +445,10 @@ int board_init(void)
 
 int checkboard(void)
 {
-	puts("Board: Wandboard\n");
+	if (is_revc1())
+		puts("Board: Wandboard rev C1\n");
+	else
+		puts("Board: Wandboard rev B1\n");
 
 	return 0;
 }
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h
index 7ce861e..e87ab90 100644
--- a/include/configs/wandboard.h
+++ b/include/configs/wandboard.h
@@ -232,10 +232,14 @@
 			"bootz; " \
 		"fi;\0" \
 	"findfdt="\
-		"if test $board_rev = MX6Q ; then " \
+		"if test $board_name = C1 && test $board_rev = MX6Q ; then " \
 			"setenv fdtfile imx6q-wandboard.dtb; fi; " \
-		"if test $board_rev = MX6DL ; then " \
+		"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
 			"setenv fdtfile imx6dl-wandboard.dtb; fi; " \
+		"if test $board_name = B1 && test $board_rev = MX6Q ; then " \
+			"setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \
+		"if test $board_name = B1 && test $board_rev = MX6DL ; then " \
+			"setenv fdtfile imx6dl-wandboard-revb1.dtb; fi; " \
 		"if test $fdtfile = undefined; then " \
 			"echo WARNING: Could not determine dtb to use; fi; \0" \
 
-- 
1.9.1

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Stefano Babic]

Hi Fabio,

On 22/05/2015 00:24, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There are two revisions of wandboard: version B1 and C1.
> 
> Add the revision detection support, so that the correct dtb file can
> be automatically loaded.
> 
> Based on the patch from Richard Hu <hakahu@gmail.com>.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v1:
> 
> - Fixed commit log to 'automatically loaded'.
> 
>  board/wandboard/wandboard.c | 26 +++++++++++++++++++++++++-
>  include/configs/wandboard.h |  8 ++++++--
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c
> index 90625ab..0af63d2 100644
> --- a/board/wandboard/wandboard.c
> +++ b/board/wandboard/wandboard.c
> @@ -50,6 +50,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define USDHC1_CD_GPIO		IMX_GPIO_NR(1, 2)
>  #define USDHC3_CD_GPIO		IMX_GPIO_NR(3, 9)
>  #define ETH_PHY_RESET		IMX_GPIO_NR(3, 29)
> +#define REV_DETECTION		IMX_GPIO_NR(2, 28)
>  
>  int dram_init(void)
>  {
> @@ -105,6 +106,10 @@ static iomux_v3_cfg_t const enet_pads[] = {
>  	IOMUX_PADS(PAD_EIM_D29__GPIO3_IO29    | MUX_PAD_CTRL(NO_PAD_CTRL)),
>  };
>  
> +static iomux_v3_cfg_t const rev_detection_pad[] = {
> +	IOMUX_PADS(PAD_EIM_EB0__GPIO2_IO28  | MUX_PAD_CTRL(NO_PAD_CTRL)),
> +};
> +
>  static void setup_iomux_uart(void)
>  {
>  	SETUP_IOMUX_PADS(uart1_pads);
> @@ -393,6 +398,17 @@ static const struct boot_mode board_boot_modes[] = {
>  };
>  #endif
>  
> +static bool is_revc1(void)
> +{
> +	SETUP_IOMUX_PADS(rev_detection_pad);
> +	gpio_direction_input(REV_DETECTION);
> +
> +	if (gpio_get_value(REV_DETECTION))
> +		return true;
> +	else
> +		return false;
> +}
> +
>  int board_late_init(void)
>  {
>  #ifdef CONFIG_CMD_BMODE
> @@ -404,6 +420,11 @@ int board_late_init(void)
>  		setenv("board_rev", "MX6Q");
>  	else
>  		setenv("board_rev", "MX6DL");
> +
> +	if (is_revc1())
> +		setenv("board_name", "C1");
> +	else
> +		setenv("board_name", "B1");
>  #endif
>  	return 0;
>  }
> @@ -424,7 +445,10 @@ int board_init(void)
>  
>  int checkboard(void)
>  {
> -	puts("Board: Wandboard\n");
> +	if (is_revc1())
> +		puts("Board: Wandboard rev C1\n");
> +	else
> +		puts("Board: Wandboard rev B1\n");
>  
>  	return 0;
>  }
> diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h
> index 7ce861e..e87ab90 100644
> --- a/include/configs/wandboard.h
> +++ b/include/configs/wandboard.h
> @@ -232,10 +232,14 @@
>  			"bootz; " \
>  		"fi;\0" \
>  	"findfdt="\
> -		"if test $board_rev = MX6Q ; then " \
> +		"if test $board_name = C1 && test $board_rev = MX6Q ; then " \
>  			"setenv fdtfile imx6q-wandboard.dtb; fi; " \
> -		"if test $board_rev = MX6DL ; then " \
> +		"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
>  			"setenv fdtfile imx6dl-wandboard.dtb; fi; " \
> +		"if test $board_name = B1 && test $board_rev = MX6Q ; then " \
> +			"setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \
> +		"if test $board_name = B1 && test $board_rev = MX6DL ; then " \
> +			"setenv fdtfile imx6dl-wandboard-revb1.dtb; fi; " \
>  		"if test $fdtfile = undefined; then " \
>  			"echo WARNING: Could not determine dtb to use; fi; \0" \
>  
> 

Reviewed-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Vagrant Cascadian]

On 2015-05-21, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> There are two revisions of wandboard: version B1 and C1.
>
> Add the revision detection support, so that the correct dtb file can
> be automatically loaded.
>
> Based on the patch from Richard Hu <hakahu@gmail.com>.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Tested on a Wandboard dual (rev c1) and wandboard quad (rev b1).

Tested-By: Vagrant Cascadian <vagrant@aikidev.net>

> ---
> Changes since v1:
>
> - Fixed commit log to 'automatically loaded'.
>
>  board/wandboard/wandboard.c | 26 +++++++++++++++++++++++++-
>  include/configs/wandboard.h |  8 ++++++--
>  2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c
> index 90625ab..0af63d2 100644
> --- a/board/wandboard/wandboard.c
> +++ b/board/wandboard/wandboard.c
> @@ -50,6 +50,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define USDHC1_CD_GPIO		IMX_GPIO_NR(1, 2)
>  #define USDHC3_CD_GPIO		IMX_GPIO_NR(3, 9)
>  #define ETH_PHY_RESET		IMX_GPIO_NR(3, 29)
> +#define REV_DETECTION		IMX_GPIO_NR(2, 28)
>  
>  int dram_init(void)
>  {
> @@ -105,6 +106,10 @@ static iomux_v3_cfg_t const enet_pads[] = {
>  	IOMUX_PADS(PAD_EIM_D29__GPIO3_IO29    | MUX_PAD_CTRL(NO_PAD_CTRL)),
>  };
>  
> +static iomux_v3_cfg_t const rev_detection_pad[] = {
> +	IOMUX_PADS(PAD_EIM_EB0__GPIO2_IO28  | MUX_PAD_CTRL(NO_PAD_CTRL)),
> +};
> +
>  static void setup_iomux_uart(void)
>  {
>  	SETUP_IOMUX_PADS(uart1_pads);
> @@ -393,6 +398,17 @@ static const struct boot_mode board_boot_modes[] = {
>  };
>  #endif
>  
> +static bool is_revc1(void)
> +{
> +	SETUP_IOMUX_PADS(rev_detection_pad);
> +	gpio_direction_input(REV_DETECTION);
> +
> +	if (gpio_get_value(REV_DETECTION))
> +		return true;
> +	else
> +		return false;
> +}
> +
>  int board_late_init(void)
>  {
>  #ifdef CONFIG_CMD_BMODE
> @@ -404,6 +420,11 @@ int board_late_init(void)
>  		setenv("board_rev", "MX6Q");
>  	else
>  		setenv("board_rev", "MX6DL");
> +
> +	if (is_revc1())
> +		setenv("board_name", "C1");
> +	else
> +		setenv("board_name", "B1");
>  #endif
>  	return 0;
>  }
> @@ -424,7 +445,10 @@ int board_init(void)
>  
>  int checkboard(void)
>  {
> -	puts("Board: Wandboard\n");
> +	if (is_revc1())
> +		puts("Board: Wandboard rev C1\n");
> +	else
> +		puts("Board: Wandboard rev B1\n");
>  
>  	return 0;
>  }
> diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h
> index 7ce861e..e87ab90 100644
> --- a/include/configs/wandboard.h
> +++ b/include/configs/wandboard.h
> @@ -232,10 +232,14 @@
>  			"bootz; " \
>  		"fi;\0" \
>  	"findfdt="\
> -		"if test $board_rev = MX6Q ; then " \
> +		"if test $board_name = C1 && test $board_rev = MX6Q ; then " \
>  			"setenv fdtfile imx6q-wandboard.dtb; fi; " \
> -		"if test $board_rev = MX6DL ; then " \
> +		"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
>  			"setenv fdtfile imx6dl-wandboard.dtb; fi; " \
> +		"if test $board_name = B1 && test $board_rev = MX6Q ; then " \
> +			"setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \
> +		"if test $board_name = B1 && test $board_rev = MX6DL ; then " \
> +			"setenv fdtfile imx6dl-wandboard-revb1.dtb; fi; " \
>  		"if test $fdtfile = undefined; then " \
>  			"echo WARNING: Could not determine dtb to use; fi; \0" \
>  
> -- 
> 1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150522/5726fa75/attachment.sig>

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Alexander Holler]

Am 23.05.2015 um 02:38 schrieb Vagrant Cascadian:
> On 2015-05-21, Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> There are two revisions of wandboard: version B1 and C1.
>>
>> Add the revision detection support, so that the correct dtb file can
>> be automatically loaded.
>>
>> Based on the patch from Richard Hu <hakahu@gmail.com>.
>>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>
> Tested on a Wandboard dual (rev c1) and wandboard quad (rev b1).
>
> Tested-By: Vagrant Cascadian <vagrant@aikidev.net>
>
>> ---
>> Changes since v1:
>>
>> - Fixed commit log to 'automatically loaded'.
>>
>>   board/wandboard/wandboard.c | 26 +++++++++++++++++++++++++-
>>   include/configs/wandboard.h |  8 ++++++--

Wouldn't it be better to just enable CONFIG_CMD_GPIO and then change the 
boot-script in the config to something like "if gpio ..." instead of 
adding something special?

Assuming the gpio command works on imx, which I haven't tested or looked up.

Regards,

Alexander Holler

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Stefano Babic]

Hi Fabio, Alexander,

On 23/05/2015 18:27, Alexander Holler wrote:
> Am 23.05.2015 um 02:38 schrieb Vagrant Cascadian:
>> On 2015-05-21, Fabio Estevam wrote:
>>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>>
>>> There are two revisions of wandboard: version B1 and C1.
>>>
>>> Add the revision detection support, so that the correct dtb file can
>>> be automatically loaded.
>>>
>>> Based on the patch from Richard Hu <hakahu@gmail.com>.
>>>
>>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Tested on a Wandboard dual (rev c1) and wandboard quad (rev b1).
>>
>> Tested-By: Vagrant Cascadian <vagrant@aikidev.net>
>>
>>> ---
>>> Changes since v1:
>>>
>>> - Fixed commit log to 'automatically loaded'.
>>>
>>>   board/wandboard/wandboard.c | 26 +++++++++++++++++++++++++-
>>>   include/configs/wandboard.h |  8 ++++++--
> 
> Wouldn't it be better to just enable CONFIG_CMD_GPIO and then change the
> boot-script in the config to something like "if gpio ..." instead of
> adding something special?
> 
> Assuming the gpio command works on imx, which I haven't tested or looked
> up.

gpio works - this is really a good idea, moving the check into the
script. Fabio, what do you mind ?

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

Hi Stefano and Alexander,

On Sun, May 24, 2015 at 4:43 AM, Stefano Babic <sbabic@denx.de> wrote:

>> Wouldn't it be better to just enable CONFIG_CMD_GPIO and then change the
>> boot-script in the config to something like "if gpio ..." instead of
>> adding something special?
>>
>> Assuming the gpio command works on imx, which I haven't tested or looked
>> up.
>
> gpio works - this is really a good idea, moving the check into the
> script. Fabio, what do you mind ?

I think the idea is good, thanks.

I wanted to keep consistency with the mx6cuboxi implementation (which
was based on TI's implementation suggested by Tom during the review of
the mx6cuboxi patches).

Also, the gpio script idea would work fine for selecting the dtb file,
but not inside checkboard() function, where I print the board revision
name.

Other aspect I thought is the fact that in case we have another
revision of the board in the future, I think that C code is more
flexible for handling it.

So I like the idea of gpio script, but I would prefer to keep the
current implementation if possible due to the reasons stated above.

Thanks for the suggestion.

Fabio Estevam

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Stefano Babic]

Hi Fabio,

On 24/05/2015 16:27, Fabio Estevam wrote:
> 
> I think the idea is good, thanks.
> 
> I wanted to keep consistency with the mx6cuboxi implementation (which
> was based on TI's implementation suggested by Tom during the review of
> the mx6cuboxi patches).
> 
> Also, the gpio script idea would work fine for selecting the dtb file,
> but not inside checkboard() function, where I print the board revision
> name.
> 
> Other aspect I thought is the fact that in case we have another
> revision of the board in the future, I think that C code is more
> flexible for handling it.

This is a good point, thanks.

> 
> So I like the idea of gpio script, but I would prefer to keep the
> current implementation if possible due to the reasons stated above.
> 

ok, clear enough - on my site it is ok to maintain current implementation.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Alexander Holler]

Am 24.05.2015 um 16:27 schrieb Fabio Estevam:
> Hi Stefano and Alexander,
>
> On Sun, May 24, 2015 at 4:43 AM, Stefano Babic <sbabic@denx.de> wrote:
>
>>> Wouldn't it be better to just enable CONFIG_CMD_GPIO and then change the
>>> boot-script in the config to something like "if gpio ..." instead of
>>> adding something special?
>>>
>>> Assuming the gpio command works on imx, which I haven't tested or looked
>>> up.
>>
>> gpio works - this is really a good idea, moving the check into the
>> script. Fabio, what do you mind ?
>
> I think the idea is good, thanks.
>
> I wanted to keep consistency with the mx6cuboxi implementation (which
> was based on TI's implementation suggested by Tom during the review of
> the mx6cuboxi patches).
>
> Also, the gpio script idea would work fine for selecting the dtb file,
> but not inside checkboard() function, where I print the board revision
> name.

Printing the board revision in the script is as easy as selecting the dtb.

> Other aspect I thought is the fact that in case we have another
> revision of the board in the future, I think that C code is more
> flexible for handling it.

Not really. Then it it would need again a patch for the C source. Using 
the gpio command one could just change the check even by just changing 
uEnv.txt. Look at how long it now needed until someone did this patch 
(your patch) for u-boot.

> So I like the idea of gpio script, but I would prefer to keep the
> current implementation if possible due to the reasons stated above.

I would suggest to change the stuff for mx6cuboxi to use the gpio 
command too instead of taking the same (imho wrong) approach.

But enough said from me, I don't really care. ;)

Regards,

Alexander Holler

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Alexander Holler]

Am 24.05.2015 um 19:29 schrieb Alexander Holler:
> Am 24.05.2015 um 16:27 schrieb Fabio Estevam:
>> Hi Stefano and Alexander,
>>
>> On Sun, May 24, 2015 at 4:43 AM, Stefano Babic <sbabic@denx.de> wrote:
>>
>>>> Wouldn't it be better to just enable CONFIG_CMD_GPIO and then change
>>>> the
>>>> boot-script in the config to something like "if gpio ..." instead of
>>>> adding something special?
>>>>
>>>> Assuming the gpio command works on imx, which I haven't tested or
>>>> looked
>>>> up.
>>>
>>> gpio works - this is really a good idea, moving the check into the
>>> script. Fabio, what do you mind ?
>>
>> I think the idea is good, thanks.
>>
>> I wanted to keep consistency with the mx6cuboxi implementation (which
>> was based on TI's implementation suggested by Tom during the review of
>> the mx6cuboxi patches).
>>
>> Also, the gpio script idea would work fine for selecting the dtb file,
>> but not inside checkboard() function, where I print the board revision
>> name.
>
> Printing the board revision in the script is as easy as selecting the dtb.
>
>> Other aspect I thought is the fact that in case we have another
>> revision of the board in the future, I think that C code is more
>> flexible for handling it.
>
> Not really. Then it it would need again a patch for the C source. Using
> the gpio command one could just change the check even by just changing
> uEnv.txt. Look at how long it now needed until someone did this patch
> (your patch) for u-boot.
>
>> So I like the idea of gpio script, but I would prefer to keep the
>> current implementation if possible due to the reasons stated above.
>
> I would suggest to change the stuff for mx6cuboxi to use the gpio
> command too instead of taking the same (imho wrong) approach.
>
> But enough said from me, I don't really care. ;)

Hmm, just one comment more.

If the gpio command would be enabled, it would even be possible to reset 
the BRCM- WLAN and Bluetooth modules by just adding some stuff to 
uEnv.txt. So at least WLAN would reliable work even after a reboot or 
reset, without the need for the rfkill-driver. Also the 
wandboard-rfkill-driver has more advantages, e.g. the rfkill. ;)

For those which don't know it, currently, without the 
wandboard-rfkill-driver, WLAN works only once after power up, but not 
after the reboot. Thats because the HW doesn't reset the brcm on reset, 
so after a reboot or reset uploading the firmware fails (because it is 
already uploaded due to the missing reset of the module).


> Regards,
>
> Alexander Holler
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

On Sun, May 24, 2015 at 2:29 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> I would suggest to change the stuff for mx6cuboxi to use the gpio command
> too instead of taking the same (imho wrong) approach.

mx6cuboxi uses two GPIOs for distinguishing between the various
revisions of hummingboard and cuboxi.

Handling two GPIOs via script would add extra complexity with no real advantage.

Regards,

Fabio Estevam

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

On Sun, May 24, 2015 at 3:03 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> Hmm, just one comment more.
>
> If the gpio command would be enabled, it would even be possible to reset the
> BRCM- WLAN and Bluetooth modules by just adding some stuff to uEnv.txt. So
> at least WLAN would reliable work even after a reboot or reset, without the
> need for the rfkill-driver. Also the wandboard-rfkill-driver has more
> advantages, e.g. the rfkill. ;)
>
> For those which don't know it, currently, without the
> wandboard-rfkill-driver, WLAN works only once after power up, but not after
> the reboot. Thats because the HW doesn't reset the brcm on reset, so after a
> reboot or reset uploading the firmware fails (because it is already uploaded
> due to the missing reset of the module).

Mainline kernel has proper way of handling the reset of the Wifi chip
via pwrseq mechanism.

Please see this patch I did for cuboxi:
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/dt&id=ec5757d85c0fd32274a6052b8a4482d626c01ae5

So we should really not add such hack in U-boot as mainline kernel has
the proper solution.

Regards,

Fabio Estevam

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Tom Rini]

On Sun, May 24, 2015 at 08:03:12PM +0200, Alexander Holler wrote:
> Am 24.05.2015 um 19:29 schrieb Alexander Holler:
> >Am 24.05.2015 um 16:27 schrieb Fabio Estevam:
> >>Hi Stefano and Alexander,
> >>
> >>On Sun, May 24, 2015 at 4:43 AM, Stefano Babic <sbabic@denx.de> wrote:
> >>
> >>>>Wouldn't it be better to just enable CONFIG_CMD_GPIO and then change
> >>>>the
> >>>>boot-script in the config to something like "if gpio ..." instead of
> >>>>adding something special?
> >>>>
> >>>>Assuming the gpio command works on imx, which I haven't tested or
> >>>>looked
> >>>>up.
> >>>
> >>>gpio works - this is really a good idea, moving the check into the
> >>>script. Fabio, what do you mind ?
> >>
> >>I think the idea is good, thanks.
> >>
> >>I wanted to keep consistency with the mx6cuboxi implementation (which
> >>was based on TI's implementation suggested by Tom during the review of
> >>the mx6cuboxi patches).
> >>
> >>Also, the gpio script idea would work fine for selecting the dtb file,
> >>but not inside checkboard() function, where I print the board revision
> >>name.
> >
> >Printing the board revision in the script is as easy as selecting the dtb.
> >
> >>Other aspect I thought is the fact that in case we have another
> >>revision of the board in the future, I think that C code is more
> >>flexible for handling it.
> >
> >Not really. Then it it would need again a patch for the C source. Using
> >the gpio command one could just change the check even by just changing
> >uEnv.txt. Look at how long it now needed until someone did this patch
> >(your patch) for u-boot.
> >
> >>So I like the idea of gpio script, but I would prefer to keep the
> >>current implementation if possible due to the reasons stated above.
> >
> >I would suggest to change the stuff for mx6cuboxi to use the gpio
> >command too instead of taking the same (imho wrong) approach.
> >
> >But enough said from me, I don't really care. ;)
> 
> Hmm, just one comment more.
> 
> If the gpio command would be enabled, it would even be possible to
> reset the BRCM- WLAN and Bluetooth modules by just adding some stuff
> to uEnv.txt. So at least WLAN would reliable work even after a
> reboot or reset, without the need for the rfkill-driver. Also the
> wandboard-rfkill-driver has more advantages, e.g. the rfkill. ;)

The gpio command should be enabled and the series that consolidates all
of the common mx6 options into a single config file to inherit does so,
JFYI.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150524/46339948/attachment.sig>

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Alexander Holler]

Am 24.05.2015 um 20:47 schrieb Fabio Estevam:
> On Sun, May 24, 2015 at 3:03 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> 
>> Hmm, just one comment more.
>>
>> If the gpio command would be enabled, it would even be possible to reset the
>> BRCM- WLAN and Bluetooth modules by just adding some stuff to uEnv.txt. So
>> at least WLAN would reliable work even after a reboot or reset, without the
>> need for the rfkill-driver. Also the wandboard-rfkill-driver has more
>> advantages, e.g. the rfkill. ;)
>>
>> For those which don't know it, currently, without the
>> wandboard-rfkill-driver, WLAN works only once after power up, but not after
>> the reboot. Thats because the HW doesn't reset the brcm on reset, so after a
>> reboot or reset uploading the firmware fails (because it is already uploaded
>> due to the missing reset of the module).
> 
> Mainline kernel has proper way of handling the reset of the Wifi chip
> via pwrseq mechanism.
> 
> Please see this patch I did for cuboxi:
> https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/dt&id=ec5757d85c0fd32274a6052b8a4482d626c01ae5
> 
> So we should really not add such hack in U-boot as mainline kernel has
> the proper solution.

I've not said that I want such a hack in u-boot.

In fact I said that I don't want your hack in u-boot. My suggestion is
to enable the gpio-command in order to make it possible to do stuff like
checking for board revisions without having to modify u-boot at all (the
binary as well as the source).

As long as u-boot doesn't (have to) care about the board-revision, I
don't see the need to add such stuff to a board-file.

But as said, I don't care. But, please, don't suggest that I want to add
a hack.

Alexander Holler

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

On Sun, May 24, 2015 at 7:22 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> I've not said that I want such a hack in u-boot.

Well, you wrote:

"If the gpio command would be enabled, it would even be possible to reset the
BRCM- WLAN and Bluetooth modules by just adding some stuff to uEnv.txt."

This sounds like a real hack for me, sorry.

> In fact I said that I don't want your hack in u-boot. My suggestion is
> to enable the gpio-command in order to make it possible to do stuff like
> checking for board revisions without having to modify u-boot at all (the
> binary as well as the source).

Your suggestion would not work if we use Falcon mode, for example.

> As long as u-boot doesn't (have to) care about the board-revision, I
> don't see the need to add such stuff to a board-file.

U-boot needs to care about board revision to load the correct dtb file.

Regards,

Fabio Estevam

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Wolfgang Denk]

Dear Fabio,

In message <CAOMZO5DFpUjmgF_SoRNaGtxExzBQ1-bVTFVTBnTMy40QpDwXZw@mail.gmail.com> you wrote:
> 
> "If the gpio command would be enabled, it would even be possible to reset the
> BRCM- WLAN and Bluetooth modules by just adding some stuff to uEnv.txt."
> 
> This sounds like a real hack for me, sorry.

Consider it as an example for being able to do many different things
that have not even been thought of at the time of implementations. to
try out things.  Maybe we decide later to implement the features a C
code, but for testing or working aound problems is is always nice to
have scripting capabilities.

> Your suggestion would not work if we use Falcon mode, for example.

Well, with Falcon mode you usually prepare the specific images to use
in full U-Boot context, so the selection would be made when we have
the feature available. Or am I missing something?

> U-boot needs to care about board revision to load the correct dtb file.

Actually this is nothing that affects U-Boot code.  It is part of the
U-Boot functionality, but that does not mean it has to be implemented
as part of the U-Boot code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If there was anything that depressed him more than his own  cynicism,
it was that quite often it still wasn't as cynical as real life.
                                 - Terry Pratchett, _Guards! Guards!_

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

Hi Wolfgang,

On Mon, May 25, 2015 at 6:41 AM, Wolfgang Denk <wd@denx.de> wrote:

> Consider it as an example for being able to do many different things
> that have not even been thought of at the time of implementations. to
> try out things.  Maybe we decide later to implement the features a C
> code, but for testing or working aound problems is is always nice to
> have scripting capabilities.

CONFIG_CMD_GPIO is going to be selected by the common mx6_config in
Peter Robinson's consolidation series.

> Actually this is nothing that affects U-Boot code.  It is part of the
> U-Boot functionality, but that does not mean it has to be implemented
> as part of the U-Boot code.

The reason I took this approach was to make it consistent with the way
we do on mx6cuboxi as requested by Tom:
http://lists.denx.de/pipermail/u-boot/2015-April/212221.html

Regards,

Fabio Estevam

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Alexander Holler]

Am 25.05.2015 um 11:41 schrieb Wolfgang Denk:
> Dear Fabio,
>
> In message <CAOMZO5DFpUjmgF_SoRNaGtxExzBQ1-bVTFVTBnTMy40QpDwXZw@mail.gmail.com> you wrote:
>>
>> "If the gpio command would be enabled, it would even be possible to reset the
>> BRCM- WLAN and Bluetooth modules by just adding some stuff to uEnv.txt."
>>
>> This sounds like a real hack for me, sorry.
>
> Consider it as an example for being able to do many different things
> that have not even been thought of at the time of implementations. to
> try out things.  Maybe we decide later to implement the features a C
> code, but for testing or working aound problems is is always nice to
> have scripting capabilities.

Exactly. It's just another example besides the possibility to choose the 
correct dtb based on the board revision without modifying u-boot at all 
(if gpio would be enabled). And also the example is a hack, that's the 
stuff which is necessary very often, until someone fixes something in a 
driver (and managed it to get the fix upstream), or even to circumvent 
problems with some HW.

Btw., in regard to the above specific hack, the same problem (failure to 
upload firmware because it's already uploaded) exists for Bluetooth too. 
Personally I've modified (hacked) the wandboard-rfkill-driver a year ago 
to disable the regulator of the brcm-module completely for some 
milliseconds at startup (and when the module is unloaded, to save 
power). But that involves having to manage own patches for the kernel, 
so I might even prefer to use just a hack in uEnv.txt to fix the reset 
problem with the brcm-module.

Besides that I don't really care what a maintainer said to some previous 
similiar problem. Maintainers are humans too, maybe he just had 
forgotten about the gpio-command.

Regards,

Alexander Holler

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Fabio Estevam]

On Tue, May 26, 2015 at 3:55 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> Besides that I don't really care what a maintainer said to some previous
> similiar problem. Maintainers are humans too, maybe he just had forgotten
> about the gpio-command.

The gpio command is now available in Stefano's tree after Peter's
config consolidation series has been applied.

Regards,

Fabio Estevam

[U-Boot] [PATCH v2] wandboard: Add board revision detection support

[Stefano Babic]

Hi Fabio,

On 22/05/2015 00:24, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There are two revisions of wandboard: version B1 and C1.
> 
> Add the revision detection support, so that the correct dtb file can
> be automatically loaded.
> 
> Based on the patch from Richard Hu <hakahu@gmail.com>.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---


Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================