Re: [PATCH v2 5/5] PCI: cadence: Add MSI-X capability to EP driver

[Kishon Vijay Abraham I <kishon@xxxxxx>]

Hi,

On Wednesday 15 August 2018 07:19 PM, Alan Douglas wrote:
> Add set_msix and get_msix functions to driver, and handle
> PCI_EPC_IRQ_MSIX request in raise_irq.  BAR5 is used for
> the MSI-X vectors.
>
> Signed-off-by: Alan Douglas <adouglas@xxxxxxxxxxx>
> ---
>   drivers/pci/controller/pcie-cadence-ep.c | 107 ++++++++++++++++++++++++++++++-
>   drivers/pci/controller/pcie-cadence.h    |   1 +
>   2 files changed, 107 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
> index 1248d75..259b9a6 100644
> --- a/drivers/pci/controller/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/pcie-cadence-ep.c
> @@ -16,6 +16,7 @@
>   #define CDNS_PCIE_EP_MIN_APERTURE		128	/* 128 bytes */
>   #define CDNS_PCIE_EP_IRQ_PCI_ADDR_NONE		0x1
>   #define CDNS_PCIE_EP_IRQ_PCI_ADDR_LEGACY	0x3
> +#define CDNS_PCIE_EP_MSIX_BAR			0x5
>   
>   /**
>    * struct cdns_pcie_ep - private data for this PCIe endpoint controller driver
> @@ -255,6 +256,43 @@ static int cdns_pcie_ep_get_msi(struct pci_epc *epc, u8 fn)
>   	return mme;
>   }
>   
> +static int cdns_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
> +{
> +	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;
> +
> +	reg = cap + PCI_MSIX_FLAGS;
> +	val = cdns_pcie_ep_fn_readw(pcie, func_no, reg);
> +	if (!(val & PCI_MSIX_FLAGS_ENABLE))
> +		return -EINVAL;
> +
> +	val &= PCI_MSIX_FLAGS_QSIZE;
> +
> +	return val;
> +}
> +
> +static int cdns_pcie_ep_set_msix(struct pci_epc *epc, u8 fn, u16 interrupts)
> +{
> +	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;
> +
> +	reg = cap + PCI_MSIX_FLAGS;
> +	val = cdns_pcie_ep_fn_readw(pcie, fn, reg);
> +	val &= ~PCI_MSIX_FLAGS_QSIZE;
> +	val |= interrupts;
> +	cdns_pcie_ep_fn_writew(pcie, fn, reg, val);
> +	/* Set MSIX BAR and offset */
> +	cdns_pcie_ep_fn_writel(pcie, fn, 0xb4, CDNS_PCIE_EP_MSIX_BAR);

Please add a macro for MSIX table offset.
I think it relies on endpoint function driver to invoke set_bar for 
BAR5? It's possible a function driver can invoke set_msix without set_bar.

> +	/* Set PBA BAR and offset.  BAR must match MSIX BAR */
> +	cdns_pcie_ep_fn_writel(pcie, fn, 0xb8, 0x10000 | CDNS_PCIE_EP_MSIX_BAR);

Here too add a macro for PBA.
How did you get the 0x10000?

Thanks
Kishon