Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 7 May 2020 16:42:34 -0500
From:   Rob Herring <robh@kernel.org>
To:     Alan Mikhak <alan.mikhak@sifive.com>
Cc:     linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        kishon@ti.com, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
        ulf.hansson@linaro.org, sebott@linux.ibm.com, efremov@linux.com,
        vidyas@nvidia.com, paul.walmsley@sifive.com
Subject: Re: [PATCH] PCI: endpoint: functions/pci-epf-test: Enable picking
 DMA channel by name
Message-ID: <20200507214234.GA5449@bogus>
References: <1588350008-8143-1-git-send-email-alan.mikhak@sifive.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1588350008-8143-1-git-send-email-alan.mikhak@sifive.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, May 01, 2020 at 09:20:08AM -0700, Alan Mikhak wrote:
> From: Alan Mikhak <alan.mikhak@sifive.com>
> 
> Modify pci_epf_test_init_dma_chan() to call dma_request_channel() with a
> filter function to pick DMA channel by name, if desired.
> 
> Add a new filter function pci_epf_test_pick_dma_chan() which takes a name
> string as an optional parameter. If desired name is specified, the filter
> function checks the name of each DMA channel candidate against the desired
> name. If no match, the filter function rejects the candidate channel.
> Otherwise, the candidate channel is accepted. If optional name parameter
> is null or an empty string, filter function picks the first DMA channel
> candidate, thereby preserving the existing behavior of pci-epf-test.
> 
> Currently, pci-epf-test picks the first suitable DMA channel. Adding a
> filter function enables a developer to modify the optional parameter
> during debugging by providing the name of a desired DMA channel. This is
> useful during debugging because it allows different DMA channels to be
> exercised.
> 
> Adding a filter function also takes one step toward modifying pcitest to
> allow the user to choose a DMA channel by providing a name string at the
> command line when issuing the -d parameter for DMA transfers.

This mostly looks fine, but needs to be part of a series giving it a 
user.

> Signed-off-by: Alan Mikhak <alan.mikhak@sifive.com>
> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 60330f3e3751..043916d3ab5f 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -149,10 +149,26 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
>  }
>  
>  /**
> - * pci_epf_test_init_dma_chan() - Function to initialize EPF test DMA channel
> - * @epf_test: the EPF test device that performs data transfer operation
> + * pci_epf_test_pick_dma_chan() - Filter DMA channel based on desired criteria
> + * @chan: the DMA channel to examine
>   *
> - * Function to initialize EPF test DMA channel.
> + * Filter DMA channel candidates by matching against an optional desired name.
> + * Pick first candidate channel if desired name is not specified.
> + * Reject candidate channel if its name does not match the desired name.
> + */
> +static bool pci_epf_test_pick_dma_chan(struct dma_chan *chan, void *name)
> +{
> +	if (name && strlen(name) && strcmp(dma_chan_name(chan), name))

Doesn't this cause warning with 'name' being void*?


> +		return false;
> +
> +	return true;
> +}
> +
> +/**
> + * pci_epf_test_init_dma_chan() - Helper to initialize EPF DMA channel
> + * @epf: the EPF device that has to perform the data transfer operation
> + *
> + * Helper to initialize EPF DMA channel.
>   */
>  static int pci_epf_test_init_dma_chan(struct pci_epf_test *epf_test)
>  {
> @@ -165,7 +181,7 @@ static int pci_epf_test_init_dma_chan(struct pci_epf_test *epf_test)
>  	dma_cap_zero(mask);
>  	dma_cap_set(DMA_MEMCPY, mask);
>  
> -	dma_chan = dma_request_chan_by_mask(&mask);
> +	dma_chan = dma_request_channel(mask, pci_epf_test_pick_dma_chan, NULL);
>  	if (IS_ERR(dma_chan)) {
>  		ret = PTR_ERR(dma_chan);
>  		if (ret != -EPROBE_DEFER)
> -- 
> 2.7.4
>