Details
Details
- Reviewers
bret_ketchum_dell.com mav cem vangyzen - Group Reviewers
manpages - Commits
- rS366969: ntb: Add Intel Xeon Gen3 support
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Looks good to me.
| sys/dev/ntb/ntb_hw/ntb_hw_intel.c | ||
|---|---|---|
| 1306 ↗ | (On Diff #77900) | Might want to keep the comment and assert on default. |
Comment Actions
- A few fixups, switch on the NTB hardware type to make code a bit clearer.
- Update the man page.
Comment Actions
No objection. I don’t really want to be an NTB maintainer, fwiw. I skimmed maybe half of this and it seemed reasonable.
Comment Actions
Thanks for looking. If I don't get any feedback in the next few days I'll just go ahead and commit.
Comment Actions
I'm not an NTB expert, but LGTM.
| sys/dev/ntb/ntb_hw/ntb_hw_intel.c | ||
|---|---|---|
| 1146 ↗ | (On Diff #78164) | desired_vectors was just assigned to num_vectors, so this is always true. |
| 1155 ↗ | (On Diff #78164) | Indentation? |
| 1840 ↗ | (On Diff #78164) | Indentation? |
| 2147–2148 ↗ | (On Diff #78164) | Indentation? I assume this is a rare case, but adding the sizes to the error message could be helpful. |
| 3253–3256 ↗ | (On Diff #78164) | You could remove the condition and keep only limit = base + size;. Or maybe a different condition is needed? |
| 1306 ↗ | (On Diff #77900) | It would catch a scribbler earlier. |
| sys/dev/ntb/ntb_hw/ntb_hw_intel.c | ||
|---|---|---|
| 1155 ↗ | (On Diff #78164) | I think this can just be removed. |
| 3253–3256 ↗ | (On Diff #78164) | I believe we can just remove the conditional. |