pci-epf-test advertises the doorbell target to the RC as a BAR number
and an offset, and the RC rings the doorbell with a single DWORD MMIO
write.

Some doorbell backends may report that the doorbell target is already
exposed via a platform-owned fixed BAR (db_msg[0].bar/offset). In that
case, reuse the pre-exposed window and do not reprogram the BAR with
pci_epc_set_bar().

Also honor db_msg[0].irq_flags when requesting the doorbell IRQ, and
only restore the original BAR mapping on disable if pci-epf-test
programmed it.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 87 +++++++++++++------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 12705858e502..f5a74108e180 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -86,6 +86,7 @@ struct pci_epf_test {
 	bool			dma_private;
 	const struct pci_epc_features *epc_features;
 	struct pci_epf_bar	db_bar;
+	bool			db_bar_programmed;
 	size_t			bar_size[PCI_STD_NUM_BARS];
 };
 
@@ -725,7 +726,9 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
 {
 	u32 status = le32_to_cpu(reg->status);
 	struct pci_epf *epf = epf_test->epf;
+	struct pci_epf_doorbell_msg *db;
 	struct pci_epc *epc = epf->epc;
+	unsigned long irq_flags;
 	struct msi_msg *msg;
 	enum pci_barno bar;
 	size_t offset;
@@ -735,13 +738,28 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
 	if (ret)
 		goto set_status_err;
 
-	msg = &epf->db_msg[0].msg;
-	bar = pci_epc_get_next_free_bar(epf_test->epc_features, epf_test->test_reg_bar + 1);
-	if (bar < BAR_0)
-		goto err_doorbell_cleanup;
+	db = &epf->db_msg[0];
+	msg = &db->msg;
+	epf_test->db_bar_programmed = false;
+
+	if (db->bar != NO_BAR) {
+		/*
+		 * The doorbell target is already exposed via a platform-owned
+		 * fixed BAR
+		 */
+		bar = db->bar;
+		offset = db->offset;
+	} else {
+		bar = pci_epc_get_next_free_bar(epf_test->epc_features,
+						epf_test->test_reg_bar + 1);
+		if (bar < BAR_0)
+			goto err_doorbell_cleanup;
+	}
+
+	irq_flags = epf->db_msg[0].irq_flags | IRQF_ONESHOT;
 
 	ret = request_threaded_irq(epf->db_msg[0].virq, NULL,
-				   pci_epf_test_doorbell_handler, IRQF_ONESHOT,
+				   pci_epf_test_doorbell_handler, irq_flags,
 				   "pci-ep-test-doorbell", epf_test);
 	if (ret) {
 		dev_err(&epf->dev,
@@ -753,22 +771,33 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
 	reg->doorbell_data = cpu_to_le32(msg->data);
 	reg->doorbell_bar = cpu_to_le32(bar);
 
-	msg = &epf->db_msg[0].msg;
-	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
-					 &epf_test->db_bar.phys_addr, &offset);
+	if (db->bar == NO_BAR) {
+		ret = pci_epf_align_inbound_addr(epf, bar,
+						 ((u64)msg->address_hi << 32) |
+						 msg->address_lo,
+						 &epf_test->db_bar.phys_addr,
+						 &offset);
 
-	if (ret)
-		goto err_free_irq;
+		if (ret)
+			goto err_free_irq;
+	}
+
+	if (size_add(offset, sizeof(u32)) > epf->bar[bar].size)
+		goto err_doorbell_cleanup;
 
 	reg->doorbell_offset = cpu_to_le32(offset);
 
-	epf_test->db_bar.barno = bar;
-	epf_test->db_bar.size = epf->bar[bar].size;
-	epf_test->db_bar.flags = epf->bar[bar].flags;
+	if (db->bar == NO_BAR) {
+		epf_test->db_bar.barno = bar;
+		epf_test->db_bar.size = epf->bar[bar].size;
+		epf_test->db_bar.flags = epf->bar[bar].flags;
 
-	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf_test->db_bar);
-	if (ret)
-		goto err_free_irq;
+		ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf_test->db_bar);
+		if (ret)
+			goto err_free_irq;
+
+		epf_test->db_bar_programmed = true;
+	}
 
 	status |= STATUS_DOORBELL_ENABLE_SUCCESS;
 	reg->status = cpu_to_le32(status);
@@ -798,17 +827,21 @@ static void pci_epf_test_disable_doorbell(struct pci_epf_test *epf_test,
 	free_irq(epf->db_msg[0].virq, epf_test);
 	pci_epf_test_doorbell_cleanup(epf_test);
 
-	/*
-	 * The doorbell feature temporarily overrides the inbound translation
-	 * to point to the address stored in epf_test->db_bar.phys_addr, i.e.,
-	 * it calls set_bar() twice without ever calling clear_bar(), as
-	 * calling clear_bar() would clear the BAR's PCI address assigned by
-	 * the host. Thus, when disabling the doorbell, restore the inbound
-	 * translation to point to the memory allocated for the BAR.
-	 */
-	ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
-	if (ret)
-		goto set_status_err;
+	if (epf_test->db_bar_programmed) {
+		/*
+		 * The doorbell feature temporarily overrides the inbound translation
+		 * to point to the address stored in epf_test->db_bar.phys_addr, i.e.,
+		 * it calls set_bar() twice without ever calling clear_bar(), as
+		 * calling clear_bar() would clear the BAR's PCI address assigned by
+		 * the host. Thus, when disabling the doorbell, restore the inbound
+		 * translation to point to the memory allocated for the BAR.
+		 */
+		ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
+		if (ret)
+			goto set_status_err;
+
+		epf_test->db_bar_programmed = false;
+	}
 
 	status |= STATUS_DOORBELL_DISABLE_SUCCESS;
 	reg->status = cpu_to_le32(status);
-- 
2.51.0

Hello Koichiro,

On Tue, Feb 17, 2026 at 05:06:00PM +0900, Koichiro Den wrote:
> pci-epf-test advertises the doorbell target to the RC as a BAR number
> and an offset, and the RC rings the doorbell with a single DWORD MMIO
> write.
> 
> Some doorbell backends may report that the doorbell target is already
> exposed via a platform-owned fixed BAR (db_msg[0].bar/offset). In that
> case, reuse the pre-exposed window and do not reprogram the BAR with
> pci_epc_set_bar().
> 
> Also honor db_msg[0].irq_flags when requesting the doorbell IRQ, and
> only restore the original BAR mapping on disable if pci-epf-test
> programmed it.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

(snip)

> @@ -753,22 +771,33 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
>  	reg->doorbell_data = cpu_to_le32(msg->data);
>  	reg->doorbell_bar = cpu_to_le32(bar);
>  
> -	msg = &epf->db_msg[0].msg;
> -	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
> -					 &epf_test->db_bar.phys_addr, &offset);
> +	if (db->bar == NO_BAR) {
> +		ret = pci_epf_align_inbound_addr(epf, bar,
> +						 ((u64)msg->address_hi << 32) |
> +						 msg->address_lo,
> +						 &epf_test->db_bar.phys_addr,
> +						 &offset);
>  
> -	if (ret)
> -		goto err_free_irq;
> +		if (ret)
> +			goto err_free_irq;
> +	}

I tried this series on Rock5b (RK3588), and was surprised to see the doorbell
test case still failing.


> +
> +	if (size_add(offset, sizeof(u32)) > epf->bar[bar].size)
> +		goto err_doorbell_cleanup;

It turns out that this check is the reason for it still failing.

You see, for a BAR that is marked as BAR_RESERVED, pci-epf-test will not
allocate backing memory, so epf->bar[bar].size will be 0.

If I removed this check, I could get the test case to pass.

As I suggested in my previous email, perhaps this check is better suited
in pci_epf_alloc_doorbell(). (As a DWORD alignment check inside
pci_epf_alloc_doorbell(). pci_epf_alloc_doorbell() could itself return
error if the doorbell is not DWORD aligned.)

That way, you could remove this check from pci_epf_test_enable_doorbell(),
and we don't need to care about epf->bar[bar].size.


Kind regards,
Niklas

On Tue, Feb 17, 2026 at 11:15:08AM +0100, Niklas Cassel wrote:
> Hello Koichiro,
> 
> On Tue, Feb 17, 2026 at 05:06:00PM +0900, Koichiro Den wrote:
> > pci-epf-test advertises the doorbell target to the RC as a BAR number
> > and an offset, and the RC rings the doorbell with a single DWORD MMIO
> > write.
> > 
> > Some doorbell backends may report that the doorbell target is already
> > exposed via a platform-owned fixed BAR (db_msg[0].bar/offset). In that
> > case, reuse the pre-exposed window and do not reprogram the BAR with
> > pci_epc_set_bar().
> > 
> > Also honor db_msg[0].irq_flags when requesting the doorbell IRQ, and
> > only restore the original BAR mapping on disable if pci-epf-test
> > programmed it.
> > 
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> 
> (snip)
> 
> > @@ -753,22 +771,33 @@ static void pci_epf_test_enable_doorbell(struct pci_epf_test *epf_test,
> >  	reg->doorbell_data = cpu_to_le32(msg->data);
> >  	reg->doorbell_bar = cpu_to_le32(bar);
> >  
> > -	msg = &epf->db_msg[0].msg;
> > -	ret = pci_epf_align_inbound_addr(epf, bar, ((u64)msg->address_hi << 32) | msg->address_lo,
> > -					 &epf_test->db_bar.phys_addr, &offset);
> > +	if (db->bar == NO_BAR) {
> > +		ret = pci_epf_align_inbound_addr(epf, bar,
> > +						 ((u64)msg->address_hi << 32) |
> > +						 msg->address_lo,
> > +						 &epf_test->db_bar.phys_addr,
> > +						 &offset);
> >  
> > -	if (ret)
> > -		goto err_free_irq;
> > +		if (ret)
> > +			goto err_free_irq;
> > +	}
> 
> I tried this series on Rock5b (RK3588), and was surprised to see the doorbell
> test case still failing.

Thank you very much for testing, and apologies for not being able to test on
RK3588 on my side right now.

> 
> 
> > +
> > +	if (size_add(offset, sizeof(u32)) > epf->bar[bar].size)
> > +		goto err_doorbell_cleanup;
> 
> It turns out that this check is the reason for it still failing.
> 
> You see, for a BAR that is marked as BAR_RESERVED, pci-epf-test will not
> allocate backing memory, so epf->bar[bar].size will be 0.
> 
> If I removed this check, I could get the test case to pass.
> 
> As I suggested in my previous email, perhaps this check is better suited
> in pci_epf_alloc_doorbell(). (As a DWORD alignment check inside
> pci_epf_alloc_doorbell(). pci_epf_alloc_doorbell() could itself return
> error if the doorbell is not DWORD aligned.)

Yes, and I mentioned there I would reconsider this when respinning this feature
series. For reference:
https://lore.kernel.org/linux-pci/jcjson6zedvhkpctwzfao2wfaaujtdfqsnnm3k25e2vpz2evf4@hbsegnyevisu/
I hadn't noticed the bar.size==0 possibility at that time.

I agree that this check is better placed in pci_epf_alloc_doorbell().
I'll respin accordingly.

Thanks again for the testing and review,
Koichiro

> 
> That way, you could remove this check from pci_epf_test_enable_doorbell(),
> and we don't need to care about epf->bar[bar].size.
> 
> 
> Kind regards,
> Niklas