Re: [PATCH] PCI: endpoint: functions/pci-epf-test: Enable picking DMA channel by name

[Rob Herring <robh@kernel.org>]

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
>