Hi Simon, Please see my reply inline.
> -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of Simon Glass > Sent: Saturday, November 12, 2016 12:18 AM > To: Y.T. Tang <[email protected]> > Cc: Bin Meng <[email protected]>; U-Boot Mailing List <u- > [email protected]> > Subject: Re: [PATCH] sata: fix sata command not being executed bug > > Hi Tang, > > On 9 November 2016 at 02:37, <[email protected]> wrote: > > From: Tang Yuantian <[email protected]> > > > > 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 <[email protected]> > > --- > > 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 _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
