Skip to content

Conversation

@BenReed161
Copy link
Collaborator

No description provided.

}
if (num_dwords > 132) {
fprintf(stderr, "TLP data cannot exceed 132 dwords \n");
free(raw_tlp_dwords);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not mentioned in the commit message. The free() call looks a bit suspicious but its removal should probably have some comments with it.

.dest_port = port_id,
.tlp_type = tlp_type,
.tlp_length = tlp_length,
.ecrc = ecrc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For multi-byte field, need to handle endianness properly on input/output data.

lib/diag.c Outdated
};
for (int i = 0; i < tlp_in.tlp_length; i++)
{
tlp_in.raw_tlp_data[i] = *(raw_tlp_data + i);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to convert to the target endianness that the MRPC defined from host endianness.

ptr = str;
for (int i = 0; i < *num_dwords; i++) {
char *endptr;
(*dwords)[i] = (uint32_t)strtoul(ptr, &endptr, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if user incorrectly input value like '0x0102030405' which exceeds a DW size?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we handle this somewhere else?

cli/diag.c Outdated
{"enable_ecrc", 'e', "", CFG_NONE, &cfg.ecrc, no_argument,
"Enable the ecrc to be included at the end of the input data (Default: disabled)"},
{"tlp_data", 'd', "DW0 DW1 ... DW131", CFG_STRING, &cfg.raw_tlp_data, required_argument,
"DW to be sent as part of the raw TLP (Maximum 132 DWs)"},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we expect every DW must start with '0x', probably need to let user know.

cli/diag.c Outdated
fprintf(stderr, "Error with tlp data provided \n");
return -1;
}
if (num_dwords > 132) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

132 is used multiple times, please use a macro instead.

@kelvin-cao
Copy link
Collaborator

As a rule of thumb, if you've made corrections to a commit, consider squashing it and performing a hard push. It's better to avoid introducing additional commits to rectify an earlier one within the same pull request.

@BenReed161 BenReed161 force-pushed the add-tlp-inject-command branch 2 times, most recently from ffde207 to d5a6069 Compare April 25, 2025 20:58
lib/diag.c Outdated
.ecrc = ecrc
};
for (int i = 0; i < tlp_in.tlp_length; i++)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: '{' should be on the same line of 'for'

uint32_t tlp_type;
uint32_t tlp_length;
uint32_t ecrc;
uint32_t raw_tlp_data[132];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need the macro TLP_MAX_DWS in the header and replace this '132' instance as well.

@BenReed161 BenReed161 force-pushed the add-tlp-inject-command branch 2 times, most recently from 37b9a20 to 31ad75e Compare May 13, 2025 17:11
TLP injection command added to use the defined MRPC interface
to send a raw TLP to a given destination port. User defined type
and enable the ecrc.

Raw tlp data is accepted as a string of characters and converted to
appropriate dword type and sent as part of new structure defined by
spec. Up to 132 dwords of raw tlp data can be sent a time, this limit is
defined by a macro in the switchtec header file.
@BenReed161 BenReed161 force-pushed the add-tlp-inject-command branch from 31ad75e to f4fe1db Compare May 13, 2025 17:23
@kelvin-cao kelvin-cao merged commit 3bb6ce6 into Microsemi:master May 13, 2025
5 checks passed
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