-
Notifications
You must be signed in to change notification settings - Fork 703
logging: add nvme_mi_submit entry and exit functions #2807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
LGTM. |
jk-ozlabs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a couple of comments inline.
| .cdw15 = le32_to_cpu(hdr->cdw15), | ||
| }; | ||
|
|
||
| nvme_show_common(&cmd); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we would want include the dofst / dlen here too, otherwise the data may not make much sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added the doff and dlen mentioned and also fix to use le32_to_cpu() for the cdw0 result output.
| const void *data, size_t data_len) | ||
| { | ||
| printf("result : %08x\n", hdr->cdw0); | ||
| printf("err : %d\n", hdr->status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we don't want the data here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently nvme_submit_passthru() for PCIe does not print the data so at this moment not required as same but later possible to be added the data output also I think.
Only supported for the NVMe-MI message type NVMe admin command. Signed-off-by: Tokunori Ikegami <[email protected]>
c33e4aa to
900adf5
Compare
jk-ozlabs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not fully across the logging patterns in general, but +1 from me on the MI-related parts.
|
FWIW, I think it's safe to modify the logging later on. I don't consider this as a stable interface in any sorts. Let's start with this and see what works and what needs some more changes later on. |
|
Thanks! |
Only supported for the NVMe-MI message type NVMe admin command.