Re: [PATCH v2 3/5] PCI: endpoint: pci-epf-test: add doorbell test
From: Manivannan Sadhasivam <mani@kernel.org>
Date: 2023-10-20 17:44:42
Also in:
imx, linux-devicetree, linux-pci, lkml
On Mon, Sep 11, 2023 at 06:09:18PM -0400, Frank Li wrote: Subject could be, PCI: endpoint: pci-epf-test: Add doorbell support
Add three register: doorbell_bar, doorbell_addr, doorbell_data, doorbell_done. Call pci_epf_alloc_doorbell() all a doorbell address space. Root complex(RC) side driver can trigger pci-epc-test's doorbell callback handler by write doorbell_data to mapped doorbell_bar's address space. pci-epc-test will set doorbell_done in doorbell callback.
How about, Add doorbell support to the EPF test driver by introducing 3 new registers: doorbell_bar doorbell_addr doorbell_data The PCI RC driver can trigger the doorbell on the EP side by writing the content of "doorbell_data" to the address specified by the "doorbell_addr" register in the "doorbell_bar" BAR region.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
You should also update Documentation/PCI/endpoint/pci-test-* files in a separate commit with doorbell support.
quoted hunk ↗ jump to hunk
--- drivers/pci/endpoint/functions/pci-epf-test.c | 59 ++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index 1f0d2b84296a3..566549919b87b 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c@@ -11,6 +11,7 @@ #include <linux/dmaengine.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/msi.h> #include <linux/slab.h> #include <linux/pci_ids.h> #include <linux/random.h>@@ -39,17 +40,21 @@ #define STATUS_IRQ_RAISED BIT(6) #define STATUS_SRC_ADDR_INVALID BIT(7) #define STATUS_DST_ADDR_INVALID BIT(8) +#define STATUS_DOORBELL_SUCCESS BIT(9) #define FLAG_USE_DMA BIT(0) #define TIMER_RESOLUTION 1 +#define MAGIC_VERSION_MASK GENMASK(7, 0) + static struct workqueue_struct *kpcitest_workqueue; struct pci_epf_test { void *reg[PCI_STD_NUM_BARS]; struct pci_epf *epf; enum pci_barno test_reg_bar; + enum pci_barno doorbell_bar; size_t msix_table_offset; struct delayed_work cmd_handler; struct dma_chan *dma_chan_tx;@@ -74,6 +79,9 @@ struct pci_epf_test_reg { u32 irq_type; u32 irq_number; u32 flags; + u32 doorbell_bar; + u32 doorbell_addr; + u32 doorbell_data; } __packed; static struct pci_epf_header test_header = {@@ -693,6 +701,8 @@ static void pci_epf_test_unbind(struct pci_epf *epf) struct pci_epf_bar *epf_bar; int bar; + pci_epf_free_doorbell(epf); + cancel_delayed_work(&epf_test->cmd_handler); pci_epf_test_clean_dma_chan(epf_test); for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {@@ -808,9 +818,22 @@ static int pci_epf_test_link_up(struct pci_epf *epf) return 0; } +static int pci_epf_test_doorbell(struct pci_epf *epf, int index) +{ + struct pci_epf_test *epf_test = epf_get_drvdata(epf); + enum pci_barno test_reg_bar = epf_test->test_reg_bar; + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; + + reg->status |= STATUS_DOORBELL_SUCCESS; + pci_epf_test_raise_irq(epf_test, reg); + + return 0; +} + static const struct pci_epc_event_ops pci_epf_test_event_ops = { .core_init = pci_epf_test_core_init, .link_up = pci_epf_test_link_up, + .doorbell = pci_epf_test_doorbell,
I would like to pass this callback directly to the pci_epf_alloc_doorbell() API. Would that be feasible?
quoted hunk ↗ jump to hunk
}; static int pci_epf_test_alloc_space(struct pci_epf *epf)@@ -859,7 +882,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf) epf_bar = &epf->bar[bar]; add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1; - if (bar == test_reg_bar) + if (bar == test_reg_bar || bar == epf_test->doorbell_bar) continue; if (!!(epc_features->reserved_bar & (1 << bar)))@@ -900,9 +923,14 @@ static int pci_epf_test_bind(struct pci_epf *epf) struct pci_epf_test *epf_test = epf_get_drvdata(epf); const struct pci_epc_features *epc_features; enum pci_barno test_reg_bar = BAR_0; + enum pci_barno doorbell_bar = NO_BAR; struct pci_epc *epc = epf->epc; bool linkup_notifier = false; bool core_init_notifier = false; + struct pci_epf_test_reg *reg; + struct msi_msg *msg; + u64 doorbell_addr; + u32 align; if (WARN_ON_ONCE(!epc)) return -EINVAL;@@ -923,10 +951,39 @@ static int pci_epf_test_bind(struct pci_epf *epf) epf_test->test_reg_bar = test_reg_bar; epf_test->epc_features = epc_features; + align = epc_features->align; + align = align ? align : 128; + + ret = pci_epf_alloc_doorbell(epf, 1);
This should be renamed as pci_epc_alloc_doorbell() as per comment on patch 1/3. Also, the "msi_msg" pointer should be part of the EPC struct.
+ if (!ret) {
+ msg = epf->msg;
+ doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
+
+ if (doorbell_bar > 0) {
+ epf_test->doorbell_bar = doorbell_bar;
+ doorbell_addr = msg->address_hi;
+ doorbell_addr <<= 32;
+ doorbell_addr |= msg->address_lo;
+ epf->bar[doorbell_bar].phys_addr = round_down(doorbell_addr, align);
+ epf->bar[doorbell_bar].barno = doorbell_bar;
+ epf->bar[doorbell_bar].size = align;
+ } else {
+ pci_epf_free_doorbell(epf);This one too should be renamed.
+ } + } + ret = pci_epf_test_alloc_space(epf);
This one too.
if (ret) return ret; + reg = epf_test->reg[test_reg_bar]; + reg->magic |= FIELD_PREP(MAGIC_VERSION_MASK, 0x1);
Why are you writing this register? This register serves for the purpose of testing BAR0. - Mani
+ if (doorbell_bar > 0) {
+ reg->doorbell_addr = doorbell_addr & (align - 1);
+ reg->doorbell_data = msg->data;
+ reg->doorbell_bar = doorbell_bar;
+ }
+
if (!core_init_notifier) {
ret = pci_epf_test_core_init(epf);
if (ret)
--
2.34.1-- மணிவண்ணன் சதாசிவம் _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel