PCI: endpoint: IRQ callback fixes and cleanups
Subsystems (10)
v3 cover PCI: endpoint: IRQ callback fixes and cleanups
Hello all,
The first two patches in this series are IRQ callback fixes that should get backported.
The reason why the bugs existed in the first place is because the APIs are very confusing. The rest of the patches are cleanups of the APIs. These cleanups should not get backported.
Changes since v2:
-Reorder patches to have more logical ordering
-Capitalize first letter after prefix (Krzysztof)
-Improved commit message of the cleanup patches (Damien) -Renamed parameter interrupts to nr_irqs (Damien)
-Picked up tags. Did not pick up tags on patches that changed significantly
Niklas Cassel (6):
PCI: dwc: ep: Fix broken set_msix() callback
PCI: cadence-ep: Fix broken set_msix() callback
PCI: endpoint: Cleanup get_msi() callback
PCI: endpoint: Cleanup get_msix() callback
PCI: endpoint: Cleanup set_msi() callback
PCI: endpoint: Cleanup set_msix() callback
.../pci/controller/cadence/pcie-cadence-ep.c | 14 +++++----- .../pci/controller/dwc/pcie-designware-ep.c | 15 ++++++----- drivers/pci/controller/pcie-rcar-ep.c | 8 +++--- drivers/pci/controller/pcie-rockchip-ep.c | 9 ++++--- drivers/pci/endpoint/pci-epc-core.c | 26 +++++++------------ include/linux/pci-epc.h | 11 ++++---- 6 files changed, 39 insertions(+), 44 deletions(-)
Signature · 1 line
2.49.0
PATCH v3 1/6 PCI: dwc: ep: Fix broken set_msix() callback
While the set_msix() callback function in pcie-designware-ep writes the Table Size field correctly (N-1), the calculation of the PBA offset is wrong because it calculates space for (N-1) entries instead of N.
This results in e.g. the following error when using QEMU with PCI
passthrough on a device which relies on the PCI endpoint subsystem:
failed to add PCI capability 0x11[0x50]@0xb0: table & pba overlap, or they don't fit in BARs, or don't align
Fix the calculation of PBA offset in the MSI-X capability.
--- drivers/pci/controller/dwc/pcie-designware-ep.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/controller/dwc/pcie-designware-ep.c +3 -2
@@ -585,6 +585,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,struct dw_pcie *pci =to_dw_pcie_from_ep (ep);struct dw_pcie_ep_func *ep_func;u32 val, reg; +u16 actual_interrupts = interrupts +1 ; ep_func =dw_pcie_ep_get_func_from_ep (ep, func_no);if (!ep_func || !ep_func->msix_cap)
@@ -595,7 +596,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, reg = ep_func->msix_cap +PCI_MSIX_FLAGS ; val =dw_pcie_ep_readw_dbi (ep, func_no, reg); val &= ~PCI_MSIX_FLAGS_QSIZE ; - val |= interrupts; + val |= interrupts;/* 0's based value */ dw_pcie_writew_dbi (pci, reg, val); reg = ep_func->msix_cap +PCI_MSIX_TABLE ;
@@ -603,7 +604,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,dw_pcie_ep_writel_dbi (ep, func_no, reg, val); reg = ep_func->msix_cap +PCI_MSIX_PBA ; - val = (offset + (interrupts *PCI_MSIX_ENTRY_SIZE )) | bir; + val = (offset + (actual_interrupts *PCI_MSIX_ENTRY_SIZE )) | bir;dw_pcie_ep_writel_dbi (ep, func_no, reg, val);dw_pcie_dbi_ro_wr_dis (pci);
Signature · 1 line
2.49.0
PATCH v3 2/6 PCI: cadence-ep: Fix broken set_msix() callback
In reply to Niklas CasselWhile the set_msix() callback function in pcie-cadence-ep writes the Table Size field correctly (N-1), the calculation of the PBA offset is wrong because it calculates space for (N-1) entries instead of N.
This results in e.g. the following error when using QEMU with PCI
passthrough on a device which relies on the PCI endpoint subsystem:
failed to add PCI capability 0x11[0x50]@0xb0: table & pba overlap, or they don't fit in BARs, or don't align
Fix the calculation of PBA offset in the MSI-X capability.
--- drivers/pci/controller/cadence/pcie-cadence-ep.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/controller/cadence/pcie-cadence-ep.c +3 -2
@@ -292,13 +292,14 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,struct cdns_pcie *pcie = &ep->pcie;u32 cap =CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET ;u32 val, reg; +u16 actual_interrupts = interrupts +1 ; fn =cdns_pcie_get_fn_from_vfn (pcie, fn, vfn); reg = cap +PCI_MSIX_FLAGS ; val =cdns_pcie_ep_fn_readw (pcie, fn, reg); val &= ~PCI_MSIX_FLAGS_QSIZE ; - val |= interrupts; + val |= interrupts;/* 0's based value */ cdns_pcie_ep_fn_writew (pcie, fn, reg, val);/* Set MSI-X BAR and offset */
@@ -308,7 +309,7 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,/* Set PBA BAR and offset. BAR must match MSI-X BAR */ reg = cap +PCI_MSIX_PBA ; - val = (offset + (interrupts *PCI_MSIX_ENTRY_SIZE )) | bir; + val = (offset + (actual_interrupts *PCI_MSIX_ENTRY_SIZE )) | bir;cdns_pcie_ep_fn_writel (pcie, fn, reg, val);return 0 ;
Signature · 1 line
2.49.0
PATCH v3 3/6 PCI: endpoint: Cleanup get_msi() callback
In reply to Niklas CasselThe kdoc for pci_epc_get_msi() says:
"Invoke to get the number of MSI interrupts allocated by the RC" the kdoc for the callback pci_epc_ops->get_msi() says:
"ops to get the number of MSI interrupts allocated by the RC from the MSI capability register"
pci_epc_ops->get_msi() does however return the number of interrupts in the encoding as defined by the MME Multiple Message Enable field.
Nowhere in the kdoc does it say that the returned number of interrupts is in MME encoding.
Thus, it is very confusing that the wrapper function (pci_epc_get_msi()) and the callback function (pci_epc_ops->get_msi()) don't return the same value.
Cleanup the API so that the wrapper function and the callback function will have the same semantics, i.e. return the number of interrupts, regardless of the internal encoding of that value.
--- drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 +- drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +- drivers/pci/controller/pcie-rcar-ep.c | 2 +- drivers/pci/controller/pcie-rockchip-ep.c | 4 ++-- drivers/pci/endpoint/pci-epc-core.c | 2 -- 5 files changed, 5 insertions(+), 7 deletions(-)
drivers/pci/controller/cadence/pcie-cadence-ep.c +1 -1
@@ -262,7 +262,7 @@ static int cdns_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)*/ mme =FIELD_GET (PCI_MSI_FLAGS_QSIZE , flags); -return mme; +return 1 << mme; }static int cdns_pcie_ep_get_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no)
drivers/pci/controller/dwc/pcie-designware-ep.c +1 -1
@@ -532,7 +532,7 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no) val =FIELD_GET (PCI_MSI_FLAGS_QSIZE , val); -return val; +return 1 << val; } static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
drivers/pci/controller/pcie-rcar-ep.c +1 -1
@@ -280,7 +280,7 @@ static int rcar_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)if (!(flags &MSICAP0_MSIE ))return -EINVAL ; -return ((flags &MSICAP0_MMESE_MASK ) >>MSICAP0_MMESE_OFFSET ); +return 1 << ((flags &MSICAP0_MMESE_MASK ) >>MSICAP0_MMESE_OFFSET ); } static int rcar_pcie_ep_map_addr(struct pci_epc *epc, u8 fn, u8 vfn,
drivers/pci/controller/pcie-rockchip-ep.c +2 -2
@@ -340,8 +340,8 @@ static int rockchip_pcie_ep_get_msi(struct pci_epc *epc, u8 fn, u8 vfn)if (!(flags &ROCKCHIP_PCIE_EP_MSI_CTRL_ME ))return -EINVAL ; -return ((flags &ROCKCHIP_PCIE_EP_MSI_CTRL_MME_MASK ) >> -ROCKCHIP_PCIE_EP_MSI_CTRL_MME_OFFSET ); +return 1 << ((flags &ROCKCHIP_PCIE_EP_MSI_CTRL_MME_MASK ) >> +ROCKCHIP_PCIE_EP_MSI_CTRL_MME_OFFSET ); } static void rockchip_pcie_ep_assert_intx(struct rockchip_pcie_ep *ep, u8 fn,
drivers/pci/endpoint/pci-epc-core.c +0 -2
@@ -293,8 +293,6 @@ int pci_epc_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)if (interrupt <0 )return 0 ; - interrupt =1 << interrupt; -return interrupt; }EXPORT_SYMBOL_GPL (pci_epc_get_msi);
Signature · 1 line
2.49.0
PATCH v3 4/6 PCI: endpoint: Cleanup get_msix() callback
In reply to Niklas CasselThe kdoc for pci_epc_get_msix() says:
"Invoke to get the number of MSI-X interrupts allocated by the RC" the kdoc for the callback pci_epc_ops->get_msix() says:
"ops to get the number of MSI-X interrupts allocated by the RC from the MSI-X capability register"
pci_epc_ops->get_msix() does however return the number of interrupts in the encoding as defined by the Table Size field.
Nowhere in the kdoc does it say that the returned number of interrupts is in Table Size encoding.
Thus, it is very confusing that the wrapper function (pci_epc_get_msix()) and the callback function (pci_epc_ops->get_msix()) don't return the same value.
Cleanup the API so that the wrapper function and the callback function will have the same semantics, i.e. return the number of interrupts, regardless of the internal encoding of that value.
--- drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 +- drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +- drivers/pci/endpoint/pci-epc-core.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/pci/controller/cadence/pcie-cadence-ep.c +1 -1
@@ -281,7 +281,7 @@ static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) val &=PCI_MSIX_FLAGS_QSIZE ; -return val; +return val +1 ; } static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,
drivers/pci/controller/dwc/pcie-designware-ep.c +1 -1
@@ -575,7 +575,7 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) val &=PCI_MSIX_FLAGS_QSIZE ; -return val; +return val +1 ; } static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
drivers/pci/endpoint/pci-epc-core.c +1 -1
@@ -355,7 +355,7 @@ int pci_epc_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)if (interrupt <0 )return 0 ; -return interrupt +1 ; +return interrupt; }EXPORT_SYMBOL_GPL (pci_epc_get_msix);
Signature · 1 line
2.49.0
PATCH v3 5/6 PCI: endpoint: Cleanup set_msi() callback
In reply to Niklas CasselThe kdoc for pci_epc_set_msi() says:
"Invoke to set the required number of MSI interrupts."
the kdoc for the callback pci_epc_ops->set_msi() says:
"ops to set the requested number of MSI interrupts in the MSI capability register"
pci_epc_ops->set_msi() does however expect the parameter 'interrupts' to be in the encoding as defined by the MMC Multiple Message Capable field.
Nowhere in the kdoc does it say that the number of interrupts should be in MMC encoding.
Thus, it is very confusing that the wrapper function (pci_epc_set_msi()) and the callback function (pci_epc_ops->set_msi()) both take a parameter named interrupts, but they both expect completely different encodings.
Cleanup the API so that the wrapper function and the callback function will have the same semantics, i.e. the parameter represents the number of interrupts, regardless of the internal encoding of that value.
Also rename the parameter 'interrupts' to 'nr_irqs', in both the wrapper function and the callback function, such that the name is unambiguous.
--- drivers/pci/controller/cadence/pcie-cadence-ep.c | 3 ++- drivers/pci/controller/dwc/pcie-designware-ep.c | 5 +++-- drivers/pci/controller/pcie-rcar-ep.c | 6 +++--- drivers/pci/controller/pcie-rockchip-ep.c | 5 +++-- drivers/pci/endpoint/pci-epc-core.c | 11 ++++------- include/linux/pci-epc.h | 5 ++--- 6 files changed, 17 insertions(+), 18 deletions(-)
drivers/pci/controller/cadence/pcie-cadence-ep.c +2 -1
@@ -220,10 +220,11 @@ static void cdns_pcie_ep_unmap_addr(struct pci_epc *epc, u8 fn, u8 vfn,clear_bit (r, &ep->ob_region_map); } -static int cdns_pcie_ep_set_msi (struct pci_epc *epc,u8 fn,u8 vfn,u8 mmc) +static int cdns_pcie_ep_set_msi (struct pci_epc *epc,u8 fn,u8 vfn,u8 nr_irqs) {struct cdns_pcie_ep *ep =epc_get_drvdata (epc);struct cdns_pcie *pcie = &ep->pcie; +u8 mmc =order_base_2 (nr_irqs);u32 cap =CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET ;u16 flags;
drivers/pci/controller/dwc/pcie-designware-ep.c +3 -2
@@ -536,11 +536,12 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no) }static int dw_pcie_ep_set_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u8 interrupts) +u8 nr_irqs) {struct dw_pcie_ep *ep =epc_get_drvdata (epc);struct dw_pcie *pci =to_dw_pcie_from_ep (ep);struct dw_pcie_ep_func *ep_func; +u8 mmc =order_base_2 (nr_irqs);u32 val, reg; ep_func =dw_pcie_ep_get_func_from_ep (ep, func_no);
@@ -550,7 +551,7 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no, reg = ep_func->msi_cap +PCI_MSI_FLAGS ; val =dw_pcie_ep_readw_dbi (ep, func_no, reg); val &= ~PCI_MSI_FLAGS_QMASK ; - val |=FIELD_PREP (PCI_MSI_FLAGS_QMASK , interrupts); + val |=FIELD_PREP (PCI_MSI_FLAGS_QMASK , mmc);dw_pcie_dbi_ro_wr_en (pci);dw_pcie_ep_writew_dbi (ep, func_no, reg, val);dw_pcie_dbi_ro_wr_dis (pci);
drivers/pci/controller/pcie-rcar-ep.c +3 -3
@@ -256,15 +256,15 @@ static void rcar_pcie_ep_clear_bar(struct pci_epc *epc, u8 fn, u8 vfn,clear_bit (atu_index +1 , ep->ib_window_map); } -static int rcar_pcie_ep_set_msi (struct pci_epc *epc,u8 fn,u8 vfn, -u8 interrupts) +static int rcar_pcie_ep_set_msi (struct pci_epc *epc,u8 fn,u8 vfn,u8 nr_irqs) {struct rcar_pcie_endpoint *ep =epc_get_drvdata (epc);struct rcar_pcie *pcie = &ep->pcie; +u8 mmc =order_base_2 (nr_irqs);u32 flags; flags =rcar_pci_read_reg (pcie,MSICAP (fn)); - flags |= interrupts <<MSICAP0_MMESCAP_OFFSET ; + flags |= mmc <<MSICAP0_MMESCAP_OFFSET ;rcar_pci_write_reg (pcie, flags,MSICAP (fn));return 0 ;
drivers/pci/controller/pcie-rockchip-ep.c +3 -2
@@ -308,10 +308,11 @@ static void rockchip_pcie_ep_unmap_addr(struct pci_epc *epc, u8 fn, u8 vfn, }static int rockchip_pcie_ep_set_msi (struct pci_epc *epc,u8 fn,u8 vfn, -u8 multi_msg_cap) +u8 nr_irqs) {struct rockchip_pcie_ep *ep =epc_get_drvdata (epc);struct rockchip_pcie *rockchip = &ep->rockchip; +u8 mmc =order_base_2 (nr_irqs);u32 flags; flags = rockchip_pcie_read(rockchip,
@@ -319,7 +320,7 @@ static int rockchip_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, ROCKCHIP_PCIE_EP_MSI_CTRL_REG); flags &= ~ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_MASK ; flags |= - (multi_msg_cap <<ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_OFFSET ) | + (mmc <<ROCKCHIP_PCIE_EP_MSI_CTRL_MMC_OFFSET ) | (PCI_MSI_FLAGS_64BIT <<ROCKCHIP_PCIE_EP_MSI_FLAGS_OFFSET ); flags &= ~ROCKCHIP_PCIE_EP_MSI_CTRL_MASK_MSI_CAP ; rockchip_pcie_write(rockchip, flags,
drivers/pci/endpoint/pci-epc-core.c +4 -7
@@ -302,28 +302,25 @@ EXPORT_SYMBOL_GPL(pci_epc_get_msi);* @epc: the EPC device on which MSI has to be configured * @func_no: the physical endpoint function number in the EPC device * @vfunc_no: the virtual endpoint function number in the physical function -* @interrupts: number of MSI interrupts required by the EPF +* @nr_irqs: number of MSI interrupts required by the EPF * * Invoke to set the required number of MSI interrupts. */ -int pci_epc_set_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u8 interrupts) +int pci_epc_set_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u8 nr_irqs) {int ret; -u8 encode_int;if (!pci_epc_function_is_valid (epc, func_no, vfunc_no))return -EINVAL ; -if (interrupts <1 || interrupts >32 ) +if (nr_irqs <1 || nr_irqs >32 )return -EINVAL ;if (!epc->ops->set_msi)return 0 ; - encode_int =order_base_2 (interrupts); -mutex_lock (&epc->lock); - ret = epc->ops->set_msi (epc, func_no, vfunc_no, encode_int); + ret = epc->ops->set_msi (epc, func_no, vfunc_no, nr_irqs);mutex_unlock (&epc->lock);return ret;
include/linux/pci-epc.h +2 -3
@@ -100,7 +100,7 @@ struct pci_epc_ops {void (*unmap_addr)(struct pci_epc *epc,u8 func_no,u8 vfunc_no,phys_addr_t addr);int (*set_msi)(struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u8 interrupts); +u8 nr_irqs);int (*get_msi)(struct pci_epc *epc,u8 func_no,u8 vfunc_no);int (*set_msix)(struct pci_epc *epc,u8 func_no,u8 vfunc_no,u16 interrupts,enum pci_barno ,u32 offset);
@@ -286,8 +286,7 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u64 pci_addr, size_t size);void pci_epc_unmap_addr (struct pci_epc *epc,u8 func_no,u8 vfunc_no,phys_addr_t phys_addr); -int pci_epc_set_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u8 interrupts); +int pci_epc_set_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u8 nr_irqs);int pci_epc_get_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no);int pci_epc_set_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u16 interrupts,enum pci_barno ,u32 offset);
Signature · 1 line
2.49.0
PATCH v3 6/6 PCI: endpoint: Cleanup set_msix() callback
In reply to Niklas CasselThe kdoc for pci_epc_set_msix() says:
"Invoke to set the required number of MSI-X interrupts." the kdoc for the callback pci_epc_ops->set_msix() says:
"ops to set the requested number of MSI-X interrupts in the MSI-X capability register"
pci_epc_ops->set_msix() does however expect the parameter 'interrupts' to be in the encoding as defined by the Table Size field.
Nowhere in the kdoc does it say that the number of interrupts should be in Table Size encoding.
Thus, it is very confusing that the wrapper function (pci_epc_set_msix()) and the callback function (pci_epc_ops->set_msix()) both take a parameter named interrupts, but they both expect completely different encodings.
Cleanup the API so that the wrapper function and the callback function will have the same semantics, i.e. the parameter represents the number of interrupts, regardless of the internal encoding of that value.
Also rename the parameter 'interrupts' to 'nr_irqs', in both the wrapper function and the callback function, such that the name is unambiguous.
--- drivers/pci/controller/cadence/pcie-cadence-ep.c | 8 +++----- drivers/pci/controller/dwc/pcie-designware-ep.c | 7 +++---- drivers/pci/endpoint/pci-epc-core.c | 11 +++++------ include/linux/pci-epc.h | 6 +++--- 4 files changed, 14 insertions(+), 18 deletions(-)
drivers/pci/controller/cadence/pcie-cadence-ep.c +3 -5
@@ -286,21 +286,19 @@ static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) }static int cdns_pcie_ep_set_msix (struct pci_epc *epc,u8 fn,u8 vfn, -u16 interrupts,enum pci_barno bir, -u32 offset) +u16 nr_irqs,enum pci_barno bir,u32 offset) {struct cdns_pcie_ep *ep =epc_get_drvdata (epc);struct cdns_pcie *pcie = &ep->pcie;u32 cap =CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET ;u32 val, reg; -u16 actual_interrupts = interrupts +1 ; fn =cdns_pcie_get_fn_from_vfn (pcie, fn, vfn); reg = cap +PCI_MSIX_FLAGS ; val =cdns_pcie_ep_fn_readw (pcie, fn, reg); val &= ~PCI_MSIX_FLAGS_QSIZE ; - val |= interrupts;/* 0's based value */ + val |= nr_irqs -1 ;/* encoded as N-1 */ cdns_pcie_ep_fn_writew (pcie, fn, reg, val);/* Set MSI-X BAR and offset */
@@ -310,7 +308,7 @@ static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u8 vfn,/* Set PBA BAR and offset. BAR must match MSI-X BAR */ reg = cap +PCI_MSIX_PBA ; - val = (offset + (actual_interrupts *PCI_MSIX_ENTRY_SIZE )) | bir; + val = (offset + (nr_irqs *PCI_MSIX_ENTRY_SIZE )) | bir;cdns_pcie_ep_fn_writel (pcie, fn, reg, val);return 0 ;
drivers/pci/controller/dwc/pcie-designware-ep.c +3 -4
@@ -580,13 +580,12 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no) }static int dw_pcie_ep_set_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u16 interrupts,enum pci_barno bir,u32 offset) +u16 nr_irqs,enum pci_barno bir,u32 offset) {struct dw_pcie_ep *ep =epc_get_drvdata (epc);struct dw_pcie *pci =to_dw_pcie_from_ep (ep);struct dw_pcie_ep_func *ep_func;u32 val, reg; -u16 actual_interrupts = interrupts +1 ; ep_func =dw_pcie_ep_get_func_from_ep (ep, func_no);if (!ep_func || !ep_func->msix_cap)
@@ -597,7 +596,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, reg = ep_func->msix_cap +PCI_MSIX_FLAGS ; val =dw_pcie_ep_readw_dbi (ep, func_no, reg); val &= ~PCI_MSIX_FLAGS_QSIZE ; - val |= interrupts;/* 0's based value */ + val |= nr_irqs -1 ;/* encoded as N-1 */ dw_pcie_writew_dbi (pci, reg, val); reg = ep_func->msix_cap +PCI_MSIX_TABLE ;
@@ -605,7 +604,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,dw_pcie_ep_writel_dbi (ep, func_no, reg, val); reg = ep_func->msix_cap +PCI_MSIX_PBA ; - val = (offset + (actual_interrupts *PCI_MSIX_ENTRY_SIZE )) | bir; + val = (offset + (nr_irqs *PCI_MSIX_ENTRY_SIZE )) | bir;dw_pcie_ep_writel_dbi (ep, func_no, reg, val);dw_pcie_dbi_ro_wr_dis (pci);
drivers/pci/endpoint/pci-epc-core.c +5 -6
@@ -361,29 +361,28 @@ EXPORT_SYMBOL_GPL(pci_epc_get_msix);* @epc: the EPC device on which MSI-X has to be configured * @func_no: the physical endpoint function number in the EPC device * @vfunc_no: the virtual endpoint function number in the physical function -* @interrupts: number of MSI-X interrupts required by the EPF +* @nr_irqs: number of MSI-X interrupts required by the EPF * @bir: BAR where the MSI-X table resides * @offset: Offset pointing to the start of MSI-X table * * Invoke to set the required number of MSI-X interrupts. */ -int pci_epc_set_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u16 interrupts,enum pci_barno bir,u32 offset) +int pci_epc_set_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u16 nr_irqs, +enum pci_barno bir,u32 offset) {int ret;if (!pci_epc_function_is_valid (epc, func_no, vfunc_no))return -EINVAL ; -if (interrupts <1 || interrupts >2048 ) +if (nr_irqs <1 || nr_irqs >2048 )return -EINVAL ;if (!epc->ops->set_msix)return 0 ;mutex_lock (&epc->lock); - ret = epc->ops->set_msix (epc, func_no, vfunc_no, interrupts -1 , bir, - offset); + ret = epc->ops->set_msix (epc, func_no, vfunc_no, nr_irqs, bir, offset);mutex_unlock (&epc->lock);return ret;
include/linux/pci-epc.h +3 -3
@@ -103,7 +103,7 @@ struct pci_epc_ops { u8 nr_irqs);int (*get_msi)(struct pci_epc *epc,u8 func_no,u8 vfunc_no);int (*set_msix)(struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u16 interrupts,enum pci_barno ,u32 offset); +u16 nr_irqs,enum pci_barno ,u32 offset);int (*get_msix)(struct pci_epc *epc,u8 func_no,u8 vfunc_no);int (*raise_irq)(struct pci_epc *epc,u8 func_no,u8 vfunc_no,unsigned int type,u16 interrupt_num);
@@ -288,8 +288,8 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no, phys_addr_t phys_addr);int pci_epc_set_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u8 nr_irqs);int pci_epc_get_msi (struct pci_epc *epc,u8 func_no,u8 vfunc_no); -int pci_epc_set_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no, -u16 interrupts,enum pci_barno ,u32 offset); +int pci_epc_set_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no,u16 nr_irqs, +enum pci_barno ,u32 offset);int pci_epc_get_msix (struct pci_epc *epc,u8 func_no,u8 vfunc_no); int pci_epc_map_msi_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no, phys_addr_t phys_addr, u8 interrupt_num,
Signature · 1 line
2.49.0
Manivannan Sadhasivam
AppliedQuoted 8 lines · Niklas Cassel
The first two patches in this series are IRQ callback fixes that should get backported.
The reason why the bugs existed in the first place is because the APIs are very confusing. The rest of the patches are cleanups of the APIs. These cleanups should not get backported.
[...]
Applied, thanks!
[1/6] PCI: dwc: ep: Fix broken set_msix() callback
commit: ef2a2813f4c738a5c8f71d47537a8e79b1058319
[2/6] PCI: cadence-ep: Fix broken set_msix() callback
commit: 24e50b43ebb98f147984054730cf30aebae23c51
[3/6] PCI: endpoint: Cleanup get_msi() callback
commit: 6f91c4cae6a3d895265e533630c00e1c4a0b8dee
[4/6] PCI: endpoint: Cleanup get_msix() callback
commit: 262df0e1a10fa8470103d343fe85afbba4b25698
[5/6] PCI: endpoint: Cleanup set_msi() callback
commit: 2b9391dcb26739d0b43927b329d2b670418c69a3
[6/6] PCI: endpoint: Cleanup set_msix() callback
commit: 210de38727c862164e933d978ba39b66569ab552Best regards,
Signature · 1 line
Manivannan Sadhasivam <[email protected]>