Hi David,
Thanks a lot for taking the time to look at the driver, the feedback was
very helpful. I have reworked the series accordingly and will send a v3.
Below is how each point was addressed.
> the driver is riddled with runtime checks on the ntb_ops presence.
> none of those checks are required [...] all ops should be enforced at
> probing time when selecting the vendor ops. This could be a first
> preparation commit.
Done. v3 patch 1 ("raw/ntb: validate vendor ops at probe") is a pure
preparation commit: it validates the mandatory ops once, right after the
vendor ops table is selected at probe time, and removes all the per-call
NULL checks scattered across the common code.
> The handshake op should be mandatory and provided by both Intel and AMD
> implementations. This probably makes the mw_set_trans op a vendor
> specific concern which does not have to be exposed as a op.
Done. dev_handshake is now mandatory and implemented by both drivers; the
built-in scratchpad handshake that used to live in the common code moved
into the Intel driver. mw_set_trans is no longer an op - it is now a
vendor-internal detail of each driver's handshake/cleanup.
> I see no reason to have a separate interrupt handler. This should stay
> in common code. It just needs some abstractions of some operations:
> read_peer_config, peer_mw_validate, link_cleanup.
Done. There is now a single common interrupt handler. The AMD-specific
handler is gone and its logic is expressed through the ops you suggested:
read_peer_config (already present), and the newly abstracted
peer_mw_validate and link_cleanup, both mandatory and provided by Intel
and AMD.
> Only handle_hw_event seems optional, and it is better to keep optional
> ops to a minimum, this reduces the runtime checks.
Agreed. handle_hw_event is the only optional op; it is used by AMD for its
SMU/power-management events and is the single remaining NULL check in the
common interrupt handler.
> There is some rebase needed on the RN, and the dpdk-devbind.py (trivial).
> Please make sure to send a series freshly rebased so that CI can pass on
> next revision (did not happen on v2).
v3 is freshly rebased on current main (26.11-rc0); the release note and
dpdk-devbind.py conflicts are resolved and checkpatches/check-git-log are
clean locally.
Thanks again,
Raghavendra
