Hi nico,
I sent the patch inline with suggested changes.kindly have a look at it.

Regards,
Laxman

On Mon, 5 Nov 2018 at 14:07, Nicolas Dechesne <nicolas.dechesne@linaro.org> wrote:
hi Laxman!

thanks for the patch. Can you please send the patch inline (e.g. using
git send-email?) that is much easier to review.

more below

diff --git a/qdl.c b/qdl.c
index 8cf1011..cf493b4 100644
--- a/qdl.c
+++ b/qdl.c
@@ -227,7 +227,7 @@ static void print_usage(void)
 int main(int argc, char **argv)
 {
  struct termios tios;
- char *prog_mbn;
+ char *prog_mbn, *storage;
  char *incdir = NULL;
  int type;
  int ret;
@@ -266,6 +266,9 @@ int main(int argc, char **argv)
  return 1;
  }

+ storage = argv[1];
+ optind ++;
+

^^ I would prefer to use an optional arg in struct option instead.
What you are doing here means that it now becomes a mandatory arg and
would break all existing documentation and scripts. Instead you can
set it to ufs by default, and add an arg to let the users change it
if/when needed.

  prog_mbn = argv[optind++];

  do {
@@ -303,7 +306,7 @@ int main(int argc, char **argv)
  if (ret < 0)
  goto out;

- ret = firehose_run(fd, incdir);
+ ret = firehose_run(fd, incdir, storage);

 out:
  ret = tcsetattr(fd, TCSANOW, &tios);
diff --git a/qdl.h b/qdl.h
index 0ed2607..208ede0 100644
--- a/qdl.h
+++ b/qdl.h
@@ -6,7 +6,7 @@
 #include "patch.h"
 #include "program.h"

-int firehose_run(int fd, const char *incdir);
+int firehose_run(int fd, const char *incdir, const char *storage);
 int sahara_run(int fd, char *prog_mbn);
 void print_hex_dump(const char *prefix, const void *buf, size_t len);

--
2.7.4
On Mon, Nov 5, 2018 at 9:16 AM laxman siripuram <itsmelaxman91@gmail.com> wrote:
>
> Hi  nico,
> with the attached patch, i have validated qdl functionality on both emmc and ufs storage based boards. It is working fine.
>
> i have passed the storage type as  first argument to qdl as
> sudo ./qdl <storage> <program_file> <raw_program> <patch_file>
>
> kindly verify the patch and let me know if it is okay or not.
>
> Regards
> Laxman
>
>
>
>
> On Mon, 5 Nov 2018 at 05:30, Nicolas Dechesne <nicolas.dechesne@linaro.org> wrote:
>>
>> hi Laxman,
>>
>> I believe the context of your patch is:
>>
>> https://discuss.96boards.org/t/snapdragon820-recovery-package-for-emmc-storage/5945/2
>>
>> I suspect this change would break QDL on DB820c, so you are probably
>> not testing on DB820c... I agree that we need a mechanism to set
>> MemoryName to either ufs or emmc, instead of hardcoding it here so
>> that we can support both DB820c and 820 boards with eMMC... Do you
>> think you can write up a patch that has a new argument to configure
>> memory to be emmc or ufs and share with everyone?
>>
>> Just copy/pasting your patch for others to have a look.
>>
>> From 655bafe23ac4a9ad33bfe3ad8caa8be5e771a71c Mon Sep 17 00:00:00 2001
>> From: laxman <laxman@inforcecomputing.com>
>> Date: Fri, 2 Nov 2018 11:44:40 +0530
>> Subject: [PATCH] qdl:support for emmc based boards
>>
>> qdl for emmc storage
>>
>> Signed-off-by: laxman <laxman@inforcecomputing.com>
>> ---
>>  firehose.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/firehose.c b/firehose.c
>> index c4e9500..027264f 100644
>> --- a/firehose.c
>> +++ b/firehose.c
>> @@ -277,7 +277,7 @@ static int firehose_send_configure(int fd, size_t
>> payload_size, bool skip_storag
>>   xmlDocSetRootElement(doc, root);
>>
>>   node = xmlNewChild(root, NULL, (xmlChar*)"configure", NULL);
>> - xml_setpropf(node, "MemoryName", "ufs");
>> + xml_setpropf(node, "MemoryName", "emmc");
>>   xml_setpropf(node, "MaxPayloadSizeToTargetInBytes", "%d", payload_size);
>>   xml_setpropf(node, "verbose", "%d", 0);
>>   xml_setpropf(node, "ZLPAwareHost", "%d", 0);
>> --
>> 2.7.4
>> On Fri, Nov 2, 2018 at 7:41 AM laxman siripuram <itsmelaxman91@gmail.com> wrote:
>> >
>> > Hi,
>> > Added qdl support for emmc storage based devices. It is validated on sd660/sd820 chipset boards based on emmc storage.
>> >
>> >
>> > Regards,
>> > Laxman
>> > _______________________________________________
>> > dragonboard mailing list
>> > dragonboard@lists.96boards.org
>> > https://lists.96boards.org/mailman/listinfo/dragonboard