Page MenuHomeFreeBSD

mpr: endianess fix for set/get dpm page0
ClosedPublic

Authored by rew on Jan 18 2024, 8:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 17 2024, 8:55 AM
Unknown Object (File)
Mar 14 2024, 4:12 PM
Unknown Object (File)
Feb 24 2024, 5:32 PM
Unknown Object (File)
Feb 1 2024, 10:31 AM
Unknown Object (File)
Jan 20 2024, 7:53 PM
Unknown Object (File)
Jan 19 2024, 4:41 AM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rew requested review of this revision.Jan 18 2024, 8:04 PM

What about the header fields we have discussed?

This revision is now accepted and ready to land.Jan 18 2024, 8:27 PM
imp added inline comments.
sys/dev/mpr/mpr_config.c
737–738

I'd be tempted to add a comment /* already le */ here and maybe elsewhere. It gets confusing why some are swapped and others aren't otherwise.

Id also be tempted to always do the requests in host endian and do the happing before we submit. Likewise with the replies. Not sure how feasible that is though.

In D43505#991783, @imp wrote:

Id also be tempted to always do the requests in host endian and do the happing before we submit. Likewise with the replies. Not sure how feasible that is though.

I think this is the last place to do the swap before the request gets submitted...at least the way it is written right now.

Should mps(4) be updated with this as well? it looks like it has the same issue

sys/dev/mpr/mpr_config.c
737–738

yea, I know what you mean, especially the confusing part.

I think it's slightly confusing since there is some code duplication going on which could be reduced by converting the functions that read/write config pages to use mpr_read_config_page() and mpr_write_config_page().

Then, at least, there would be only a single set of comments explaining that replies from the controller are little-endian.

What about the header fields we have discussed?

It is grouped in with some other small fixes that I will start submitting reviews for in the coming week.

In D43505#992149, @rew wrote:
In D43505#991783, @imp wrote:

Id also be tempted to always do the requests in host endian and do the happing before we submit. Likewise with the replies. Not sure how feasible that is though.

I think this is the last place to do the swap before the request gets submitted...at least the way it is written right now.

Should mps(4) be updated with this as well? it looks like it has the same issue

I was under the impression that mps(4) didn't work at all on big endian. If that's not the case, it too needs the same treatement.

I'd also look at mpi3mr, new to the tree, to see what it needs in this area since it is also a MPT driver

This revision was automatically updated to reflect the committed changes.