Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1385174

Re: [PATCH v2] dmaengine: tegra-apb: proper default init of channel slave_id

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Fri, Apr 22, 2016 at 06:14:53PM +0530, Shardar Shariff Md wrote:
> Initialize default channel slave_id(req_sel) to invalid id
> (i.e max supported slave id + 1) to avoid overwriting of slave_id
> during tegra_dma_slave_config() with client data if slave_id
> is not initialized through DT
> 
> Signed-off-by: Shardar Shariff Md <smohammed@nvidia.com>
> 
> ---
> - Instead of initializing the slave id to -1 define macros for
>   max slave id and invalid slave id and do the checks accordingly.
> ---
>  drivers/dma/tegra20-apb-dma.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 3871f29..2957e26 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -114,6 +114,9 @@
>  /* Channel base address offset from APBDMA base address */
>  #define TEGRA_APBDMA_CHANNEL_BASE_ADD_OFFSET	0x1000
>  
> +#define TEGRA_APBDMA_SLAVE_ID_MAX		31
> +#define TEGRA_APBDMA_SLAVE_ID_INVALID		(TEGRA_APBDMA_SLAVE_ID_MAX + 1)
> +
>  struct tegra_dma;
>  
>  /*
> @@ -353,7 +356,7 @@ static int tegra_dma_slave_config(struct dma_chan *dc,
>  	}
>  
>  	memcpy(&tdc->dma_sconfig, sconfig, sizeof(*sconfig));
> -	if (!tdc->slave_id)
> +	if (tdc->slave_id == TEGRA_APBDMA_SLAVE_ID_INVALID)
>  		tdc->slave_id = sconfig->slave_id;
>  	tdc->config_init = true;
>  	return 0;
> @@ -1236,7 +1239,7 @@ static void tegra_dma_free_chan_resources(struct dma_chan *dc)
>  	}
>  	pm_runtime_put(tdma->dev);
>  
> -	tdc->slave_id = 0;
> +	tdc->slave_id = TEGRA_APBDMA_SLAVE_ID_INVALID;
>  }
>  
>  static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec,
> @@ -1253,6 +1256,11 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec,
>  	tdc = to_tegra_dma_chan(chan);
>  	tdc->slave_id = dma_spec->args[0];
>  
> +	if (tdc->slave_id > TEGRA_APBDMA_SLAVE_ID_MAX) {
> +		dev_err(tdc2dev(tdc), "Invalid slave id\n");
> +		return NULL;
> +	}

Should this check happen before the channel is requested? As it is,
you're setting up the private data of the channel with a slave_id and
error out after that. As I understand the channel would already have
its ID set to the invalid ID, but without going too deep into the DMA
engine core code it looks as though once requested, the channel needs
to be released again (which the invalid ID handling code here doesn't).

Thierry

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2] dmaengine: tegra-apb: proper default init of channel slave_id Shardar Shariff Md <smohammed@nvidia.com> - 2016-04-22 14:50 +0200
  Re: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Jon Hunter <jonathanh@nvidia.com> - 2016-04-22 15:40 +0200
    RE: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Shardar Mohammed <smohammed@nvidia.com> - 2016-04-22 16:00 +0200
      Re: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Jon Hunter <jonathanh@nvidia.com> - 2016-04-22 16:50 +0200
  Re: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Thierry Reding <thierry.reding@gmail.com> - 2016-04-22 15:40 +0200
    RE: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Shardar Mohammed <smohammed@nvidia.com> - 2016-04-22 16:00 +0200
  Re: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Jon Hunter <jonathanh@nvidia.com> - 2016-04-22 15:50 +0200
    RE: [PATCH v2] dmaengine: tegra-apb: proper default init of channel  slave_id Shardar Mohammed <smohammed@nvidia.com> - 2016-04-22 16:00 +0200

csiph-web