usb: add --list flag for list devices #176
Conversation
andersson
left a comment
andersson
left a comment
There was a problem hiding this comment.
I really like this feature, please find some requests for improvements inline.
Rather than the "feat: " subject prefix, I think "usb: " would be better. Also please do include a commit message stating why this patch exist (to discover the serial numbers to be passed to --serial I presume?).
Please also add you Signed-off-by (git commit --amend -s) to give your certificate of the origin of this code.
lucarin91
force-pushed
the
add-list-command
branch
from
59eb5b7 to
22af740
Compare
lucarin91
changed the title
lucarin91
force-pushed
the
add-list-command
branch
2 times, most recently
from
fa36cb8 to
a90be2c
Compare
lucarin91
changed the title
Hi, thanks for the review and the patience. I don't commonly program in C, so I am not aware of conventions and best practices. I implemented all the review comments. Can you please have a look again? I would also like some suggestions on the list output. Right now, it is very basic: Do you think we should extract the serial number and maybe the part that indicates the board type? I think it is |
lucarin91
force-pushed
the
add-list-command
branch
from
a90be2c to
9481606
Compare
andersson
left a comment
andersson
left a comment
There was a problem hiding this comment.
Thank you for the update, I provided some additional feedback.
I think we should parse out the SN: and print that. I'm not sure what the CID represents, the number I get from the board I have infront of me doesn't match anything in the Linux kernel socinfo table... But if you prefer you could parse that out as well, (I'd prefer storing that in a separate struct member) and then pretty print both.
Sorry for not thinking more about the qdl --list previously, but after trying this out a few times I really would prefer it to be "qdl list", I've provided a suggestion of how to implement this inline.
Again, that you for your contribution. This looks good, looking forward to your next revision!
andersson
force-pushed
the
add-list-command
branch
from
9481606 to
82a2396
Compare
andersson
left a comment
andersson
left a comment
There was a problem hiding this comment.
I took the liberty of updating the change in accordance to the feedback I had provided.
When working with multiple devices and the --serial argument, the serial numbers must be known. Add a new command "qdl list" to list the connected devices in EDL mode and their serial number. Signed-off-by: lucarin91 <lucarin@protonmail.com> [bjorn: Replaced --list with list, changed output to only print serial, some stylistic changes] Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
andersson
force-pushed
the
add-list-command
branch
from
82a2396 to
d45335d
Compare
Thanks. Sorry, but I haven't had time these days to work on that. |
2 participants
I think it could be useful to expose a command to list all EDL-compatible devices.