dmaengine Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
@ 2019-07-12 11:10 Sameer Pujar
  2019-07-23  6:11 ` Sameer Pujar
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sameer Pujar @ 2019-07-12 11:10 UTC (permalink / raw)
  To: vkoul, jonathanh, ldewangan
  Cc: thierry.reding, dmaengine, linux-tegra, Sameer Pujar

From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
configuration register(bits 7:4) which defines the maximum number of reads
from the source and writes to the destination that may be outstanding at
any given point of time. This field must be programmed with a value
between 1 and 8. A value of 0 will prevent any transfers from happening.

Thus added 'ch_pending_req' member in chip data structure and the same is
populated with maximum allowed pending requests. Since the field is not
applicable to Tegra210, mentioned bit fields are unused and hence the
member is initialized with 0. For Tegra186, by default program this field
with the maximum permitted value of 8.

Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for Tegra186/Tegra194")

Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 drivers/dma/tegra210-adma.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 2805853..5ab4e3a9 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -74,6 +74,8 @@
 				    TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
 				    TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
 
+#define TEGRA186_DMA_MAX_PENDING_REQS			8
+
 #define ADMA_CH_REG_FIELD_VAL(val, mask, shift)	(((val) & mask) << shift)
 
 struct tegra_adma;
@@ -85,6 +87,7 @@ struct tegra_adma;
  * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
  * @ch_req_rx_shift: Register offset for AHUB receive channel select.
  * @ch_base_offset: Register offset of DMA channel registers.
+ * @ch_pending_req: Outstaning DMA requests for a channel.
  * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
  * @ch_req_mask: Mask for Tx or Rx channel select.
  * @ch_req_max: Maximum number of Tx or Rx channels available.
@@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
 	unsigned int ch_req_tx_shift;
 	unsigned int ch_req_rx_shift;
 	unsigned int ch_base_offset;
+	unsigned int ch_pending_req;
 	unsigned int ch_fifo_ctrl;
 	unsigned int ch_req_mask;
 	unsigned int ch_req_max;
@@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
 			 ADMA_CH_CTRL_FLOWCTRL_EN;
 	ch_regs->config |= cdata->adma_get_burst_config(burst_size);
 	ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
+	ch_regs->config |= cdata->ch_pending_req;
 	ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
 	ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
 
@@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
 	.ch_req_tx_shift	= 28,
 	.ch_req_rx_shift	= 24,
 	.ch_base_offset		= 0,
+	.ch_pending_req		= 0,
 	.ch_fifo_ctrl		= TEGRA210_FIFO_CTRL_DEFAULT,
 	.ch_req_mask		= 0xf,
 	.ch_req_max		= 10,
@@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
 	.ch_req_tx_shift	= 27,
 	.ch_req_rx_shift	= 22,
 	.ch_base_offset		= 0x10000,
+	.ch_pending_req		= (TEGRA186_DMA_MAX_PENDING_REQS << 4),
 	.ch_fifo_ctrl		= TEGRA186_FIFO_CTRL_DEFAULT,
 	.ch_req_mask		= 0x1f,
 	.ch_req_max		= 20,
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
  2019-07-12 11:10 [PATCH v2] dmaengine: tegra210-adma: fix transfer failure Sameer Pujar
@ 2019-07-23  6:11 ` Sameer Pujar
  2019-08-06 14:45   ` Sameer Pujar
  2019-08-08 14:08 ` Vinod Koul
  2019-08-19 17:40 ` Jon Hunter
  2 siblings, 1 reply; 7+ messages in thread
From: Sameer Pujar @ 2019-07-23  6:11 UTC (permalink / raw)
  To: vkoul, jonathanh, ldewangan; +Cc: thierry.reding, dmaengine, linux-tegra

Hi Reviewers,

Please review.

Thanks,
Sameer.

On 7/12/2019 4:40 PM, Sameer Pujar wrote:
>  From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
> configuration register(bits 7:4) which defines the maximum number of reads
> from the source and writes to the destination that may be outstanding at
> any given point of time. This field must be programmed with a value
> between 1 and 8. A value of 0 will prevent any transfers from happening.
>
> Thus added 'ch_pending_req' member in chip data structure and the same is
> populated with maximum allowed pending requests. Since the field is not
> applicable to Tegra210, mentioned bit fields are unused and hence the
> member is initialized with 0. For Tegra186, by default program this field
> with the maximum permitted value of 8.
>
> Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for Tegra186/Tegra194")
>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>   drivers/dma/tegra210-adma.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 2805853..5ab4e3a9 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -74,6 +74,8 @@
>   				    TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
>   				    TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
>   
> +#define TEGRA186_DMA_MAX_PENDING_REQS			8
> +
>   #define ADMA_CH_REG_FIELD_VAL(val, mask, shift)	(((val) & mask) << shift)
>   
>   struct tegra_adma;
> @@ -85,6 +87,7 @@ struct tegra_adma;
>    * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
>    * @ch_req_rx_shift: Register offset for AHUB receive channel select.
>    * @ch_base_offset: Register offset of DMA channel registers.
> + * @ch_pending_req: Outstaning DMA requests for a channel.
>    * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
>    * @ch_req_mask: Mask for Tx or Rx channel select.
>    * @ch_req_max: Maximum number of Tx or Rx channels available.
> @@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
>   	unsigned int ch_req_tx_shift;
>   	unsigned int ch_req_rx_shift;
>   	unsigned int ch_base_offset;
> +	unsigned int ch_pending_req;
>   	unsigned int ch_fifo_ctrl;
>   	unsigned int ch_req_mask;
>   	unsigned int ch_req_max;
> @@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
>   			 ADMA_CH_CTRL_FLOWCTRL_EN;
>   	ch_regs->config |= cdata->adma_get_burst_config(burst_size);
>   	ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
> +	ch_regs->config |= cdata->ch_pending_req;
>   	ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
>   	ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
>   
> @@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
>   	.ch_req_tx_shift	= 28,
>   	.ch_req_rx_shift	= 24,
>   	.ch_base_offset		= 0,
> +	.ch_pending_req		= 0,
>   	.ch_fifo_ctrl		= TEGRA210_FIFO_CTRL_DEFAULT,
>   	.ch_req_mask		= 0xf,
>   	.ch_req_max		= 10,
> @@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
>   	.ch_req_tx_shift	= 27,
>   	.ch_req_rx_shift	= 22,
>   	.ch_base_offset		= 0x10000,
> +	.ch_pending_req		= (TEGRA186_DMA_MAX_PENDING_REQS << 4),
>   	.ch_fifo_ctrl		= TEGRA186_FIFO_CTRL_DEFAULT,
>   	.ch_req_mask		= 0x1f,
>   	.ch_req_max		= 20,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
  2019-07-23  6:11 ` Sameer Pujar
@ 2019-08-06 14:45   ` Sameer Pujar
  0 siblings, 0 replies; 7+ messages in thread
From: Sameer Pujar @ 2019-08-06 14:45 UTC (permalink / raw)
  To: vkoul, jonathanh, ldewangan; +Cc: thierry.reding, dmaengine, linux-tegra

Request for review.

On 7/23/2019 11:41 AM, Sameer Pujar wrote:
> Hi Reviewers,
>
> Please review.
>
> Thanks,
> Sameer.
>
> On 7/12/2019 4:40 PM, Sameer Pujar wrote:
>>  From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
>> configuration register(bits 7:4) which defines the maximum number of 
>> reads
>> from the source and writes to the destination that may be outstanding at
>> any given point of time. This field must be programmed with a value
>> between 1 and 8. A value of 0 will prevent any transfers from happening.
>>
>> Thus added 'ch_pending_req' member in chip data structure and the 
>> same is
>> populated with maximum allowed pending requests. Since the field is not
>> applicable to Tegra210, mentioned bit fields are unused and hence the
>> member is initialized with 0. For Tegra186, by default program this 
>> field
>> with the maximum permitted value of 8.
>>
>> Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for 
>> Tegra186/Tegra194")
>>
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> ---
>>   drivers/dma/tegra210-adma.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index 2805853..5ab4e3a9 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -74,6 +74,8 @@
>> TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
>> TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
>>   +#define TEGRA186_DMA_MAX_PENDING_REQS            8
>> +
>>   #define ADMA_CH_REG_FIELD_VAL(val, mask, shift) (((val) & mask) << 
>> shift)
>>     struct tegra_adma;
>> @@ -85,6 +87,7 @@ struct tegra_adma;
>>    * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
>>    * @ch_req_rx_shift: Register offset for AHUB receive channel select.
>>    * @ch_base_offset: Register offset of DMA channel registers.
>> + * @ch_pending_req: Outstaning DMA requests for a channel.
>>    * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
>>    * @ch_req_mask: Mask for Tx or Rx channel select.
>>    * @ch_req_max: Maximum number of Tx or Rx channels available.
>> @@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
>>       unsigned int ch_req_tx_shift;
>>       unsigned int ch_req_rx_shift;
>>       unsigned int ch_base_offset;
>> +    unsigned int ch_pending_req;
>>       unsigned int ch_fifo_ctrl;
>>       unsigned int ch_req_mask;
>>       unsigned int ch_req_max;
>> @@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct 
>> tegra_adma_chan *tdc,
>>                ADMA_CH_CTRL_FLOWCTRL_EN;
>>       ch_regs->config |= cdata->adma_get_burst_config(burst_size);
>>       ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
>> +    ch_regs->config |= cdata->ch_pending_req;
>>       ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
>>       ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
>>   @@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data 
>> tegra210_chip_data = {
>>       .ch_req_tx_shift    = 28,
>>       .ch_req_rx_shift    = 24,
>>       .ch_base_offset        = 0,
>> +    .ch_pending_req        = 0,
>>       .ch_fifo_ctrl        = TEGRA210_FIFO_CTRL_DEFAULT,
>>       .ch_req_mask        = 0xf,
>>       .ch_req_max        = 10,
>> @@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data 
>> tegra186_chip_data = {
>>       .ch_req_tx_shift    = 27,
>>       .ch_req_rx_shift    = 22,
>>       .ch_base_offset        = 0x10000,
>> +    .ch_pending_req        = (TEGRA186_DMA_MAX_PENDING_REQS << 4),
>>       .ch_fifo_ctrl        = TEGRA186_FIFO_CTRL_DEFAULT,
>>       .ch_req_mask        = 0x1f,
>>       .ch_req_max        = 20,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
  2019-07-12 11:10 [PATCH v2] dmaengine: tegra210-adma: fix transfer failure Sameer Pujar
  2019-07-23  6:11 ` Sameer Pujar
@ 2019-08-08 14:08 ` Vinod Koul
  2019-09-16  7:35   ` Sameer Pujar
  2019-08-19 17:40 ` Jon Hunter
  2 siblings, 1 reply; 7+ messages in thread
From: Vinod Koul @ 2019-08-08 14:08 UTC (permalink / raw)
  To: Sameer Pujar; +Cc: jonathanh, ldewangan, thierry.reding, dmaengine, linux-tegra

On 12-07-19, 16:40, Sameer Pujar wrote:
> >From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
  ^^
please remove the leading char from the para

> configuration register(bits 7:4) which defines the maximum number of reads
> from the source and writes to the destination that may be outstanding at
> any given point of time. This field must be programmed with a value
> between 1 and 8. A value of 0 will prevent any transfers from happening.
> 
> Thus added 'ch_pending_req' member in chip data structure and the same is
> populated with maximum allowed pending requests. Since the field is not
> applicable to Tegra210, mentioned bit fields are unused and hence the
> member is initialized with 0. For Tegra186, by default program this field
> with the maximum permitted value of 8.
> 
> Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for Tegra186/Tegra194")

Should this be tagged stable? Also some reviews from Tegra folks would
be great

> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  drivers/dma/tegra210-adma.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 2805853..5ab4e3a9 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -74,6 +74,8 @@
>  				    TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
>  				    TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
>  
> +#define TEGRA186_DMA_MAX_PENDING_REQS			8
> +
>  #define ADMA_CH_REG_FIELD_VAL(val, mask, shift)	(((val) & mask) << shift)
>  
>  struct tegra_adma;
> @@ -85,6 +87,7 @@ struct tegra_adma;
>   * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
>   * @ch_req_rx_shift: Register offset for AHUB receive channel select.
>   * @ch_base_offset: Register offset of DMA channel registers.
> + * @ch_pending_req: Outstaning DMA requests for a channel.
>   * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
>   * @ch_req_mask: Mask for Tx or Rx channel select.
>   * @ch_req_max: Maximum number of Tx or Rx channels available.
> @@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
>  	unsigned int ch_req_tx_shift;
>  	unsigned int ch_req_rx_shift;
>  	unsigned int ch_base_offset;
> +	unsigned int ch_pending_req;
>  	unsigned int ch_fifo_ctrl;
>  	unsigned int ch_req_mask;
>  	unsigned int ch_req_max;
> @@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
>  			 ADMA_CH_CTRL_FLOWCTRL_EN;
>  	ch_regs->config |= cdata->adma_get_burst_config(burst_size);
>  	ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
> +	ch_regs->config |= cdata->ch_pending_req;

so for tegra186 this will be 0, which per above would prevent any
transfers?? What did i miss

>  	ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
>  	ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
>  
> @@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
>  	.ch_req_tx_shift	= 28,
>  	.ch_req_rx_shift	= 24,
>  	.ch_base_offset		= 0,
> +	.ch_pending_req		= 0,
>  	.ch_fifo_ctrl		= TEGRA210_FIFO_CTRL_DEFAULT,
>  	.ch_req_mask		= 0xf,
>  	.ch_req_max		= 10,
> @@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
>  	.ch_req_tx_shift	= 27,
>  	.ch_req_rx_shift	= 22,
>  	.ch_base_offset		= 0x10000,
> +	.ch_pending_req		= (TEGRA186_DMA_MAX_PENDING_REQS << 4),
>  	.ch_fifo_ctrl		= TEGRA186_FIFO_CTRL_DEFAULT,
>  	.ch_req_mask		= 0x1f,
>  	.ch_req_max		= 20,
> -- 
> 2.7.4

-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
  2019-07-12 11:10 [PATCH v2] dmaengine: tegra210-adma: fix transfer failure Sameer Pujar
  2019-07-23  6:11 ` Sameer Pujar
  2019-08-08 14:08 ` Vinod Koul
@ 2019-08-19 17:40 ` Jon Hunter
  2019-09-16  7:37   ` Sameer Pujar
  2 siblings, 1 reply; 7+ messages in thread
From: Jon Hunter @ 2019-08-19 17:40 UTC (permalink / raw)
  To: Sameer Pujar, vkoul, ldewangan; +Cc: thierry.reding, dmaengine, linux-tegra


On 12/07/2019 12:10, Sameer Pujar wrote:
> From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
> configuration register(bits 7:4) which defines the maximum number of reads
> from the source and writes to the destination that may be outstanding at
> any given point of time. This field must be programmed with a value
> between 1 and 8. A value of 0 will prevent any transfers from happening.
> 
> Thus added 'ch_pending_req' member in chip data structure and the same is
> populated with maximum allowed pending requests. Since the field is not
> applicable to Tegra210, mentioned bit fields are unused and hence the
> member is initialized with 0. For Tegra186, by default program this field
> with the maximum permitted value of 8.
> 
> Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for Tegra186/Tegra194")
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  drivers/dma/tegra210-adma.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 2805853..5ab4e3a9 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -74,6 +74,8 @@
>  				    TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
>  				    TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
>  
> +#define TEGRA186_DMA_MAX_PENDING_REQS			8
> +
>  #define ADMA_CH_REG_FIELD_VAL(val, mask, shift)	(((val) & mask) << shift)
>  
>  struct tegra_adma;
> @@ -85,6 +87,7 @@ struct tegra_adma;
>   * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
>   * @ch_req_rx_shift: Register offset for AHUB receive channel select.
>   * @ch_base_offset: Register offset of DMA channel registers.
> + * @ch_pending_req: Outstaning DMA requests for a channel.

s/Outstaning/Outstanding

I do wonder if this variable should be a boolean variable
'has_oustanding_reqs' because this is not applicable to Tegra210. I
think this will be clearer that this is a difference between SoC
versions and that it should not be configured for Tegra210. And then ...

>   * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
>   * @ch_req_mask: Mask for Tx or Rx channel select.
>   * @ch_req_max: Maximum number of Tx or Rx channels available.
> @@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
>  	unsigned int ch_req_tx_shift;
>  	unsigned int ch_req_rx_shift;
>  	unsigned int ch_base_offset;
> +	unsigned int ch_pending_req;
>  	unsigned int ch_fifo_ctrl;
>  	unsigned int ch_req_mask;
>  	unsigned int ch_req_max;
> @@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
>  			 ADMA_CH_CTRL_FLOWCTRL_EN;
>  	ch_regs->config |= cdata->adma_get_burst_config(burst_size);
>  	ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
> +	ch_regs->config |= cdata->ch_pending_req;

... you can ...

        if (cdata->has_outstanding_reqs)
            ch_regs->config |= TEGRA186_ADMA_CH_CONFIG_OUTSTNDREQS(8)

>  	ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
>  	ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
>  
> @@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
>  	.ch_req_tx_shift	= 28,
>  	.ch_req_rx_shift	= 24,
>  	.ch_base_offset		= 0,
> +	.ch_pending_req		= 0,
>  	.ch_fifo_ctrl		= TEGRA210_FIFO_CTRL_DEFAULT,
>  	.ch_req_mask		= 0xf,
>  	.ch_req_max		= 10,
> @@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
>  	.ch_req_tx_shift	= 27,
>  	.ch_req_rx_shift	= 22,
>  	.ch_base_offset		= 0x10000,
> +	.ch_pending_req		= (TEGRA186_DMA_MAX_PENDING_REQS << 4),
>  	.ch_fifo_ctrl		= TEGRA186_FIFO_CTRL_DEFAULT,
>  	.ch_req_mask		= 0x1f,
>  	.ch_req_max		= 20,
> 

-- 
nvpublic

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
  2019-08-08 14:08 ` Vinod Koul
@ 2019-09-16  7:35   ` Sameer Pujar
  0 siblings, 0 replies; 7+ messages in thread
From: Sameer Pujar @ 2019-09-16  7:35 UTC (permalink / raw)
  To: Vinod Koul; +Cc: jonathanh, ldewangan, thierry.reding, dmaengine, linux-tegra

Sorry for the delayed reply.

On 8/8/2019 7:38 PM, Vinod Koul wrote:
> On 12-07-19, 16:40, Sameer Pujar wrote:
>> >From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
>    ^^
> please remove the leading char from the para
Are you referring to extra char '>' here? I don't see this in my diff patch.
>
>> configuration register(bits 7:4) which defines the maximum number of reads
>> from the source and writes to the destination that may be outstanding at
>> any given point of time. This field must be programmed with a value
>> between 1 and 8. A value of 0 will prevent any transfers from happening.
>>
>> Thus added 'ch_pending_req' member in chip data structure and the same is
>> populated with maximum allowed pending requests. Since the field is not
>> applicable to Tegra210, mentioned bit fields are unused and hence the
>> member is initialized with 0. For Tegra186, by default program this field
>> with the maximum permitted value of 8.
>>
>> Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for Tegra186/Tegra194")
> Should this be tagged stable? Also some reviews from Tegra folks would
> be great
Ok, will tag this for stable.
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> ---
>>   drivers/dma/tegra210-adma.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index 2805853..5ab4e3a9 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -74,6 +74,8 @@
>>   				    TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
>>   				    TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
>>   
>> +#define TEGRA186_DMA_MAX_PENDING_REQS			8
>> +
>>   #define ADMA_CH_REG_FIELD_VAL(val, mask, shift)	(((val) & mask) << shift)
>>   
>>   struct tegra_adma;
>> @@ -85,6 +87,7 @@ struct tegra_adma;
>>    * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
>>    * @ch_req_rx_shift: Register offset for AHUB receive channel select.
>>    * @ch_base_offset: Register offset of DMA channel registers.
>> + * @ch_pending_req: Outstaning DMA requests for a channel.
>>    * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
>>    * @ch_req_mask: Mask for Tx or Rx channel select.
>>    * @ch_req_max: Maximum number of Tx or Rx channels available.
>> @@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
>>   	unsigned int ch_req_tx_shift;
>>   	unsigned int ch_req_rx_shift;
>>   	unsigned int ch_base_offset;
>> +	unsigned int ch_pending_req;
>>   	unsigned int ch_fifo_ctrl;
>>   	unsigned int ch_req_mask;
>>   	unsigned int ch_req_max;
>> @@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
>>   			 ADMA_CH_CTRL_FLOWCTRL_EN;
>>   	ch_regs->config |= cdata->adma_get_burst_config(burst_size);
>>   	ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
>> +	ch_regs->config |= cdata->ch_pending_req;
> so for tegra186 this will be 0, which per above would prevent any
> transfers?? What did i miss
For tegra186/tegra194 this will be non-zero and for tegra210 it will be 0.
The chip data reflects above.
>>   	ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
>>   	ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
>>   
>> @@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
>>   	.ch_req_tx_shift	= 28,
>>   	.ch_req_rx_shift	= 24,
>>   	.ch_base_offset		= 0,
>> +	.ch_pending_req		= 0,
>>   	.ch_fifo_ctrl		= TEGRA210_FIFO_CTRL_DEFAULT,
>>   	.ch_req_mask		= 0xf,
>>   	.ch_req_max		= 10,
>> @@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
>>   	.ch_req_tx_shift	= 27,
>>   	.ch_req_rx_shift	= 22,
>>   	.ch_base_offset		= 0x10000,
>> +	.ch_pending_req		= (TEGRA186_DMA_MAX_PENDING_REQS << 4),
>>   	.ch_fifo_ctrl		= TEGRA186_FIFO_CTRL_DEFAULT,
>>   	.ch_req_mask		= 0x1f,
>>   	.ch_req_max		= 20,
>> -- 
>> 2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: tegra210-adma: fix transfer failure
  2019-08-19 17:40 ` Jon Hunter
@ 2019-09-16  7:37   ` Sameer Pujar
  0 siblings, 0 replies; 7+ messages in thread
From: Sameer Pujar @ 2019-09-16  7:37 UTC (permalink / raw)
  To: Jon Hunter, vkoul, ldewangan; +Cc: thierry.reding, dmaengine, linux-tegra

Sorry for delayed reply, I was consumed in some other work.

On 8/19/2019 11:10 PM, Jon Hunter wrote:
> On 12/07/2019 12:10, Sameer Pujar wrote:
>>  From Tegra186 onwards OUTSTANDING_REQUESTS field is added in channel
>> configuration register(bits 7:4) which defines the maximum number of reads
>> from the source and writes to the destination that may be outstanding at
>> any given point of time. This field must be programmed with a value
>> between 1 and 8. A value of 0 will prevent any transfers from happening.
>>
>> Thus added 'ch_pending_req' member in chip data structure and the same is
>> populated with maximum allowed pending requests. Since the field is not
>> applicable to Tegra210, mentioned bit fields are unused and hence the
>> member is initialized with 0. For Tegra186, by default program this field
>> with the maximum permitted value of 8.
>>
>> Fixes: 433de642a76c ("dmaengine: tegra210-adma: add support for Tegra186/Tegra194")
>>
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> ---
>>   drivers/dma/tegra210-adma.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index 2805853..5ab4e3a9 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -74,6 +74,8 @@
>>   				    TEGRA186_ADMA_CH_FIFO_CTRL_TXSIZE(3)    | \
>>   				    TEGRA186_ADMA_CH_FIFO_CTRL_RXSIZE(3))
>>   
>> +#define TEGRA186_DMA_MAX_PENDING_REQS			8
>> +
>>   #define ADMA_CH_REG_FIELD_VAL(val, mask, shift)	(((val) & mask) << shift)
>>   
>>   struct tegra_adma;
>> @@ -85,6 +87,7 @@ struct tegra_adma;
>>    * @ch_req_tx_shift: Register offset for AHUB transmit channel select.
>>    * @ch_req_rx_shift: Register offset for AHUB receive channel select.
>>    * @ch_base_offset: Register offset of DMA channel registers.
>> + * @ch_pending_req: Outstaning DMA requests for a channel.
> s/Outstaning/Outstanding
will fix.
> I do wonder if this variable should be a boolean variable
> 'has_oustanding_reqs' because this is not applicable to Tegra210. I
> think this will be clearer that this is a difference between SoC
> versions and that it should not be configured for Tegra210. And then ...
>
>>    * @ch_fifo_ctrl: Default value for channel FIFO CTRL register.
>>    * @ch_req_mask: Mask for Tx or Rx channel select.
>>    * @ch_req_max: Maximum number of Tx or Rx channels available.
>> @@ -98,6 +101,7 @@ struct tegra_adma_chip_data {
>>   	unsigned int ch_req_tx_shift;
>>   	unsigned int ch_req_rx_shift;
>>   	unsigned int ch_base_offset;
>> +	unsigned int ch_pending_req;
>>   	unsigned int ch_fifo_ctrl;
>>   	unsigned int ch_req_mask;
>>   	unsigned int ch_req_max;
>> @@ -602,6 +606,7 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
>>   			 ADMA_CH_CTRL_FLOWCTRL_EN;
>>   	ch_regs->config |= cdata->adma_get_burst_config(burst_size);
>>   	ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1);
>> +	ch_regs->config |= cdata->ch_pending_req;
> ... you can ...
>
>          if (cdata->has_outstanding_reqs)
>              ch_regs->config |= TEGRA186_ADMA_CH_CONFIG_OUTSTNDREQS(8)
yes, this can be done. Will update in next revision.
>
>>   	ch_regs->fifo_ctrl = cdata->ch_fifo_ctrl;
>>   	ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK;
>>   
>> @@ -786,6 +791,7 @@ static const struct tegra_adma_chip_data tegra210_chip_data = {
>>   	.ch_req_tx_shift	= 28,
>>   	.ch_req_rx_shift	= 24,
>>   	.ch_base_offset		= 0,
>> +	.ch_pending_req		= 0,
>>   	.ch_fifo_ctrl		= TEGRA210_FIFO_CTRL_DEFAULT,
>>   	.ch_req_mask		= 0xf,
>>   	.ch_req_max		= 10,
>> @@ -800,6 +806,7 @@ static const struct tegra_adma_chip_data tegra186_chip_data = {
>>   	.ch_req_tx_shift	= 27,
>>   	.ch_req_rx_shift	= 22,
>>   	.ch_base_offset		= 0x10000,
>> +	.ch_pending_req		= (TEGRA186_DMA_MAX_PENDING_REQS << 4),
>>   	.ch_fifo_ctrl		= TEGRA186_FIFO_CTRL_DEFAULT,
>>   	.ch_req_mask		= 0x1f,
>>   	.ch_req_max		= 20,
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-12 11:10 [PATCH v2] dmaengine: tegra210-adma: fix transfer failure Sameer Pujar
2019-07-23  6:11 ` Sameer Pujar
2019-08-06 14:45   ` Sameer Pujar
2019-08-08 14:08 ` Vinod Koul
2019-09-16  7:35   ` Sameer Pujar
2019-08-19 17:40 ` Jon Hunter
2019-09-16  7:37   ` Sameer Pujar

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox