[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Jagdish Gediya]

Add command "boot_bank X" to switch the boot bank to either
1 or 2.

Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
---
 board/freescale/ls1012ardb/ls1012ardb.c | 85 +++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/board/freescale/ls1012ardb/ls1012ardb.c b/board/freescale/ls1012ardb/ls1012ardb.c
index ed5a8e6..4d4f6fb 100644
--- a/board/freescale/ls1012ardb/ls1012ardb.c
+++ b/board/freescale/ls1012ardb/ls1012ardb.c
@@ -26,6 +26,9 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#define BOOT_FROM_UPPER_BANK	0x2
+#define BOOT_FROM_LOWER_BANK	0x1
+
 int checkboard(void)
 {
 #ifdef CONFIG_TARGET_LS1012ARDB
@@ -212,3 +215,85 @@ int ft_board_setup(void *blob, bd_t *bd)
 
 	return 0;
 }
+
+static int switch_to_bank1(void)
+{
+	u8 data;
+	int ret;
+
+	i2c_set_bus_num(0);
+
+	data = 0xf4;
+	ret = i2c_write(0x24, 0x3, 1, &data, 1);
+	if (ret) {
+		printf("i2c write error to chip : %u, addr : %u, data : %u\n",
+		       0x24, 0x3, data);
+	}
+
+	return ret;
+}
+
+static int switch_to_bank2(void)
+{
+	u8 data;
+	int ret;
+
+	i2c_set_bus_num(0);
+
+	data = 0xfc;
+	ret = i2c_write(0x24, 0x7, 1, &data, 1);
+	if (ret) {
+		printf("i2c write error to chip : %u, addr : %u, data : %u\n",
+		       0x24, 0x7, data);
+		goto err;
+	}
+
+	data = 0xf5;
+	ret = i2c_write(0x24, 0x3, 1, &data, 1);
+	if (ret) {
+		printf("i2c write error to chip : %u, addr : %u, data : %u\n",
+		       0x24, 0x3, data);
+	}
+err:
+	return ret;
+}
+
+static int convert_flash_bank(int bank)
+{
+	int ret = 0;
+
+	switch (bank) {
+	case BOOT_FROM_UPPER_BANK:
+		ret = switch_to_bank2();
+		break;
+	case BOOT_FROM_LOWER_BANK:
+		ret = switch_to_bank1();
+		break;
+	default:
+		ret = CMD_RET_USAGE;
+		break;
+	};
+
+	return ret;
+}
+
+static int flash_bank_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
+			  char * const argv[])
+{
+	if (argc != 2)
+		return CMD_RET_USAGE;
+	if (strcmp(argv[1], "1") == 0)
+		convert_flash_bank(BOOT_FROM_LOWER_BANK);
+	else if (strcmp(argv[1], "2") == 0)
+		convert_flash_bank(BOOT_FROM_UPPER_BANK);
+	else
+		return CMD_RET_USAGE;
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	boot_bank, 2, 0, flash_bank_cmd,
+	"Flash bank Selection Control",
+	"bank[1-lower bank/2-upper bank] (e.g. boot_bank 1)"
+);
-- 
1.9.1

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Calvin Johnson]

On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya <jagdish.gediya@nxp.com> wrote:
> Add command "boot_bank X" to switch the boot bank to either
> 1 or 2.

Are these functions required as this can be handled by new env vars to
switch banks?

>
> Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
> ---
>  board/freescale/ls1012ardb/ls1012ardb.c | 85 +++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
>
> diff --git a/board/freescale/ls1012ardb/ls1012ardb.c b/board/freescale/ls1012ardb/ls1012ardb.c
> index ed5a8e6..4d4f6fb 100644
> --- a/board/freescale/ls1012ardb/ls1012ardb.c
> +++ b/board/freescale/ls1012ardb/ls1012ardb.c
> @@ -26,6 +26,9 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +#define BOOT_FROM_UPPER_BANK   0x2
> +#define BOOT_FROM_LOWER_BANK   0x1
> +
>  int checkboard(void)
>  {
>  #ifdef CONFIG_TARGET_LS1012ARDB
> @@ -212,3 +215,85 @@ int ft_board_setup(void *blob, bd_t *bd)
>
>         return 0;
>  }
> +
> +static int switch_to_bank1(void)
> +{
> +       u8 data;
> +       int ret;
> +
> +       i2c_set_bus_num(0);
> +
> +       data = 0xf4;
> +       ret = i2c_write(0x24, 0x3, 1, &data, 1);
> +       if (ret) {
> +               printf("i2c write error to chip : %u, addr : %u, data : %u\n",
> +                      0x24, 0x3, data);
> +       }
> +
> +       return ret;
> +}
> +
> +static int switch_to_bank2(void)
> +{
> +       u8 data;
> +       int ret;
> +
> +       i2c_set_bus_num(0);
> +
> +       data = 0xfc;
> +       ret = i2c_write(0x24, 0x7, 1, &data, 1);
> +       if (ret) {
> +               printf("i2c write error to chip : %u, addr : %u, data : %u\n",
> +                      0x24, 0x7, data);
> +               goto err;
> +       }
> +
> +       data = 0xf5;
> +       ret = i2c_write(0x24, 0x3, 1, &data, 1);
> +       if (ret) {
> +               printf("i2c write error to chip : %u, addr : %u, data : %u\n",
> +                      0x24, 0x3, data);
> +       }
> +err:
> +       return ret;
> +}
> +
> +static int convert_flash_bank(int bank)
> +{
> +       int ret = 0;
> +
> +       switch (bank) {
> +       case BOOT_FROM_UPPER_BANK:
> +               ret = switch_to_bank2();
> +               break;
> +       case BOOT_FROM_LOWER_BANK:
> +               ret = switch_to_bank1();
> +               break;
> +       default:
> +               ret = CMD_RET_USAGE;
> +               break;
> +       };
> +
> +       return ret;
> +}
> +
> +static int flash_bank_cmd(cmd_tbl_t *cmdtp, int flag, int argc,
> +                         char * const argv[])
> +{
> +       if (argc != 2)
> +               return CMD_RET_USAGE;
> +       if (strcmp(argv[1], "1") == 0)
> +               convert_flash_bank(BOOT_FROM_LOWER_BANK);
> +       else if (strcmp(argv[1], "2") == 0)
> +               convert_flash_bank(BOOT_FROM_UPPER_BANK);
> +       else
> +               return CMD_RET_USAGE;
> +
> +       return 0;
> +}
> +
> +U_BOOT_CMD(
> +       boot_bank, 2, 0, flash_bank_cmd,
> +       "Flash bank Selection Control",
> +       "bank[1-lower bank/2-upper bank] (e.g. boot_bank 1)"
> +);
> --
> 1.9.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Scott Wood]

On Mon, 2018-04-16 at 08:40 +0530, Calvin Johnson wrote:
> On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya <jagdish.gediya@nxp.com>
> wrote:
> > Add command "boot_bank X" to switch the boot bank to either
> > 1 or 2.
> 
> Are these functions required as this can be handled by new env vars to
> switch banks?

If you're going to add something new, a command is much more pleasant than env
vars -- particularly if you stick to something like the familiar interfaces
("pix altbank", "qix altbank", etc), and include reporting of which bank was
booted from if it's not there already.  Of course, a fully standardized
interface would be even better.

I'm assuming this hardware isn't at all compatible with pixis/qixis?

-Scott

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Calvin Johnson]

On Thu, Apr 19, 2018 at 12:17 PM, Scott Wood <oss@buserror.net> wrote:
> On Mon, 2018-04-16 at 08:40 +0530, Calvin Johnson wrote:
>> On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya <jagdish.gediya@nxp.com>
>> wrote:
>> > Add command "boot_bank X" to switch the boot bank to either
>> > 1 or 2.
>>
>> Are these functions required as this can be handled by new env vars to
>> switch banks?
>
> If you're going to add something new, a command is much more pleasant than env
> vars -- particularly if you stick to something like the familiar interfaces
> ("pix altbank", "qix altbank", etc), and include reporting of which bank was
> booted from if it's not there already.  Of course, a fully standardized
> interface would be even better.

Yes, a fully standardized generic interface supporting all similar
platforms with multiple banks
would be better. What this patch currently does can be done with
simple env vars, like :

setenv boot_bank_1 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf5'
setenv boot_bank_2 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf4'

regards
Calvin

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Scott Wood]

On Thu, 2018-04-19 at 14:09 +0530, Calvin Johnson wrote:
> On Thu, Apr 19, 2018 at 12:17 PM, Scott Wood <oss@buserror.net> wrote:
> > On Mon, 2018-04-16 at 08:40 +0530, Calvin Johnson wrote:
> > > On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya <jagdish.gediya@nxp.com
> > > >
> > > wrote:
> > > > Add command "boot_bank X" to switch the boot bank to either
> > > > 1 or 2.
> > > 
> > > Are these functions required as this can be handled by new env vars to
> > > switch banks?
> > 
> > If you're going to add something new, a command is much more pleasant than
> > env
> > vars -- particularly if you stick to something like the familiar
> > interfaces
> > ("pix altbank", "qix altbank", etc), and include reporting of which bank
> > was
> > booted from if it's not there already.  Of course, a fully standardized
> > interface would be even better.
> 
> Yes, a fully standardized generic interface supporting all similar
> platforms with multiple banks

And other boot sources such as NAND and MMC.

> would be better. What this patch currently does can be done with
> simple env vars, like :
> 
> setenv boot_bank_1 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf5'
> setenv boot_bank_2 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf4'

...if the user knows to env reset those variables after the update (versus
something that shows up in help), and if they don't get corrupted in a multi-
user board farm environment, etc.

-Scott

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Calvin Johnson]

> -----Original Message-----
> From: Scott Wood [mailto:oss at buserror.net]
> Sent: Friday, April 20, 2018 6:40 AM
> To: Calvin Johnson <linux.cj@gmail.com>
> Cc: Jagdish Gediya <jagdish.gediya@nxp.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Calvin Johnson <calvin.johnson@nxp.com>; York Sun
> <york.sun@nxp.com>
> Subject: Re: [U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to
> switch QSPI bank
> 
> On Thu, 2018-04-19 at 14:09 +0530, Calvin Johnson wrote:
> > On Thu, Apr 19, 2018 at 12:17 PM, Scott Wood <oss@buserror.net> wrote:
> > > On Mon, 2018-04-16 at 08:40 +0530, Calvin Johnson wrote:
> > > > On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya <jagdish.gediya@nxp.com
> > > > >
> > > > wrote:
> > > > > Add command "boot_bank X" to switch the boot bank to either
> > > > > 1 or 2.
> > > >
> > > > Are these functions required as this can be handled by new env vars to
> > > > switch banks?
> > >
> > > If you're going to add something new, a command is much more pleasant
> than
> > > env
> > > vars -- particularly if you stick to something like the familiar
> > > interfaces
> > > ("pix altbank", "qix altbank", etc), and include reporting of which bank
> > > was
> > > booted from if it's not there already.  Of course, a fully standardized
> > > interface would be even better.
> >
> > Yes, a fully standardized generic interface supporting all similar
> > platforms with multiple banks
> 
> And other boot sources such as NAND and MMC.
> 
> > would be better. What this patch currently does can be done with
> > simple env vars, like :
> >
> > setenv boot_bank_1 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf5'
> > setenv boot_bank_2 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf4'
> 
> ...if the user knows to env reset those variables after the update (versus
> something that shows up in help), and if they don't get corrupted in a multi-
> user board farm environment, etc.

Make sense. Thanks!

Calvin

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Prabhakar Kushwaha]

Thanks Scott for reviewing this patch

> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Scott
> Wood
> Sent: Friday, April 20, 2018 6:40 AM
> To: Calvin Johnson <linux.cj@gmail.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>
> Subject: Re: [U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to
> switch QSPI bank
> 
> On Thu, 2018-04-19 at 14:09 +0530, Calvin Johnson wrote:
> > On Thu, Apr 19, 2018 at 12:17 PM, Scott Wood <oss@buserror.net> wrote:
> > > On Mon, 2018-04-16 at 08:40 +0530, Calvin Johnson wrote:
> > > > On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya
> > > > <jagdish.gediya@nxp.com
> > > > >
> > > > wrote:
> > > > > Add command "boot_bank X" to switch the boot bank to either
> > > > > 1 or 2.
> > > >
> > > > Are these functions required as this can be handled by new env
> > > > vars to switch banks?
> > >
> > > If you're going to add something new, a command is much more
> > > pleasant than env vars -- particularly if you stick to something
> > > like the familiar interfaces ("pix altbank", "qix altbank", etc),
> > > and include reporting of which bank was booted from if it's not
> > > there already.  Of course, a fully standardized interface would be
> > > even better.
> >
> > Yes, a fully standardized generic interface supporting all similar
> > platforms with multiple banks
> 
> And other boot sources such as NAND and MMC.


LS1012A has only one boot source i.e. QSPI

> 
> > would be better. What this patch currently does can be done with
> > simple env vars, like :
> >
> > setenv boot_bank_1 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf5'
> > setenv boot_bank_2 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf4'
> 
> ...if the user knows to env reset those variables after the update (versus
> something that shows up in help), and if they don't get corrupted in a multi-
> user board farm environment, etc.
> 

With env there is always a probability of getting erased. In that case user will not have these commands.

Better approach should be command which is always available. If required we can update the doc.  

--pk

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Scott Wood]

On Fri, 2018-04-20 at 03:16 +0000, Prabhakar Kushwaha wrote:
> Thanks Scott for reviewing this patch
> 
> > -----Original Message-----
> > From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Scott
> > Wood
> > Sent: Friday, April 20, 2018 6:40 AM
> > To: Calvin Johnson <linux.cj@gmail.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>
> > Subject: Re: [U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to
> > switch QSPI bank
> > 
> > On Thu, 2018-04-19 at 14:09 +0530, Calvin Johnson wrote:
> > > On Thu, Apr 19, 2018 at 12:17 PM, Scott Wood <oss@buserror.net> wrote:
> > > > On Mon, 2018-04-16 at 08:40 +0530, Calvin Johnson wrote:
> > > > > On Fri, Apr 13, 2018 at 12:18 AM, Jagdish Gediya
> > > > > <jagdish.gediya@nxp.com
> > > > > > 
> > > > > 
> > > > > wrote:
> > > > > > Add command "boot_bank X" to switch the boot bank to either
> > > > > > 1 or 2.
> > > > > 
> > > > > Are these functions required as this can be handled by new env
> > > > > vars to switch banks?
> > > > 
> > > > If you're going to add something new, a command is much more
> > > > pleasant than env vars -- particularly if you stick to something
> > > > like the familiar interfaces ("pix altbank", "qix altbank", etc),
> > > > and include reporting of which bank was booted from if it's not
> > > > there already.  Of course, a fully standardized interface would be
> > > > even better.
> > > 
> > > Yes, a fully standardized generic interface supporting all similar
> > > platforms with multiple banks
> > 
> > And other boot sources such as NAND and MMC.
> 
> 
> LS1012A has only one boot source i.e. QSPI

I meant that if there is any effort to standardize the command it should
support multiple boot sources, not just bank selection.

-Scott

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[York Sun]

On 04/19/2018 06:53 PM, Calvin Johnson wrote:
>>
>>> would be better. What this patch currently does can be done with
>>> simple env vars, like :
>>>
>>> setenv boot_bank_1 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf5'
>>> setenv boot_bank_2 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf4'
>>
>> ...if the user knows to env reset those variables after the update (versus
>> something that shows up in help), and if they don't get corrupted in a multi-
>> user board farm environment, etc.
> 
> Make sense. Thanks!
> 

Calvin,

Do you want to stay with env, or still want this patch, or standardize
the command?

York

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[Calvin Johnson]

Hi York,

> -----Original Message-----
> From: York Sun
> Sent: Tuesday, July 31, 2018 8:37 PM
> To: Calvin Johnson <calvin.johnson@nxp.com>; Scott Wood
> <oss@buserror.net>; Calvin Johnson <linux.cj@gmail.com>
> Cc: Jagdish Gediya <jagdish.gediya@nxp.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Subject: Re: [U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to
> switch QSPI bank
> 
> On 04/19/2018 06:53 PM, Calvin Johnson wrote:
> >>
> >>> would be better. What this patch currently does can be done with
> >>> simple env vars, like :
> >>>
> >>> setenv boot_bank_1 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf5'
> >>> setenv boot_bank_2 'i2c mw 0x24 0x7 0xfc; i2c mw 0x24 0x3 0xf4'
> >>
> >> ...if the user knows to env reset those variables after the update
> >> (versus something that shows up in help), and if they don't get
> >> corrupted in a multi- user board farm environment, etc.
> >
> > Make sense. Thanks!
> >
> 
> Calvin,
> 
> Do you want to stay with env, or still want this patch, or standardize the
> command?

Ideally, a fully standardized generic command interface supporting all similar 
platforms with multiple banks as well as other boot sources such as NAND and MMC, 
is the best option.

This patch looks good as an interim solution.

Regards
Calvin

[U-Boot] [PATCH] board: freescale: ls1012ardb: Add command to switch QSPI bank

[York Sun]

On 04/11/2018 11:40 PM, Jagdish Gediya wrote:
> Add command "boot_bank X" to switch the boot bank to either
> 1 or 2.
> 
> Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com>
> ---

Applied to fsl-qoriq master, awaiting upstream.
Thanks.

York