Re: [PATCH v4 04/10] PCI: dwc: Rework MSI callbacks handler

From: Gustavo Pimentel
Date: Wed Jun 20 2018 - 06:23:02 EST


Hi,

On 20/06/2018 07:49, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Monday 18 June 2018 08:30 PM, Gustavo Pimentel wrote:
>> Remove duplicate defines located on pcie-designware.h file already
>> available on /include/uapi/linux/pci-regs.h file.
>>
>> Add pci_epc_set_msi() maximum 32 interrupts validation.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
>> ---
>> Change v1->v2:
>> - Nothing changed, just to follow the patch set version.
>> Change v2->v3:
>> - Replaced wrong return value 0 to -EINVAL.
>> Change v3->v4:
>> - Rebased to Lorenzo's master branch v4.18-rc1.
>>
>> drivers/pci/controller/dwc/pcie-designware-ep.c | 49 +++++++++++++++++--------
>> drivers/pci/controller/dwc/pcie-designware.h | 11 ------
>> drivers/pci/endpoint/pci-epc-core.c | 3 +-
>> 3 files changed, 35 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> index ad25654..89d9e52 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> @@ -246,29 +246,38 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no,
>>
>> static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no)
>> {
>> - int val;
>> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + u32 val, reg;
>> +
>> + if (!ep->msi_cap)
>> + return -EINVAL;
>>
>> - val = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
>> - if (!(val & MSI_CAP_MSI_EN_MASK))
>> + reg = ep->msi_cap + PCI_MSI_FLAGS;
>> + val = dw_pcie_readw_dbi(pci, reg);
>> + if (!(val & PCI_MSI_FLAGS_ENABLE))
>> return -EINVAL;
>>
>> - val = (val & MSI_CAP_MME_MASK) >> MSI_CAP_MME_SHIFT;
>> + val = (val & PCI_MSI_FLAGS_QSIZE) >> 4;
>> +
>> return val;
>> }
>>
>> -static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 encode_int)
>> +static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>> {
>> - int val;
>> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + u32 val, reg;
>>
>> - val = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
>> - val &= ~MSI_CAP_MMC_MASK;
>> - val |= (encode_int << MSI_CAP_MMC_SHIFT) & MSI_CAP_MMC_MASK;
>> + if (!ep->msi_cap)
>> + return -EINVAL;
>> +
>> + reg = ep->msi_cap + PCI_MSI_FLAGS;
>> + val = dw_pcie_readw_dbi(pci, reg);
>> + val &= ~PCI_MSI_FLAGS_QMASK;
>> + val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK;
>> dw_pcie_dbi_ro_wr_en(pci);
>> - dw_pcie_writew_dbi(pci, MSI_MESSAGE_CONTROL, val);
>> + dw_pcie_writew_dbi(pci, reg, val);
>> dw_pcie_dbi_ro_wr_dis(pci);
>>
>> return 0;
>> @@ -367,21 +376,29 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> struct pci_epc *epc = ep->epc;
>> u16 msg_ctrl, msg_data;
>> - u32 msg_addr_lower, msg_addr_upper;
>> + u32 msg_addr_lower, msg_addr_upper, reg;
>> u64 msg_addr;
>> bool has_upper;
>> int ret;
>>
>> + if (!ep->msi_cap)
>> + return -EINVAL;
>> +
>> /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
>> - msg_ctrl = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
>> + reg = ep->msi_cap + PCI_MSI_FLAGS;
>> + msg_ctrl = dw_pcie_readw_dbi(pci, reg);
>> has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
>> - msg_addr_lower = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_L32);
>> + reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
>> + msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
>> if (has_upper) {
>> - msg_addr_upper = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_U32);
>> - msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_64);
>> + reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
>> + msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
>> + reg = ep->msi_cap + PCI_MSI_DATA_64;
>> + msg_data = dw_pcie_readw_dbi(pci, reg);
>> } else {
>> msg_addr_upper = 0;
>> - msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_32);
>> + reg = ep->msi_cap + PCI_MSI_DATA_32;
>> + msg_data = dw_pcie_readw_dbi(pci, reg);
>> }
>> msg_addr = ((u64) msg_addr_upper) << 32 | msg_addr_lower;
>> ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index b22c5bb..a0ab12f 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -96,17 +96,6 @@
>> #define PCIE_GET_ATU_INB_UNR_REG_OFFSET(region) \
>> ((0x3 << 20) | ((region) << 9) | (0x1 << 8))
>>
>> -#define MSI_MESSAGE_CONTROL 0x52
>> -#define MSI_CAP_MMC_SHIFT 1
>> -#define MSI_CAP_MMC_MASK (7 << MSI_CAP_MMC_SHIFT)
>> -#define MSI_CAP_MME_SHIFT 4
>> -#define MSI_CAP_MSI_EN_MASK 0x1
>> -#define MSI_CAP_MME_MASK (7 << MSI_CAP_MME_SHIFT)
>> -#define MSI_MESSAGE_ADDR_L32 0x54
>> -#define MSI_MESSAGE_ADDR_U32 0x58
>> -#define MSI_MESSAGE_DATA_32 0x58
>> -#define MSI_MESSAGE_DATA_64 0x5C
>> -
>> #define MAX_MSI_IRQS 256
>> #define MAX_MSI_IRQS_PER_CTRL 32
>> #define MAX_MSI_CTRLS (MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL)
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index c72e656..094dcc3 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -201,7 +201,8 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>> u8 encode_int;
>> unsigned long flags;
>>
>> - if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
>> + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
>> + interrupts > 32)
>> return -EINVAL;
>
> This is not related to $patch->subject

A new patch file will be created for this.
Thanks,
Gustavo

>
> Thanks
> Kishon
>