Re: [RFC PATCH v2 10/27] NTB: core: Add .get_pci_epc() to ntb_dev_ops

[Dave Jiang <dave.jiang@intel.com>]

On 12/1/25 11:32 PM, Koichiro Den wrote:
> On Mon, Dec 01, 2025 at 02:08:14PM -0700, Dave Jiang wrote:
>>
>>
>> On 11/29/25 9:03 AM, Koichiro Den wrote:
>>> Add an optional get_pci_epc() callback to retrieve the underlying
>>> pci_epc device associated with the NTB implementation.
>>>
>>> Signed-off-by: Koichiro Den <den@valinux.co.jp>
>>> ---
>>>  drivers/ntb/hw/epf/ntb_hw_epf.c | 11 +----------
>>>  include/linux/ntb.h             | 21 +++++++++++++++++++++
>>>  2 files changed, 22 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
>>> index a3ec411bfe49..d55ce6b0fad4 100644
>>> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
>>> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
>>> @@ -9,6 +9,7 @@
>>>  #include <linux/delay.h>
>>>  #include <linux/module.h>
>>>  #include <linux/pci.h>
>>> +#include <linux/pci-epf.h>
>>>  #include <linux/slab.h>
>>>  #include <linux/ntb.h>
>>>  
>>> @@ -49,16 +50,6 @@
>>>  
>>>  #define NTB_EPF_COMMAND_TIMEOUT	1000 /* 1 Sec */
>>>  
>>> -enum pci_barno {
>>> -	NO_BAR = -1,
>>> -	BAR_0,
>>> -	BAR_1,
>>> -	BAR_2,
>>> -	BAR_3,
>>> -	BAR_4,
>>> -	BAR_5,
>>> -};
>>> -
>>>  enum epf_ntb_bar {
>>>  	BAR_CONFIG,
>>>  	BAR_PEER_SPAD,
>>> diff --git a/include/linux/ntb.h b/include/linux/ntb.h
>>> index d7ce5d2e60d0..04dc9a4d6b85 100644
>>> --- a/include/linux/ntb.h
>>> +++ b/include/linux/ntb.h
>>> @@ -64,6 +64,7 @@ struct ntb_client;
>>>  struct ntb_dev;
>>>  struct ntb_msi;
>>>  struct pci_dev;
>>> +struct pci_epc;
>>>  
>>>  /**
>>>   * enum ntb_topo - NTB connection topology
>>> @@ -256,6 +257,7 @@ static inline int ntb_ctx_ops_is_valid(const struct ntb_ctx_ops *ops)
>>>   * @msg_clear_mask:	See ntb_msg_clear_mask().
>>>   * @msg_read:		See ntb_msg_read().
>>>   * @peer_msg_write:	See ntb_peer_msg_write().
>>> + * @get_pci_epc:	See ntb_get_pci_epc().
>>>   */
>>>  struct ntb_dev_ops {
>>>  	int (*port_number)(struct ntb_dev *ntb);
>>> @@ -331,6 +333,7 @@ struct ntb_dev_ops {
>>>  	int (*msg_clear_mask)(struct ntb_dev *ntb, u64 mask_bits);
>>>  	u32 (*msg_read)(struct ntb_dev *ntb, int *pidx, int midx);
>>>  	int (*peer_msg_write)(struct ntb_dev *ntb, int pidx, int midx, u32 msg);
>>> +	struct pci_epc *(*get_pci_epc)(struct ntb_dev *ntb);
>>
>> Seems very specific call to this particular hardware instead of something generic for the NTB dev ops. Maybe it should be something like get_private_data() or something like that?
> 
> Thank you for the suggestion.
> 
> I also felt that it's too specific, but I couldn't come up with a clean
> generic interface at the time, so I left it in this form.
> 
> .get_private_data() might indeed be better. In the callback doc comment we
> could describe it as "may be used to obtain a backing PCI controller
> pointer"?

I would add that comment in the defined callback for the hardware driver. For the actual API, it would be something like "for retrieving private data specific to the hardware driver"?

DJ

> 
> -Koichiro
> 
>>
>> DJ
>>
>>
>>>  };
>>>  
>>>  static inline int ntb_dev_ops_is_valid(const struct ntb_dev_ops *ops)
>>> @@ -393,6 +396,9 @@ static inline int ntb_dev_ops_is_valid(const struct ntb_dev_ops *ops)
>>>  		/* !ops->msg_clear_mask == !ops->msg_count	&& */
>>>  		!ops->msg_read == !ops->msg_count		&&
>>>  		!ops->peer_msg_write == !ops->msg_count		&&
>>> +
>>> +		/* Miscellaneous optional callbacks */
>>> +		/* ops->get_pci_epc			&& */
>>>  		1;
>>>  }
>>>  
>>> @@ -1567,6 +1573,21 @@ static inline int ntb_peer_msg_write(struct ntb_dev *ntb, int pidx, int midx,
>>>  	return ntb->ops->peer_msg_write(ntb, pidx, midx, msg);
>>>  }
>>>  
>>> +/**
>>> + * ntb_get_pci_epc() - get backing PCI endpoint controller if possible.
>>> + * @ntb:	NTB device context.
>>> + *
>>> + * Get the backing PCI endpoint controller representation.
>>> + *
>>> + * Return: A pointer to the pci_epc instance if available. or %NULL if not.
>>> + */
>>> +static inline struct pci_epc __maybe_unused *ntb_get_pci_epc(struct ntb_dev *ntb)
>>> +{
>>> +	if (!ntb->ops->get_pci_epc)
>>> +		return NULL;
>>> +	return ntb->ops->get_pci_epc(ntb);
>>> +}
>>> +
>>>  /**
>>>   * ntb_peer_resource_idx() - get a resource index for a given peer idx
>>>   * @ntb:	NTB device context.
>>