| Message ID | 20220922161246.20586-1-Frank.Li@nxp.com |
|---|---|
| State | New |
| Headers | show |
> -----Original Message----- > From: Frank Li > Sent: Thursday, September 22, 2022 11:14 AM > To: maz@kernel.org; tglx@linutronix.de; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; shawnguo@kernel.org; > s.hauer@pengutronix.de; kw@linux.com; bhelgaas@google.com > Cc: linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-pci@vger.kernel.org; Peng Fan > <peng.fan@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>; > jdmason@kudzu.us; kernel@pengutronix.de; festevam@gmail.com; dl-linux- > imx <linux-imx@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com; > ntb@lists.linux.dev; lznuaa@gmail.com; imx@lists.linux.dev; > manivannan.sadhasivam@linaro.org > Subject: [PATCH v12 5/6] PCI: endpoint: pci-epf-vntb: Clean up > [Frank Li] @Bjorn Helgaas, ping Patches[1-4] was already picked by irqchip. I think patch[5-6] should go through pci subsystem. Any additional comments? > Remove unused field: epf_db_phy. > Remove __iomem before epf_db. > Change epf_db to u32* from void * > Remove duplicate check if (readl(ntb->epf_db + i * 4)). > Using sizeof(u32) instead of number 4 at all place. > > Clean up sparse build warning: > Using epf_db[i] instead of readl() because epf_db is located in local > memory and allocated by dma_alloc_coherent(). Sparse build warning > when > there are not __iomem at readl(). > Added __iomem force type convert in vntb_epf_peer_spad_read\write() > and > vntb_epf_spad_read\write(). This require strong order at read and write. > > Replace pci_epc_mem_free_addr() with pci_epf_free_space() at error handle > path to match pci_epf_alloc_space(). > > Cleanup warning found by scripts/kernel-doc > Fix indentation of the struct epf_ntb_ctrl > Consolidate term > host, host1 to HOST > vhost, vHost, Vhost, VHOST2 to VHOST > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/endpoint/functions/pci-epf-vntb.c | 157 ++++++++++-------- > 1 file changed, 90 insertions(+), 67 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c > b/drivers/pci/endpoint/functions/pci-epf-vntb.c > index 1466dd1904175..acea753af29ed 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c > @@ -11,7 +11,7 @@ > * Author: Kishon Vijay Abraham I <kishon@ti.com> > */ > > -/** > +/* > * +------------+ +---------------------------------------+ > * | | | | > * +------------+ | +--------------+ > @@ -99,20 +99,20 @@ enum epf_ntb_bar { > * NTB Driver NTB Driver > */ > struct epf_ntb_ctrl { > - u32 command; > - u32 argument; > - u16 command_status; > - u16 link_status; > - u32 topology; > - u64 addr; > - u64 size; > - u32 num_mws; > - u32 reserved; > - u32 spad_offset; > - u32 spad_count; > - u32 db_entry_size; > - u32 db_data[MAX_DB_COUNT]; > - u32 db_offset[MAX_DB_COUNT]; > + u32 command; > + u32 argument; > + u16 command_status; > + u16 link_status; > + u32 topology; > + u64 addr; > + u64 size; > + u32 num_mws; > + u32 reserved; > + u32 spad_offset; > + u32 spad_count; > + u32 db_entry_size; > + u32 db_data[MAX_DB_COUNT]; > + u32 db_offset[MAX_DB_COUNT]; > } __packed; > > struct epf_ntb { > @@ -136,8 +136,7 @@ struct epf_ntb { > > struct epf_ntb_ctrl *reg; > > - phys_addr_t epf_db_phy; > - void __iomem *epf_db; > + u32 *epf_db; > > phys_addr_t vpci_mw_phy[MAX_MW]; > void __iomem *vpci_mw_addr[MAX_MW]; > @@ -156,12 +155,14 @@ static struct pci_epf_header epf_ntb_header = { > }; > > /** > - * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host > + * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST) > * @ntb: NTB device that facilitates communication between HOST and > VHOST > * @link_up: true or false indicating Link is UP or Down > * > * Once NTB function in HOST invoke ntb_link_enable(), > - * this NTB function driver will trigger a link event to vhost. > + * this NTB function driver will trigger a link event to VHOST. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up) > { > @@ -175,9 +176,9 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool > link_up) > } > > /** > - * epf_ntb_configure_mw() - Configure the Outbound Address Space for > vhost > - * to access the memory window of host > - * @ntb: NTB device that facilitates communication between host and vhost > + * epf_ntb_configure_mw() - Configure the Outbound Address Space for > VHOST > + * to access the memory window of HOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * @mw: Index of the memory window (either 0, 1, 2 or 3) > * > * EP Outbound Window > @@ -194,7 +195,9 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool > link_up) > * | | | | > * | | | | > * +--------+ +-----------+ > - * VHost PCI EP > + * VHOST PCI EP > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw) > { > @@ -219,7 +222,7 @@ static int epf_ntb_configure_mw(struct epf_ntb *ntb, > u32 mw) > > /** > * epf_ntb_teardown_mw() - Teardown the configured OB ATU > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * @mw: Index of the memory window (either 0, 1, 2 or 3) > * > * Teardown the configured OB ATU configured in epf_ntb_configure_mw() > using > @@ -234,12 +237,12 @@ static void epf_ntb_teardown_mw(struct epf_ntb > *ntb, u32 mw) > } > > /** > - * epf_ntb_cmd_handler() - Handle commands provided by the NTB Host > + * epf_ntb_cmd_handler() - Handle commands provided by the NTB HOST > * @work: work_struct for the epf_ntb_epc > * > * Workqueue function that gets invoked for the two epf_ntb_epc > * periodically (once every 5ms) to see if it has received any commands > - * from NTB host. The host can send commands to configure doorbell or > + * from NTB HOST. The HOST can send commands to configure doorbell or > * configure memory window or to update link status. > */ > static void epf_ntb_cmd_handler(struct work_struct *work) > @@ -254,12 +257,9 @@ static void epf_ntb_cmd_handler(struct work_struct > *work) > ntb = container_of(work, struct epf_ntb, cmd_handler.work); > > for (i = 1; i < ntb->db_count; i++) { > - if (readl(ntb->epf_db + i * 4)) { > - if (readl(ntb->epf_db + i * 4)) > - ntb->db |= 1 << (i - 1); > - > + if (ntb->epf_db[i]) { > ntb_db_event(&ntb->ntb, i); > - writel(0, ntb->epf_db + i * 4); > + ntb->epf_db[i] = 0; > } > } > > @@ -321,8 +321,8 @@ static void epf_ntb_cmd_handler(struct work_struct > *work) > > /** > * epf_ntb_config_sspad_bar_clear() - Clear Config + Self scratchpad BAR > - * @ntb_epc: EPC associated with one of the HOST which holds peer's > outbound > - * address. > + * @ntb: EPC associated with one of the HOST which holds peer's outbound > + * address. > * > * Clear BAR0 of EP CONTROLLER 1 which contains the HOST1's config and > * self scratchpad region (removes inbound ATU configuration). While BAR0 > is > @@ -331,8 +331,10 @@ static void epf_ntb_cmd_handler(struct work_struct > *work) > * used for self scratchpad from epf_ntb_bar[BAR_CONFIG]. > * > * Please note the self scratchpad region and config region is combined to > - * a single region and mapped using the same BAR. Also note HOST2's peer > - * scratchpad is HOST1's self scratchpad. > + * a single region and mapped using the same BAR. Also note VHOST's peer > + * scratchpad is HOST's self scratchpad. > + * > + * Returns: void > */ > static void epf_ntb_config_sspad_bar_clear(struct epf_ntb *ntb) > { > @@ -347,13 +349,15 @@ static void epf_ntb_config_sspad_bar_clear(struct > epf_ntb *ntb) > > /** > * epf_ntb_config_sspad_bar_set() - Set Config + Self scratchpad BAR > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > - * Map BAR0 of EP CONTROLLER 1 which contains the HOST1's config and > + * Map BAR0 of EP CONTROLLER which contains the VHOST's config and > * self scratchpad region. > * > * Please note the self scratchpad region and config region is combined to > * a single region and mapped using the same BAR. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_config_sspad_bar_set(struct epf_ntb *ntb) > { > @@ -380,7 +384,7 @@ static int epf_ntb_config_sspad_bar_set(struct > epf_ntb *ntb) > /** > * epf_ntb_config_spad_bar_free() - Free the physical memory associated > with > * config + scratchpad region > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > */ > static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb) > { > @@ -393,11 +397,13 @@ static void epf_ntb_config_spad_bar_free(struct > epf_ntb *ntb) > /** > * epf_ntb_config_spad_bar_alloc() - Allocate memory for config + > scratchpad > * region > - * @ntb: NTB device that facilitates communication between HOST1 and > HOST2 > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > * Allocate the Local Memory mentioned in the above diagram. The size of > * CONFIG REGION is sizeof(struct epf_ntb_ctrl) and size of SCRATCHPAD > REGION > * is obtained from "spad-count" configfs entry. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb) > { > @@ -424,7 +430,7 @@ static int epf_ntb_config_spad_bar_alloc(struct > epf_ntb *ntb) > spad_count = ntb->spad_count; > > ctrl_size = sizeof(struct epf_ntb_ctrl); > - spad_size = 2 * spad_count * 4; > + spad_size = 2 * spad_count * sizeof(u32); > > if (!align) { > ctrl_size = roundup_pow_of_two(ctrl_size); > @@ -454,7 +460,7 @@ static int epf_ntb_config_spad_bar_alloc(struct > epf_ntb *ntb) > ctrl->num_mws = ntb->num_mws; > ntb->spad_size = spad_size; > > - ctrl->db_entry_size = 4; > + ctrl->db_entry_size = sizeof(u32); > > for (i = 0; i < ntb->db_count; i++) { > ntb->reg->db_data[i] = 1 + i; > @@ -465,11 +471,13 @@ static int epf_ntb_config_spad_bar_alloc(struct > epf_ntb *ntb) > } > > /** > - * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capaiblity > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capability > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > * Configure MSI/MSI-X capability for each interface with number of > * interrupts equal to "db_count" configfs entry. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_configure_interrupt(struct epf_ntb *ntb) > { > @@ -511,18 +519,22 @@ static int epf_ntb_configure_interrupt(struct > epf_ntb *ntb) > > /** > * epf_ntb_db_bar_init() - Configure Doorbell window BARs > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_db_bar_init(struct epf_ntb *ntb) > { > const struct pci_epc_features *epc_features; > - u32 align; > struct device *dev = &ntb->epf->dev; > - int ret; > struct pci_epf_bar *epf_bar; > - void __iomem *mw_addr; > enum pci_barno barno; > - size_t size = 4 * ntb->db_count; > + void *mw_addr; > + size_t size; > + u32 align; > + int ret; > + > + size = sizeof(u32) * ntb->db_count; > > epc_features = pci_epc_get_features(ntb->epf->epc, > ntb->epf->func_no, > @@ -557,14 +569,14 @@ static int epf_ntb_db_bar_init(struct epf_ntb *ntb) > return ret; > > err_alloc_peer_mem: > - pci_epc_mem_free_addr(ntb->epf->epc, epf_bar->phys_addr, > mw_addr, epf_bar->size); > + pci_epf_free_space(ntb->epf, mw_addr, barno, 0); > return -1; > } > > /** > * epf_ntb_db_bar_clear() - Clear doorbell BAR and free memory > * allocated in peer's outbound address space > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > */ > static void epf_ntb_db_bar_clear(struct epf_ntb *ntb) > { > @@ -580,8 +592,9 @@ static void epf_ntb_db_bar_clear(struct epf_ntb *ntb) > > /** > * epf_ntb_mw_bar_init() - Configure Memory window BARs > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_mw_bar_init(struct epf_ntb *ntb) > { > @@ -629,7 +642,7 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb) > > /** > * epf_ntb_mw_bar_clear() - Clear Memory window BARs > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > */ > static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb) > { > @@ -652,7 +665,7 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb > *ntb) > > /** > * epf_ntb_epc_destroy() - Cleanup NTB EPC interface > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB > interfaces > */ > @@ -665,7 +678,9 @@ static void epf_ntb_epc_destroy(struct epf_ntb *ntb) > /** > * epf_ntb_init_epc_bar() - Identify BARs to be used for each of the NTB > * constructs (scratchpad region, doorbell, memorywindow) > - * @ntb: NTB device that facilitates communication between HOST and > vHOST > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_init_epc_bar(struct epf_ntb *ntb) > { > @@ -706,11 +721,13 @@ static int epf_ntb_init_epc_bar(struct epf_ntb *ntb) > > /** > * epf_ntb_epc_init() - Initialize NTB interface > - * @ntb: NTB device that facilitates communication between HOST and > vHOST2 > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > * Wrapper to initialize a particular EPC interface and start the workqueue > - * to check for commands from host. This function will write to the > + * to check for commands from HOST. This function will write to the > * EP controller HW for configuring it. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_epc_init(struct epf_ntb *ntb) > { > @@ -777,7 +794,7 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb) > > /** > * epf_ntb_epc_cleanup() - Cleanup all NTB interfaces > - * @ntb: NTB device that facilitates communication between HOST1 and > HOST2 > + * @ntb: NTB device that facilitates communication between HOST and > VHOST > * > * Wrapper to cleanup all NTB interfaces. > */ > @@ -934,6 +951,8 @@ static const struct config_item_type ntb_group_type > = { > * > * Add configfs directory specific to NTB. This directory will hold > * NTB specific properties like db_count, spad_count, num_mws etc., > + * > + * Returns: Pointer to config_group > */ > static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf, > struct config_group *group) > @@ -1084,11 +1103,11 @@ static int vntb_epf_link_enable(struct ntb_dev > *ntb, > static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx) > { > struct epf_ntb *ntb = ntb_ndev(ndev); > - int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * 4; > + int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * > sizeof(u32); > u32 val; > - void __iomem *base = ntb->reg; > + void __iomem *base = (void __iomem *)ntb->reg; > > - val = readl(base + off + ct + idx * 4); > + val = readl(base + off + ct + idx * sizeof(u32)); > return val; > } > > @@ -1096,10 +1115,10 @@ static int vntb_epf_spad_write(struct ntb_dev > *ndev, int idx, u32 val) > { > struct epf_ntb *ntb = ntb_ndev(ndev); > struct epf_ntb_ctrl *ctrl = ntb->reg; > - int off = ctrl->spad_offset, ct = ctrl->spad_count * 4; > - void __iomem *base = ntb->reg; > + int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32); > + void __iomem *base = (void __iomem *)ntb->reg; > > - writel(val, base + off + ct + idx * 4); > + writel(val, base + off + ct + idx * sizeof(u32)); > return 0; > } > > @@ -1108,10 +1127,10 @@ static u32 vntb_epf_peer_spad_read(struct > ntb_dev *ndev, int pidx, int idx) > struct epf_ntb *ntb = ntb_ndev(ndev); > struct epf_ntb_ctrl *ctrl = ntb->reg; > int off = ctrl->spad_offset; > - void __iomem *base = ntb->reg; > + void __iomem *base = (void __iomem *)ntb->reg; > u32 val; > > - val = readl(base + off + idx * 4); > + val = readl(base + off + idx * sizeof(u32)); > return val; > } > > @@ -1120,9 +1139,9 @@ static int vntb_epf_peer_spad_write(struct > ntb_dev *ndev, int pidx, int idx, u32 > struct epf_ntb *ntb = ntb_ndev(ndev); > struct epf_ntb_ctrl *ctrl = ntb->reg; > int off = ctrl->spad_offset; > - void __iomem *base = ntb->reg; > + void __iomem *base = (void __iomem *)ntb->reg; > > - writel(val, base + off + idx * 4); > + writel(val, base + off + idx * sizeof(u32)); > return 0; > } > > @@ -1275,6 +1294,8 @@ static struct pci_driver vntb_pci_driver = { > * Invoked when a primary interface or secondary interface is bound to EPC > * device. This function will succeed only when EPC is bound to both the > * interfaces. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_bind(struct pci_epf *epf) > { > @@ -1359,6 +1380,8 @@ static struct pci_epf_ops epf_ntb_ops = { > * > * Probe NTB function driver when endpoint function bus detects a NTB > * endpoint function. > + * > + * Returns: Zero for success, or an error code in case of failure > */ > static int epf_ntb_probe(struct pci_epf *epf) > { > -- > 2.35.1
On Thu, Oct 06, 2022 at 02:25:01PM +0000, Frank Li wrote: [...] > [Frank Li] @Bjorn Helgaas, ping > Patches[1-4] was already picked by irqchip. > I think patch[5-6] should go through pci subsystem. > Any additional comments? Yes. Don't mix clean-ups with new features. This patch should be split it does too many things at once, group the clean ups in different patches and send them as a separate series. Lorenzo > > Remove unused field: epf_db_phy. > > Remove __iomem before epf_db. > > Change epf_db to u32* from void * > > Remove duplicate check if (readl(ntb->epf_db + i * 4)). > > Using sizeof(u32) instead of number 4 at all place. > > > > Clean up sparse build warning: > > Using epf_db[i] instead of readl() because epf_db is located in local > > memory and allocated by dma_alloc_coherent(). Sparse build warning > > when > > there are not __iomem at readl(). > > Added __iomem force type convert in vntb_epf_peer_spad_read\write() > > and > > vntb_epf_spad_read\write(). This require strong order at read and write. > > > > Replace pci_epc_mem_free_addr() with pci_epf_free_space() at error handle > > path to match pci_epf_alloc_space(). > > > > Cleanup warning found by scripts/kernel-doc > > Fix indentation of the struct epf_ntb_ctrl > > Consolidate term > > host, host1 to HOST > > vhost, vHost, Vhost, VHOST2 to VHOST > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/pci/endpoint/functions/pci-epf-vntb.c | 157 ++++++++++-------- > > 1 file changed, 90 insertions(+), 67 deletions(-) > > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c > > b/drivers/pci/endpoint/functions/pci-epf-vntb.c > > index 1466dd1904175..acea753af29ed 100644 > > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c > > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c > > @@ -11,7 +11,7 @@ > > * Author: Kishon Vijay Abraham I <kishon@ti.com> > > */ > > > > -/** > > +/* > > * +------------+ +---------------------------------------+ > > * | | | | > > * +------------+ | +--------------+ > > @@ -99,20 +99,20 @@ enum epf_ntb_bar { > > * NTB Driver NTB Driver > > */ > > struct epf_ntb_ctrl { > > - u32 command; > > - u32 argument; > > - u16 command_status; > > - u16 link_status; > > - u32 topology; > > - u64 addr; > > - u64 size; > > - u32 num_mws; > > - u32 reserved; > > - u32 spad_offset; > > - u32 spad_count; > > - u32 db_entry_size; > > - u32 db_data[MAX_DB_COUNT]; > > - u32 db_offset[MAX_DB_COUNT]; > > + u32 command; > > + u32 argument; > > + u16 command_status; > > + u16 link_status; > > + u32 topology; > > + u64 addr; > > + u64 size; > > + u32 num_mws; > > + u32 reserved; > > + u32 spad_offset; > > + u32 spad_count; > > + u32 db_entry_size; > > + u32 db_data[MAX_DB_COUNT]; > > + u32 db_offset[MAX_DB_COUNT]; > > } __packed; > > > > struct epf_ntb { > > @@ -136,8 +136,7 @@ struct epf_ntb { > > > > struct epf_ntb_ctrl *reg; > > > > - phys_addr_t epf_db_phy; > > - void __iomem *epf_db; > > + u32 *epf_db; > > > > phys_addr_t vpci_mw_phy[MAX_MW]; > > void __iomem *vpci_mw_addr[MAX_MW]; > > @@ -156,12 +155,14 @@ static struct pci_epf_header epf_ntb_header = { > > }; > > > > /** > > - * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host > > + * epf_ntb_link_up() - Raise link_up interrupt to Virtual Host (VHOST) > > * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * @link_up: true or false indicating Link is UP or Down > > * > > * Once NTB function in HOST invoke ntb_link_enable(), > > - * this NTB function driver will trigger a link event to vhost. > > + * this NTB function driver will trigger a link event to VHOST. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up) > > { > > @@ -175,9 +176,9 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool > > link_up) > > } > > > > /** > > - * epf_ntb_configure_mw() - Configure the Outbound Address Space for > > vhost > > - * to access the memory window of host > > - * @ntb: NTB device that facilitates communication between host and vhost > > + * epf_ntb_configure_mw() - Configure the Outbound Address Space for > > VHOST > > + * to access the memory window of HOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * @mw: Index of the memory window (either 0, 1, 2 or 3) > > * > > * EP Outbound Window > > @@ -194,7 +195,9 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool > > link_up) > > * | | | | > > * | | | | > > * +--------+ +-----------+ > > - * VHost PCI EP > > + * VHOST PCI EP > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_configure_mw(struct epf_ntb *ntb, u32 mw) > > { > > @@ -219,7 +222,7 @@ static int epf_ntb_configure_mw(struct epf_ntb *ntb, > > u32 mw) > > > > /** > > * epf_ntb_teardown_mw() - Teardown the configured OB ATU > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * @mw: Index of the memory window (either 0, 1, 2 or 3) > > * > > * Teardown the configured OB ATU configured in epf_ntb_configure_mw() > > using > > @@ -234,12 +237,12 @@ static void epf_ntb_teardown_mw(struct epf_ntb > > *ntb, u32 mw) > > } > > > > /** > > - * epf_ntb_cmd_handler() - Handle commands provided by the NTB Host > > + * epf_ntb_cmd_handler() - Handle commands provided by the NTB HOST > > * @work: work_struct for the epf_ntb_epc > > * > > * Workqueue function that gets invoked for the two epf_ntb_epc > > * periodically (once every 5ms) to see if it has received any commands > > - * from NTB host. The host can send commands to configure doorbell or > > + * from NTB HOST. The HOST can send commands to configure doorbell or > > * configure memory window or to update link status. > > */ > > static void epf_ntb_cmd_handler(struct work_struct *work) > > @@ -254,12 +257,9 @@ static void epf_ntb_cmd_handler(struct work_struct > > *work) > > ntb = container_of(work, struct epf_ntb, cmd_handler.work); > > > > for (i = 1; i < ntb->db_count; i++) { > > - if (readl(ntb->epf_db + i * 4)) { > > - if (readl(ntb->epf_db + i * 4)) > > - ntb->db |= 1 << (i - 1); > > - > > + if (ntb->epf_db[i]) { > > ntb_db_event(&ntb->ntb, i); > > - writel(0, ntb->epf_db + i * 4); > > + ntb->epf_db[i] = 0; > > } > > } > > > > @@ -321,8 +321,8 @@ static void epf_ntb_cmd_handler(struct work_struct > > *work) > > > > /** > > * epf_ntb_config_sspad_bar_clear() - Clear Config + Self scratchpad BAR > > - * @ntb_epc: EPC associated with one of the HOST which holds peer's > > outbound > > - * address. > > + * @ntb: EPC associated with one of the HOST which holds peer's outbound > > + * address. > > * > > * Clear BAR0 of EP CONTROLLER 1 which contains the HOST1's config and > > * self scratchpad region (removes inbound ATU configuration). While BAR0 > > is > > @@ -331,8 +331,10 @@ static void epf_ntb_cmd_handler(struct work_struct > > *work) > > * used for self scratchpad from epf_ntb_bar[BAR_CONFIG]. > > * > > * Please note the self scratchpad region and config region is combined to > > - * a single region and mapped using the same BAR. Also note HOST2's peer > > - * scratchpad is HOST1's self scratchpad. > > + * a single region and mapped using the same BAR. Also note VHOST's peer > > + * scratchpad is HOST's self scratchpad. > > + * > > + * Returns: void > > */ > > static void epf_ntb_config_sspad_bar_clear(struct epf_ntb *ntb) > > { > > @@ -347,13 +349,15 @@ static void epf_ntb_config_sspad_bar_clear(struct > > epf_ntb *ntb) > > > > /** > > * epf_ntb_config_sspad_bar_set() - Set Config + Self scratchpad BAR > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > - * Map BAR0 of EP CONTROLLER 1 which contains the HOST1's config and > > + * Map BAR0 of EP CONTROLLER which contains the VHOST's config and > > * self scratchpad region. > > * > > * Please note the self scratchpad region and config region is combined to > > * a single region and mapped using the same BAR. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_config_sspad_bar_set(struct epf_ntb *ntb) > > { > > @@ -380,7 +384,7 @@ static int epf_ntb_config_sspad_bar_set(struct > > epf_ntb *ntb) > > /** > > * epf_ntb_config_spad_bar_free() - Free the physical memory associated > > with > > * config + scratchpad region > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > */ > > static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb) > > { > > @@ -393,11 +397,13 @@ static void epf_ntb_config_spad_bar_free(struct > > epf_ntb *ntb) > > /** > > * epf_ntb_config_spad_bar_alloc() - Allocate memory for config + > > scratchpad > > * region > > - * @ntb: NTB device that facilitates communication between HOST1 and > > HOST2 > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > * Allocate the Local Memory mentioned in the above diagram. The size of > > * CONFIG REGION is sizeof(struct epf_ntb_ctrl) and size of SCRATCHPAD > > REGION > > * is obtained from "spad-count" configfs entry. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb) > > { > > @@ -424,7 +430,7 @@ static int epf_ntb_config_spad_bar_alloc(struct > > epf_ntb *ntb) > > spad_count = ntb->spad_count; > > > > ctrl_size = sizeof(struct epf_ntb_ctrl); > > - spad_size = 2 * spad_count * 4; > > + spad_size = 2 * spad_count * sizeof(u32); > > > > if (!align) { > > ctrl_size = roundup_pow_of_two(ctrl_size); > > @@ -454,7 +460,7 @@ static int epf_ntb_config_spad_bar_alloc(struct > > epf_ntb *ntb) > > ctrl->num_mws = ntb->num_mws; > > ntb->spad_size = spad_size; > > > > - ctrl->db_entry_size = 4; > > + ctrl->db_entry_size = sizeof(u32); > > > > for (i = 0; i < ntb->db_count; i++) { > > ntb->reg->db_data[i] = 1 + i; > > @@ -465,11 +471,13 @@ static int epf_ntb_config_spad_bar_alloc(struct > > epf_ntb *ntb) > > } > > > > /** > > - * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capaiblity > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * epf_ntb_configure_interrupt() - Configure MSI/MSI-X capability > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > * Configure MSI/MSI-X capability for each interface with number of > > * interrupts equal to "db_count" configfs entry. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_configure_interrupt(struct epf_ntb *ntb) > > { > > @@ -511,18 +519,22 @@ static int epf_ntb_configure_interrupt(struct > > epf_ntb *ntb) > > > > /** > > * epf_ntb_db_bar_init() - Configure Doorbell window BARs > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_db_bar_init(struct epf_ntb *ntb) > > { > > const struct pci_epc_features *epc_features; > > - u32 align; > > struct device *dev = &ntb->epf->dev; > > - int ret; > > struct pci_epf_bar *epf_bar; > > - void __iomem *mw_addr; > > enum pci_barno barno; > > - size_t size = 4 * ntb->db_count; > > + void *mw_addr; > > + size_t size; > > + u32 align; > > + int ret; > > + > > + size = sizeof(u32) * ntb->db_count; > > > > epc_features = pci_epc_get_features(ntb->epf->epc, > > ntb->epf->func_no, > > @@ -557,14 +569,14 @@ static int epf_ntb_db_bar_init(struct epf_ntb *ntb) > > return ret; > > > > err_alloc_peer_mem: > > - pci_epc_mem_free_addr(ntb->epf->epc, epf_bar->phys_addr, > > mw_addr, epf_bar->size); > > + pci_epf_free_space(ntb->epf, mw_addr, barno, 0); > > return -1; > > } > > > > /** > > * epf_ntb_db_bar_clear() - Clear doorbell BAR and free memory > > * allocated in peer's outbound address space > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > */ > > static void epf_ntb_db_bar_clear(struct epf_ntb *ntb) > > { > > @@ -580,8 +592,9 @@ static void epf_ntb_db_bar_clear(struct epf_ntb *ntb) > > > > /** > > * epf_ntb_mw_bar_init() - Configure Memory window BARs > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_mw_bar_init(struct epf_ntb *ntb) > > { > > @@ -629,7 +642,7 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb) > > > > /** > > * epf_ntb_mw_bar_clear() - Clear Memory window BARs > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > */ > > static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb) > > { > > @@ -652,7 +665,7 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb > > *ntb) > > > > /** > > * epf_ntb_epc_destroy() - Cleanup NTB EPC interface > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB > > interfaces > > */ > > @@ -665,7 +678,9 @@ static void epf_ntb_epc_destroy(struct epf_ntb *ntb) > > /** > > * epf_ntb_init_epc_bar() - Identify BARs to be used for each of the NTB > > * constructs (scratchpad region, doorbell, memorywindow) > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_init_epc_bar(struct epf_ntb *ntb) > > { > > @@ -706,11 +721,13 @@ static int epf_ntb_init_epc_bar(struct epf_ntb *ntb) > > > > /** > > * epf_ntb_epc_init() - Initialize NTB interface > > - * @ntb: NTB device that facilitates communication between HOST and > > vHOST2 > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > * Wrapper to initialize a particular EPC interface and start the workqueue > > - * to check for commands from host. This function will write to the > > + * to check for commands from HOST. This function will write to the > > * EP controller HW for configuring it. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_epc_init(struct epf_ntb *ntb) > > { > > @@ -777,7 +794,7 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb) > > > > /** > > * epf_ntb_epc_cleanup() - Cleanup all NTB interfaces > > - * @ntb: NTB device that facilitates communication between HOST1 and > > HOST2 > > + * @ntb: NTB device that facilitates communication between HOST and > > VHOST > > * > > * Wrapper to cleanup all NTB interfaces. > > */ > > @@ -934,6 +951,8 @@ static const struct config_item_type ntb_group_type > > = { > > * > > * Add configfs directory specific to NTB. This directory will hold > > * NTB specific properties like db_count, spad_count, num_mws etc., > > + * > > + * Returns: Pointer to config_group > > */ > > static struct config_group *epf_ntb_add_cfs(struct pci_epf *epf, > > struct config_group *group) > > @@ -1084,11 +1103,11 @@ static int vntb_epf_link_enable(struct ntb_dev > > *ntb, > > static u32 vntb_epf_spad_read(struct ntb_dev *ndev, int idx) > > { > > struct epf_ntb *ntb = ntb_ndev(ndev); > > - int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * 4; > > + int off = ntb->reg->spad_offset, ct = ntb->reg->spad_count * > > sizeof(u32); > > u32 val; > > - void __iomem *base = ntb->reg; > > + void __iomem *base = (void __iomem *)ntb->reg; > > > > - val = readl(base + off + ct + idx * 4); > > + val = readl(base + off + ct + idx * sizeof(u32)); > > return val; > > } > > > > @@ -1096,10 +1115,10 @@ static int vntb_epf_spad_write(struct ntb_dev > > *ndev, int idx, u32 val) > > { > > struct epf_ntb *ntb = ntb_ndev(ndev); > > struct epf_ntb_ctrl *ctrl = ntb->reg; > > - int off = ctrl->spad_offset, ct = ctrl->spad_count * 4; > > - void __iomem *base = ntb->reg; > > + int off = ctrl->spad_offset, ct = ctrl->spad_count * sizeof(u32); > > + void __iomem *base = (void __iomem *)ntb->reg; > > > > - writel(val, base + off + ct + idx * 4); > > + writel(val, base + off + ct + idx * sizeof(u32)); > > return 0; > > } > > > > @@ -1108,10 +1127,10 @@ static u32 vntb_epf_peer_spad_read(struct > > ntb_dev *ndev, int pidx, int idx) > > struct epf_ntb *ntb = ntb_ndev(ndev); > > struct epf_ntb_ctrl *ctrl = ntb->reg; > > int off = ctrl->spad_offset; > > - void __iomem *base = ntb->reg; > > + void __iomem *base = (void __iomem *)ntb->reg; > > u32 val; > > > > - val = readl(base + off + idx * 4); > > + val = readl(base + off + idx * sizeof(u32)); > > return val; > > } > > > > @@ -1120,9 +1139,9 @@ static int vntb_epf_peer_spad_write(struct > > ntb_dev *ndev, int pidx, int idx, u32 > > struct epf_ntb *ntb = ntb_ndev(ndev); > > struct epf_ntb_ctrl *ctrl = ntb->reg; > > int off = ctrl->spad_offset; > > - void __iomem *base = ntb->reg; > > + void __iomem *base = (void __iomem *)ntb->reg; > > > > - writel(val, base + off + idx * 4); > > + writel(val, base + off + idx * sizeof(u32)); > > return 0; > > } > > > > @@ -1275,6 +1294,8 @@ static struct pci_driver vntb_pci_driver = { > > * Invoked when a primary interface or secondary interface is bound to EPC > > * device. This function will succeed only when EPC is bound to both the > > * interfaces. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_bind(struct pci_epf *epf) > > { > > @@ -1359,6 +1380,8 @@ static struct pci_epf_ops epf_ntb_ops = { > > * > > * Probe NTB function driver when endpoint function bus detects a NTB > > * endpoint function. > > + * > > + * Returns: Zero for success, or an error code in case of failure > > */ > > static int epf_ntb_probe(struct pci_epf *epf) > > { > > -- > > 2.35.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--- a/arch/arm64/boot/dts/freescale/imx8-ss-hsio.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8-ss-hsio.dtsi @@ -160,5 +160,6 @@ pcieb_ep: pcie_ep@5f010000 { num-ib-windows = <6>; num-ob-windows = <6>; status = "disabled"; + MSI-parent = <&lsio_mu12>; }; --- a/arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi @@ -172,6 +172,19 @@ lsio_mu6: mailbox@5d210000 { status = "disabled"; }; + lsio_mu12: mailbox@5d270000 { + compatible = "fsl,imx6sx-mu-MSI"; + msi-controller; + interrupt-controller; + reg = <0x5d270000 0x10000>, /* A side */ + <0x5d300000 0x10000>; /* B side */ + reg-names = "a", "b"; + interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&pd IMX_SC_R_MU_12A>, + <&pd IMX_SC_R_MU_12B>; + power-domain-names = "a", "b"; + }; + Change Log - Change from v11 to v12 pcie:
┌───────┐ ┌──────────┐ │ │ │ │ ┌─────────────┐ │ │ │ PCI Host │ │ MSI │◄┐ │ │ │ │ │ Controller │ │ │ │ │ │ └─────────────┘ └─┼───────┼──────────┼─Bar0 │ │ PCI │ │ Bar1 │ │ Func │ │ Bar2 │ │ │ │ Bar3 │ │ │ │ Bar4 │ │ ├─────────►│ │ └───────┘ └──────────┘ Many PCI controllers provided Endpoint functions. Generally PCI endpoint is hardware, which is not running a rich OS, like linux. But Linux also supports endpoint functions. PCI Host write BAR<n> space like write to memory. The EP side can't know memory changed by the Host driver. PCI Spec has not defined a standard method to do that. Only define MSI(x) to let EP notified RC status change. The basic idea is to trigger an IRQ when PCI RC writes to a memory address. That's what MSI controller provided. EP drivers just need to request a platform MSI interrupt, struct MSI_msg *msg will pass down a memory address and data. EP driver will map such memory address to one of PCI BAR<n>. Host just writes such an address to trigger EP side IRQ. If system have gic-its, only need update PCI EP side driver. But i.MX have not chip support gic-its yet. So we have to use MU to simulate a MSI controller. Although only 4 MSI IRQs are simulated, it matched vntb(pci-epf-vntb) network requirement. After enable MSI, ping delay reduce < 1ms from ~8ms IRQchip: imx mu worked as MSI controller: let imx mu worked as MSI controllers. Although IP is not design as MSI controller, we still can use it if limited IRQ number to 4. pcie: endpoint: pci-epf-vntb: add endpoint MSI support Based on ntb-next branch. https://github.com/jonmason/ntb/commits/ntb-next Using MSI as door bell registers mu-msi patches (1-4) and vntb patch(5-6) is totally independently. These can be applied by irqchip and pci's maintainer seperatedly. i.MX EP function driver is upstreaming by Richard Zhu. Some dts change missed at this patches. below is reference dts change fix typo in commit message change subject in commit message - Change from v10 to v11 irqchips: no change pcie: clean up build warning foundby kernel test robot clean up kernel-doc warning. clean up vhost VHost. - Change from v9 to v10 fixed build error reported by kernel test robot <lkp@intel.com> irqchips: fixed accoring to Marc Zyngier's comments Added new patch platform-msi: export symbol platform_msi_create_irq_domain() Using one lock for both reg and alloc msi irq Using predefined macro to init cfg data pcie: endpoint: fixed according to Manivannan Sadhasivam's feedback Added makeup patch before enable msi irq PCI: endpoint: makeup pci-epf-vntb.c - Change from v8 to v9 fix dt_bind_check error - Change from v7 to v8 irqchip: using name process-a-side as resource bind name pcie: endpoint: - fix build error reported by kernel test robot <lkp@intel.com> - rename epf_db_phy to epf_db_phys - rework error message - rework commit message - change ntb to vtb at apply irq. - kept name msi_virqbase because it is msi irq base number, not base address. - Change from v6 to v7 pcie: endpoint: add endpoint MSI support Fine tuning commit message Fixed issues, reviewed by Bjorn Helgaas - Change from v5 to v6 Fixed build error found by kernel test robot - Change from v4 to v5 Fixed dt-binding document add msi-cell add interrupt max number update naming reg-names and power-domain-names. Fixed irqchip-Add-IMX-MU-MSI-controller-driver.patch rework commit message remove some field in struct imx_mu_dcfg error handle when link power domain failure. add irq_domain_update_bus_token - Change from v3 to v4 Fixed dt-binding document according to Krzysztof Kozlowski's feedback Fixed irqchip-imx-mu-worked-as-msi-controller according to Marc Zyngier's comments. There are still two important points, which I am not sure. 1. clean irq_set_affinity after platform_msi_create_irq_domain. Some function, like platform_msi_write_msg() is static. so I have to set MSI_FLAG_USE_DEF_CHIP_OPS flags, which will set irq_set_affinity to default one. 2. about comments > + msi_data->msi_domain = platform_msi_create_irq_domain( > + of_node_to_fwnode(msi_data->pdev->dev.of_node), > + &imx_mu_msi_domain_info, > + msi_data->parent); "And you don't get an error due to the fact that you use the same fwnode for both domains without overriding the domain bus token?" I did not understand yet. Fixed static check warning, reported by Dan Carpenter pcie: endpoint: pci-epf-vntb: add endpoint MSI support - Change from v2 to v3 Fixed dt-binding docment check failure Fixed typo a cover letter. Change according Bjorn's comments at patch pcie: endpoint: pci-epf-vntb: add endpoint MSI support - from V1 to V2 Fixed fsl,mu-msi.yaml's problem Fixed irq-imx-mu-msi.c problem according Marc Zyngier's feeback Added a new patch to allow pass down .pm by IRQCHIP_PLATFORM_DRIVER_END Frank Li (6): platform-msi: export symbol platform_msi_create_irq_domain() irqchip: allow pass down .pm field at IRQCHIP_PLATFORM_DRIVER_END irqchip: Add IMX MU MSI controller driver dt-bindings: irqchip: imx mu work as msi controller PCI: endpoint: cleanup pci-epf-vntb.c PCI: endpoint: Add vNTB MSI support .../interrupt-controller/fsl,mu-msi.yaml | 99 ++++ drivers/base/platform-msi.c | 1 + drivers/irqchip/Kconfig | 14 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-imx-mu-msi.c | 455 ++++++++++++++++++ drivers/pci/endpoint/functions/pci-epf-vntb.c | 296 ++++++++---- include/linux/irqchip.h | 4 +- 7 files changed, 786 insertions(+), 84 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,mu-msi.yaml create mode 100644 drivers/irqchip/irq-imx-mu-msi.c