Skip to content

Conversation

@ikegami-t
Copy link
Contributor

Only supported for the NVMe-MI message type NVMe admin command.

@igaw
Copy link
Collaborator

igaw commented May 13, 2025

LGTM.

@jk-ozlabs

Copy link
Collaborator

@jk-ozlabs jk-ozlabs left a 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);
Copy link
Collaborator

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.

Copy link
Contributor Author

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);
Copy link
Collaborator

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?

Copy link
Contributor Author

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]>
Copy link
Collaborator

@jk-ozlabs jk-ozlabs left a 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.

@igaw
Copy link
Collaborator

igaw commented May 14, 2025

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.

@igaw igaw merged commit b1f4fad into linux-nvme:master May 14, 2025
17 checks passed
@igaw
Copy link
Collaborator

igaw commented May 14, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants