Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RFC 03/10] PCI: endpoint: Add MSI-X interfaces
To:     Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
        <bhelgaas@google.com>, <lorenzo.pieralisi@arm.com>,
        <Joao.Pinto@synopsys.com>, <jingoohan1@gmail.com>,
        <adouglas@cadence.com>, <niklas.cassel@axis.com>,
        <jesper.nilsson@axis.com>
References: <cover.1523379766.git.gustavo.pimentel@synopsys.com>
 <c76688ee290a9ed4f0f70f5e350b906950ec05df.1523379766.git.gustavo.pimentel@synopsys.com>
CC:     <linux-pci@vger.kernel.org>, <linux-doc@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
From:   Kishon Vijay Abraham I <kishon@ti.com>
Message-ID: <7fec2005-d7a8-daea-9451-7f3593afe285@ti.com>
Date:   Tue, 17 Apr 2018 15:54:06 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <c76688ee290a9ed4f0f70f5e350b906950ec05df.1523379766.git.gustavo.pimentel@synopsys.com>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
> Implements the generic method for calling the get/set callbacks.
> 
> Adds the PCI_EPC_IRQ_MSIX type.
> 
> Adds the MSI-X callbacks signatures to the ops structure.
> 
> Adds sysfs interface for altering the number of MSI-X entries.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  drivers/pci/endpoint/pci-ep-cfs.c   | 24 ++++++++++++++++
>  drivers/pci/endpoint/pci-epc-core.c | 57 +++++++++++++++++++++++++++++++++++++
>  include/linux/pci-epc.h             | 11 ++++++-
>  include/linux/pci-epf.h             |  1 +
>  4 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index 018ea34..d1288a0 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -286,6 +286,28 @@ static ssize_t pci_epf_msi_interrupts_show(struct config_item *item,
>  		       to_pci_epf_group(item)->epf->msi_interrupts);
>  }
>  
> +static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
> +					     const char *page, size_t len)
> +{
> +	u16 val;
> +	int ret;
> +
> +	ret = kstrtou16(page, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	to_pci_epf_group(item)->epf->msix_interrupts = val;
> +
> +	return len;
> +}
> +
> +static ssize_t pci_epf_msix_interrupts_show(struct config_item *item,
> +					    char *page)
> +{
> +	return sprintf(page, "%d\n",
> +		       to_pci_epf_group(item)->epf->msix_interrupts);
> +}
> +
>  PCI_EPF_HEADER_R(vendorid)
>  PCI_EPF_HEADER_W_u16(vendorid)
>  
> @@ -327,6 +349,7 @@ CONFIGFS_ATTR(pci_epf_, subsys_vendor_id);
>  CONFIGFS_ATTR(pci_epf_, subsys_id);
>  CONFIGFS_ATTR(pci_epf_, interrupt_pin);
>  CONFIGFS_ATTR(pci_epf_, msi_interrupts);
> +CONFIGFS_ATTR(pci_epf_, msix_interrupts);
>  
>  static struct configfs_attribute *pci_epf_attrs[] = {
>  	&pci_epf_attr_vendorid,
> @@ -340,6 +363,7 @@ static struct configfs_attribute *pci_epf_attrs[] = {
>  	&pci_epf_attr_subsys_id,
>  	&pci_epf_attr_interrupt_pin,
>  	&pci_epf_attr_msi_interrupts,
> +	&pci_epf_attr_msix_interrupts,
>  	NULL,
>  };
>  
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index b0ee427..294a383 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -218,6 +218,63 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>  EXPORT_SYMBOL_GPL(pci_epc_set_msi);
>  
>  /**
> + * pci_epc_get_msix() - get the number of MSI-X interrupt numbers allocated
> + * @epc: the EPC device to which MSI-X interrupts was requested
> + * @func_no: the endpoint function number in the EPC device
> + *
> + * Invoke to get the number of MSI-X interrupts allocated by the RC
> + */
> +int pci_epc_get_msix(struct pci_epc *epc, u8 func_no)
> +{
> +	int interrupt;
> +	unsigned long flags;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> +		return 0;
> +
> +	if (!epc->ops->get_msix)
> +		return 0;
> +
> +	spin_lock_irqsave(&epc->lock, flags);
> +	interrupt = epc->ops->get_msix(epc, func_no);
> +	spin_unlock_irqrestore(&epc->lock, flags);
> +
> +	if (interrupt < 0)
> +		return 0;
> +
> +	return interrupt++;

return interrupt + 1?
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_get_msix);
> +
> +/**
> + * pci_epc_set_msix() - set the number of MSI-X interrupt numbers required
> + * @epc: the EPC device on which MSI-X has to be configured
> + * @func_no: the endpoint function number in the EPC device
> + * @interrupts: number of MSI-X interrupts required by the EPF
> + *
> + * Invoke to set the required number of MSI-X interrupts.
> + */
> +int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
> +{
> +	int ret;
> +	unsigned long flags;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
> +	    interrupts < 1 || interrupts > 2048)
> +		return -EINVAL;
> +
> +	if (!epc->ops->set_msix)
> +		return 0;
> +
> +	spin_lock_irqsave(&epc->lock, flags);
> +	ret = epc->ops->set_msix(epc, func_no, interrupts - 1);
> +	spin_unlock_irqrestore(&epc->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_set_msix);
> +
> +/**
>   * pci_epc_unmap_addr() - unmap CPU address from PCI address
>   * @epc: the EPC device on which address is allocated
>   * @func_no: the endpoint function number in the EPC device
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index af657ca..32e8961 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -17,6 +17,7 @@ enum pci_epc_irq_type {
>  	PCI_EPC_IRQ_UNKNOWN,
>  	PCI_EPC_IRQ_LEGACY,
>  	PCI_EPC_IRQ_MSI,
> +	PCI_EPC_IRQ_MSIX,
>  };
>  
>  /**
> @@ -30,6 +31,10 @@ enum pci_epc_irq_type {
>   *	     capability register
>   * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
>   *	     the MSI capability register
> + * @set_msix: ops to set the requested number of MSI-X interrupts in the
> + *	     MSI-X capability register
> + * @get_msix: ops to get the number of MSI-X interrupts allocated by the RC
> + *	     from the MSI-X capability register
>   * @raise_irq: ops to raise a legacy or MSI interrupt
>   * @start: ops to start the PCI link
>   * @stop: ops to stop the PCI link
> @@ -48,8 +53,10 @@ struct pci_epc_ops {
>  			      phys_addr_t addr);
>  	int	(*set_msi)(struct pci_epc *epc, u8 func_no, u8 interrupts);
>  	int	(*get_msi)(struct pci_epc *epc, u8 func_no);
> +	int	(*set_msix)(struct pci_epc *epc, u8 func_no, u16 interrupts);
> +	int	(*get_msix)(struct pci_epc *epc, u8 func_no);

The 1st patch in the series is already using get_msix, set_msix. This patch
should precede the 1st patch.

Thanks
Kishon