[U-Boot] [PATCH] sata: fix sata command not being executed bug

From: Tang Yuantian Yuantian.Tang@nxp.com
Variable sata_curr_device is used to indicate if there is a available sata disk on board.
Previously, sata_curr_device is set in sata_initialize(). Now, sata_initialize() is separated from other sata commands. Accordingly, sata_curr_device is removed from sata_initialize() too. This caused sata_curr_device never gets a chance to be set. If it can't be set a proper value, other sata command will never get a change to execute.
This patch sets variable sata_curr_device properly.
Signed-off-by: Tang Yuantian yuantian.tang@nxp.com --- cmd/sata.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/cmd/sata.c b/cmd/sata.c index d18b523..71c785f 100644 --- a/cmd/sata.c +++ b/cmd/sata.c @@ -20,6 +20,7 @@ static int sata_curr_device = -1; static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int rc = 0; + int i;
if (argc == 2 && strcmp(argv[1], "stop") == 0) return sata_stop(); @@ -32,9 +33,15 @@ static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }
/* If the user has not yet run `sata init`, do it now */ - if (sata_curr_device == -1) - if (sata_initialize()) - return 1; + if (sata_curr_device == -1) { + rc = sata_initialize(); + for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) { + if (sata_dev_desc[i].lba > 0) + sata_curr_device = i; + } + if (sata_curr_device == -1) + return -1; + }
switch (argc) { case 0:

Hi Tang,
On 9 November 2016 at 02:37, yuantian.tang@nxp.com wrote:
From: Tang Yuantian Yuantian.Tang@nxp.com
Variable sata_curr_device is used to indicate if there is a available sata disk on board.
Previously, sata_curr_device is set in sata_initialize(). Now, sata_initialize() is separated from other sata commands. Accordingly, sata_curr_device is removed from sata_initialize() too. This caused sata_curr_device never gets a chance to be set. If it can't be set a proper value, other sata command will never get a change to execute.
This patch sets variable sata_curr_device properly.
Signed-off-by: Tang Yuantian yuantian.tang@nxp.com
cmd/sata.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
Does this fix commit d97dc8a0 - if so can you please add a 'Fixes:' tag?
diff --git a/cmd/sata.c b/cmd/sata.c index d18b523..71c785f 100644 --- a/cmd/sata.c +++ b/cmd/sata.c @@ -20,6 +20,7 @@ static int sata_curr_device = -1; static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int rc = 0;
int i; if (argc == 2 && strcmp(argv[1], "stop") == 0) return sata_stop();
@@ -32,9 +33,15 @@ static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }
/* If the user has not yet run `sata init`, do it now */
if (sata_curr_device == -1)
if (sata_initialize())
return 1;
if (sata_curr_device == -1) {
rc = sata_initialize();
for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) {
if (sata_dev_desc[i].lba > 0)
sata_curr_device = i;
}
if (sata_curr_device == -1)
return -1;
Can this code go in its own function?
} switch (argc) { case 0:
-- 2.1.0.27.g96db324
Regards, Simon

Hi Simon,
Please see my reply inline.
-----Original Message----- From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass Sent: Saturday, November 12, 2016 12:18 AM To: Y.T. Tang yuantian.tang@nxp.com Cc: Bin Meng bmeng.cn@gmail.com; U-Boot Mailing List <u- boot@lists.denx.de> Subject: Re: [PATCH] sata: fix sata command not being executed bug
Hi Tang,
On 9 November 2016 at 02:37, yuantian.tang@nxp.com wrote:
From: Tang Yuantian Yuantian.Tang@nxp.com
Variable sata_curr_device is used to indicate if there is a available sata disk on board.
Previously, sata_curr_device is set in sata_initialize(). Now, sata_initialize() is separated from other sata commands. Accordingly, sata_curr_device is removed from sata_initialize() too. This caused sata_curr_device never gets a chance to be set. If it can't be set a proper value, other sata command will never get a change to execute.
This patch sets variable sata_curr_device properly.
Signed-off-by: Tang Yuantian yuantian.tang@nxp.com
cmd/sata.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
Does this fix commit d97dc8a0 - if so can you please add a 'Fixes:' tag?
Yes, commit d97dc8a0 causes this issue. There was a 'fix' in title. Do you mean adding "Fixes commit d97dc8a0"?
diff --git a/cmd/sata.c b/cmd/sata.c index d18b523..71c785f 100644 --- a/cmd/sata.c +++ b/cmd/sata.c @@ -20,6 +20,7 @@ static int sata_curr_device = -1; static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int rc = 0;
int i; if (argc == 2 && strcmp(argv[1], "stop") == 0) return sata_stop();
@@ -32,9 +33,15 @@ static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
} /* If the user has not yet run `sata init`, do it now */
if (sata_curr_device == -1)
if (sata_initialize())
return 1;
if (sata_curr_device == -1) {
rc = sata_initialize();
for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) {
if (sata_dev_desc[i].lba > 0)
sata_curr_device = i;
}
if (sata_curr_device == -1)
return -1;
Can this code go in its own function?
I want to refine this patch to make it more clear. This part will be rewritten.
Regards, Yuantian
} switch (argc) { case 0:
-- 2.1.0.27.g96db324
Regards, Simon

Hi,
On 15 November 2016 at 00:20, Y.T. Tang yuantian.tang@nxp.com wrote:
Hi Simon,
Please see my reply inline.
-----Original Message----- From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass Sent: Saturday, November 12, 2016 12:18 AM To: Y.T. Tang yuantian.tang@nxp.com Cc: Bin Meng bmeng.cn@gmail.com; U-Boot Mailing List <u- boot@lists.denx.de> Subject: Re: [PATCH] sata: fix sata command not being executed bug
Hi Tang,
On 9 November 2016 at 02:37, yuantian.tang@nxp.com wrote:
From: Tang Yuantian Yuantian.Tang@nxp.com
Variable sata_curr_device is used to indicate if there is a available sata disk on board.
Previously, sata_curr_device is set in sata_initialize(). Now, sata_initialize() is separated from other sata commands. Accordingly, sata_curr_device is removed from sata_initialize() too. This caused sata_curr_device never gets a chance to be set. If it can't be set a proper value, other sata command will never get a change to execute.
This patch sets variable sata_curr_device properly.
Signed-off-by: Tang Yuantian yuantian.tang@nxp.com
cmd/sata.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
Does this fix commit d97dc8a0 - if so can you please add a 'Fixes:' tag?
Yes, commit d97dc8a0 causes this issue. There was a 'fix' in title. Do you mean adding "Fixes commit d97dc8a0"?
See the commit log for examples - here's one I found:
Fixes: e824cf3f (smbios: Allow compilation on 64bit systems)
diff --git a/cmd/sata.c b/cmd/sata.c index d18b523..71c785f 100644 --- a/cmd/sata.c +++ b/cmd/sata.c @@ -20,6 +20,7 @@ static int sata_curr_device = -1; static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { int rc = 0;
int i; if (argc == 2 && strcmp(argv[1], "stop") == 0) return sata_stop();
@@ -32,9 +33,15 @@ static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
} /* If the user has not yet run `sata init`, do it now */
if (sata_curr_device == -1)
if (sata_initialize())
return 1;
if (sata_curr_device == -1) {
rc = sata_initialize();
for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) {
if (sata_dev_desc[i].lba > 0)
sata_curr_device = i;
}
if (sata_curr_device == -1)
return -1;
Can this code go in its own function?
I want to refine this patch to make it more clear. This part will be rewritten.
Regards, Simon
participants (3)
-
Simon Glass
-
Y.T. Tang
-
yuantian.tang@nxp.com