Re: [PATCH 06/10] NTB: epf: Document legacy doorbell slot offset in ntb_epf_peer_db_set()

[Dave Jiang]

On 2/24/26 6:34 AM, Koichiro Den wrote:
> ntb_epf_peer_db_set() uses ffs(db_bits) to select a doorbell to ring.
> ffs() returns a 1-based bit index (bit 0 -> 1).
> 
> Entry 0 is reserved for link events, so doorbell bit 0 must map to entry
> 1. However, since the initial commit 812ce2f8d14e ("NTB: Add support for
> EPF PCI Non-Transparent Bridge"), the implementation has been adding an
> extra +1, ending up using entry 2 for bit 0. Fixing the extra increment
> would break interoperability with peers running older kernels.
> 
> Keep the legacy behavior and document the offset and the resulting slot
> layout to avoid confusion when enabling per-db-vector handling.
> 
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
>  drivers/ntb/hw/epf/ntb_hw_epf.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index d3ecf25a5162..bce7130fec39 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -43,6 +43,18 @@
>  #define NTB_EPF_DB_DATA(n)	(0x34 + (n) * 4)
>  #define NTB_EPF_DB_OFFSET(n)	(0xB4 + (n) * 4)
>  
> +/*
> + * Legacy doorbell slot layout when paired with pci-epf-*ntb:
> + *
> + *   slot 0 : reserved for link events
> + *   slot 1 : unused (historical extra offset)
> + *   slot 2 : DB#0
> + *   slot 3 : DB#1
> + *   ...
> + *
> + * Thus, NTB_EPF_MIN_DB_COUNT=3 means that we at least create vectors for
> + * doorbells DB#0 and DB#1.
> + */
>  #define NTB_EPF_MIN_DB_COUNT	3
>  #define NTB_EPF_MAX_DB_COUNT	31
>  
> @@ -473,6 +485,14 @@ static int ntb_epf_peer_mw_get_addr(struct ntb_dev *ntb, int idx,
>  static int ntb_epf_peer_db_set(struct ntb_dev *ntb, u64 db_bits)
>  {
>  	struct ntb_epf_dev *ndev = ntb_ndev(ntb);
> +	/*
> +	 * ffs() returns a 1-based bit index (bit 0 -> 1).
> +	 *
> +	 * With slot 0 reserved for link events, DB#0 would naturally map to
> +	 * slot 1. Historically an extra +1 offset was added, so DB#0 maps to
> +	 * slot 2 and slot 1 remains unused. Keep this mapping for
> +	 * backward-compatibility.
> +	 */
>  	u32 interrupt_num = ffs(db_bits) + 1;
>  	struct device *dev = ndev->dev;
>  	u32 db_entry_size;