diff mbox series

[v2,1/2] soc/tegra: fuse: Fix reading registers using DMA on Tegra20

Message ID 1fe535d4655ae00199a8d60768b384e87df3ed51.1507068874.git.digetx@gmail.com
State Changes Requested
Headers show
Series Correct Tegra20 FUSE driver DMA usage | expand

Commit Message

Dmitry Osipenko Oct. 3, 2017, 10:35 p.m. UTC 
DMA config is incorrect, because of it DMA transfer is never issued and
tegra20_fuse_read() always returns 0x0.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/soc/tegra/fuse/fuse-tegra.c   | 1 +
 drivers/soc/tegra/fuse/fuse-tegra20.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Jon Hunter Oct. 4, 2017, 10:47 a.m. UTC | #1 
On 03/10/17 23:35, Dmitry Osipenko wrote:
> DMA config is incorrect, because of it DMA transfer is never issued and
> tegra20_fuse_read() always returns 0x0.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/soc/tegra/fuse/fuse-tegra.c   | 1 +
>  drivers/soc/tegra/fuse/fuse-tegra20.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c
> index b7c552e3133c..73a3a2c74021 100644
> --- a/drivers/soc/tegra/fuse/fuse-tegra.c
> +++ b/drivers/soc/tegra/fuse/fuse-tegra.c
> @@ -132,6 +132,7 @@ static int tegra_fuse_probe(struct platform_device *pdev)
>  
>  	/* take over the memory region from the early initialization */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	fuse->phys = res->start;
>  	fuse->base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(fuse->base))
>  		return PTR_ERR(fuse->base);
> diff --git a/drivers/soc/tegra/fuse/fuse-tegra20.c b/drivers/soc/tegra/fuse/fuse-tegra20.c
> index 294413a969a0..a33f48c06771 100644
> --- a/drivers/soc/tegra/fuse/fuse-tegra20.c
> +++ b/drivers/soc/tegra/fuse/fuse-tegra20.c
> @@ -59,7 +59,7 @@ static u32 tegra20_fuse_read(struct tegra_fuse *fuse, unsigned int offset)
>  
>  	mutex_lock(&fuse->apbdma.lock);
>  
> -	fuse->apbdma.config.src_addr = fuse->apbdma.phys + FUSE_BEGIN + offset;
> +	fuse->apbdma.config.src_addr = fuse->phys + FUSE_BEGIN + offset;
>  
>  	err = dmaengine_slave_config(fuse->apbdma.chan, &fuse->apbdma.config);
>  	if (err)
> @@ -119,6 +119,7 @@ static int tegra20_fuse_probe(struct tegra_fuse *fuse)
>  	fuse->apbdma.config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>  	fuse->apbdma.config.src_maxburst = 1;
>  	fuse->apbdma.config.dst_maxburst = 1;
> +	fuse->apbdma.config.direction = DMA_DEV_TO_MEM;
>  
>  	init_completion(&fuse->apbdma.wait);
>  	mutex_init(&fuse->apbdma.lock);

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon
Jon Hunter Oct. 5, 2017, 12:22 p.m. UTC | #2 
On 04/10/17 11:47, Jon Hunter wrote:
> 
> On 03/10/17 23:35, Dmitry Osipenko wrote:
>> DMA config is incorrect, because of it DMA transfer is never issued and
>> tegra20_fuse_read() always returns 0x0.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/soc/tegra/fuse/fuse-tegra.c   | 1 +
>>  drivers/soc/tegra/fuse/fuse-tegra20.c | 3 ++-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c
>> index b7c552e3133c..73a3a2c74021 100644
>> --- a/drivers/soc/tegra/fuse/fuse-tegra.c
>> +++ b/drivers/soc/tegra/fuse/fuse-tegra.c
>> @@ -132,6 +132,7 @@ static int tegra_fuse_probe(struct platform_device *pdev)
>>  
>>  	/* take over the memory region from the early initialization */
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	fuse->phys = res->start;
>>  	fuse->base = devm_ioremap_resource(&pdev->dev, res);
>>  	if (IS_ERR(fuse->base))
>>  		return PTR_ERR(fuse->base);
>> diff --git a/drivers/soc/tegra/fuse/fuse-tegra20.c b/drivers/soc/tegra/fuse/fuse-tegra20.c
>> index 294413a969a0..a33f48c06771 100644
>> --- a/drivers/soc/tegra/fuse/fuse-tegra20.c
>> +++ b/drivers/soc/tegra/fuse/fuse-tegra20.c
>> @@ -59,7 +59,7 @@ static u32 tegra20_fuse_read(struct tegra_fuse *fuse, unsigned int offset)
>>  
>>  	mutex_lock(&fuse->apbdma.lock);
>>  
>> -	fuse->apbdma.config.src_addr = fuse->apbdma.phys + FUSE_BEGIN + offset;
>> +	fuse->apbdma.config.src_addr = fuse->phys + FUSE_BEGIN + offset;
>>  
>>  	err = dmaengine_slave_config(fuse->apbdma.chan, &fuse->apbdma.config);
>>  	if (err)
>> @@ -119,6 +119,7 @@ static int tegra20_fuse_probe(struct tegra_fuse *fuse)
>>  	fuse->apbdma.config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>  	fuse->apbdma.config.src_maxburst = 1;
>>  	fuse->apbdma.config.dst_maxburst = 1;
>> +	fuse->apbdma.config.direction = DMA_DEV_TO_MEM;
>>  
>>  	init_completion(&fuse->apbdma.wait);
>>  	mutex_init(&fuse->apbdma.lock);
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>

BTW, if you do rev this series, then maybe it is worth explicitly
setting 'fuse->apbdma.config.device_fc = false'.

Cheers
Jon
Dmitry Osipenko Oct. 5, 2017, 12:41 p.m. UTC | #3 
On 05.10.2017 15:22, Jon Hunter wrote:
> 
> On 04/10/17 11:47, Jon Hunter wrote:
>>
>> On 03/10/17 23:35, Dmitry Osipenko wrote:
>>> DMA config is incorrect, because of it DMA transfer is never issued and
>>> tegra20_fuse_read() always returns 0x0.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/soc/tegra/fuse/fuse-tegra.c   | 1 +
>>>  drivers/soc/tegra/fuse/fuse-tegra20.c | 3 ++-
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c
>>> index b7c552e3133c..73a3a2c74021 100644
>>> --- a/drivers/soc/tegra/fuse/fuse-tegra.c
>>> +++ b/drivers/soc/tegra/fuse/fuse-tegra.c
>>> @@ -132,6 +132,7 @@ static int tegra_fuse_probe(struct platform_device *pdev)
>>>  
>>>  	/* take over the memory region from the early initialization */
>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	fuse->phys = res->start;
>>>  	fuse->base = devm_ioremap_resource(&pdev->dev, res);
>>>  	if (IS_ERR(fuse->base))
>>>  		return PTR_ERR(fuse->base);
>>> diff --git a/drivers/soc/tegra/fuse/fuse-tegra20.c b/drivers/soc/tegra/fuse/fuse-tegra20.c
>>> index 294413a969a0..a33f48c06771 100644
>>> --- a/drivers/soc/tegra/fuse/fuse-tegra20.c
>>> +++ b/drivers/soc/tegra/fuse/fuse-tegra20.c
>>> @@ -59,7 +59,7 @@ static u32 tegra20_fuse_read(struct tegra_fuse *fuse, unsigned int offset)
>>>  
>>>  	mutex_lock(&fuse->apbdma.lock);
>>>  
>>> -	fuse->apbdma.config.src_addr = fuse->apbdma.phys + FUSE_BEGIN + offset;
>>> +	fuse->apbdma.config.src_addr = fuse->phys + FUSE_BEGIN + offset;
>>>  
>>>  	err = dmaengine_slave_config(fuse->apbdma.chan, &fuse->apbdma.config);
>>>  	if (err)
>>> @@ -119,6 +119,7 @@ static int tegra20_fuse_probe(struct tegra_fuse *fuse)
>>>  	fuse->apbdma.config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>>  	fuse->apbdma.config.src_maxburst = 1;
>>>  	fuse->apbdma.config.dst_maxburst = 1;
>>> +	fuse->apbdma.config.direction = DMA_DEV_TO_MEM;
>>>  
>>>  	init_completion(&fuse->apbdma.wait);
>>>  	mutex_init(&fuse->apbdma.lock);
>>
>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> 
> BTW, if you do rev this series, then maybe it is worth explicitly
> setting 'fuse->apbdma.config.device_fc = false'.
> 

I'll make v3 for sure. Setting device_fc isn't strictly needed, but it is good
to have for expressiveness. I'll add it to v3, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c
index b7c552e3133c..73a3a2c74021 100644
--- a/drivers/soc/tegra/fuse/fuse-tegra.c
+++ b/drivers/soc/tegra/fuse/fuse-tegra.c
@@ -132,6 +132,7 @@  static int tegra_fuse_probe(struct platform_device *pdev)
 
 	/* take over the memory region from the early initialization */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	fuse->phys = res->start;
 	fuse->base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(fuse->base))
 		return PTR_ERR(fuse->base);
diff --git a/drivers/soc/tegra/fuse/fuse-tegra20.c b/drivers/soc/tegra/fuse/fuse-tegra20.c
index 294413a969a0..a33f48c06771 100644
--- a/drivers/soc/tegra/fuse/fuse-tegra20.c
+++ b/drivers/soc/tegra/fuse/fuse-tegra20.c
@@ -59,7 +59,7 @@  static u32 tegra20_fuse_read(struct tegra_fuse *fuse, unsigned int offset)
 
 	mutex_lock(&fuse->apbdma.lock);
 
-	fuse->apbdma.config.src_addr = fuse->apbdma.phys + FUSE_BEGIN + offset;
+	fuse->apbdma.config.src_addr = fuse->phys + FUSE_BEGIN + offset;
 
 	err = dmaengine_slave_config(fuse->apbdma.chan, &fuse->apbdma.config);
 	if (err)
@@ -119,6 +119,7 @@  static int tegra20_fuse_probe(struct tegra_fuse *fuse)
 	fuse->apbdma.config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	fuse->apbdma.config.src_maxburst = 1;
 	fuse->apbdma.config.dst_maxburst = 1;
+	fuse->apbdma.config.direction = DMA_DEV_TO_MEM;
 
 	init_completion(&fuse->apbdma.wait);
 	mutex_init(&fuse->apbdma.lock);