Re: [PATCH v1 1/3] raw/ntb: generalize framework for multiple vendors

[Raghavendra Ningoji <raghavendra.ningoji@amd.com>]

On Thu, Sep 18, 2026 at 03:06:49PM +0000, Bruce Richardson wrote:
> > +	void (*interrupt_handler)(void *param);
> > +	/* Optional vendor-specific handshake. If NULL, the built-in
> > +	 * scratchpad handshake is used. Used by hardware (e.g. AMD) whose
> > +	 * scratchpad layout differs from the built-in protocol.
> > +	 */
> > +	int (*dev_handshake)(const struct rte_rawdev *dev);
> > +	/* Optional vendor-specific peer-config read at device start. If NULL,
> > +	 * the built-in scratchpad reads are used.
> > +	 */
> > +	int (*read_peer_config)(const struct rte_rawdev *dev);
> >  };
>
> For these new op fields, do you foresee cases where other drivers might use
> the "default" functions as you have now? Might it be better to simplify
> things and always use driver-supplied ops, converting the existing
> functions into intel-specific ops, rather than making them fallback
> functions?

Thanks Bruce, that makes sense - I agree the uniform "always call the op"
style is cleaner than the NULL-fallback branches.

But the built-in handshake (ntb_handshake_work) and the peer-config read 
in ntb_dev_start  implement the  scratchpad  protocol considering Intel-specific scratchpad layout
 -  vendor with similar  intel scratchpad layout could reuse them . AMD is the exception because
it has only 8 usable scratchpad registers per side and needs its own
packed layout. - Future vendors with similar  AMD scratchpad layout would reuse them.

If you think still it's essentially necessary  will take up in next iteration/v2:
Rather than hiding them as fallbacks, keep them as named, 
reusable default ops and wire them into the Intel driver explicitly:

  - export ntb_handshake_work and set .dev_handshake in intel_ntb_ops;
  - factor the inline peer-config read in ntb_dev_start() into
    ntb_spad_read_peer_config() and set .read_peer_config for Intel;
  - set .interrupt_handler = ntb_dev_intr_handler for Intel;
  - drop the "if (op != NULL) ... else ..." branches so the common code
    always calls the op.

That removes the implicit fallbacks you are flagging while still letting any
future standard-scratchpad vendor opt into the generic implementations.


Thanks,
Raghavendra