[v1,2/3] ARM: dts: tegra20: Add DMA phandle to 'fuse' node

Message ID 8268404736bd3c254f8516109465bb8db4739c33.1506378772.git.digetx@gmail.com
State Superseded
Headers show
Series
  • [v1,1/3] soc/tegra: fuse: Fix reading registers using DMA on Tegra20
Related show

Commit Message

Dmitry Osipenko Sept. 25, 2017, 10:35 p.m.
Currently efuse driver requests DMA channel from an arbitrary DMA device,
it is not a problem since there is only one DMA provider for Tegra20 yet,
but it will become troublesome once another provider would be added.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/boot/dts/tegra20.dtsi | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jon Hunter Sept. 26, 2017, 8:54 p.m. | #1
On 25/09/17 23:35, Dmitry Osipenko wrote:
> Currently efuse driver requests DMA channel from an arbitrary DMA device,
> it is not a problem since there is only one DMA provider for Tegra20 yet,
> but it will become troublesome once another provider would be added.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  arch/arm/boot/dts/tegra20.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index fb485a5e63d7..f1579c9a7ef4 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -600,6 +600,8 @@
>  		clock-names = "fuse";
>  		resets = <&tegra_car 39>;
>  		reset-names = "fuse";
> +		dmas = <&apbdma 0>;
> +		dma-names = "fuse";
>  	};
>  
>  	pcie@80003000 {
> 

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

Cheers
Jon
Jon Hunter Sept. 26, 2017, 9:25 p.m. | #2
On 26/09/17 21:54, Jon Hunter wrote:
> 
> On 25/09/17 23:35, Dmitry Osipenko wrote:
>> Currently efuse driver requests DMA channel from an arbitrary DMA device,
>> it is not a problem since there is only one DMA provider for Tegra20 yet,
>> but it will become troublesome once another provider would be added.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/boot/dts/tegra20.dtsi | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>> index fb485a5e63d7..f1579c9a7ef4 100644
>> --- a/arch/arm/boot/dts/tegra20.dtsi
>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>> @@ -600,6 +600,8 @@
>>  		clock-names = "fuse";
>>  		resets = <&tegra_car 39>;
>>  		reset-names = "fuse";
>> +		dmas = <&apbdma 0>;
>> +		dma-names = "fuse";
>>  	};
>>  
>>  	pcie@80003000 {
>>
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>

Actually, request-id '0' is a valid request. Does this work ok?

Cheers
Jon
Dmitry Osipenko Sept. 26, 2017, 9:54 p.m. | #3
On 27.09.2017 00:25, Jon Hunter wrote:
> 
> On 26/09/17 21:54, Jon Hunter wrote:
>>
>> On 25/09/17 23:35, Dmitry Osipenko wrote:
>>> Currently efuse driver requests DMA channel from an arbitrary DMA device,
>>> it is not a problem since there is only one DMA provider for Tegra20 yet,
>>> but it will become troublesome once another provider would be added.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  arch/arm/boot/dts/tegra20.dtsi | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>> index fb485a5e63d7..f1579c9a7ef4 100644
>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>> @@ -600,6 +600,8 @@
>>>  		clock-names = "fuse";
>>>  		resets = <&tegra_car 39>;
>>>  		reset-names = "fuse";
>>> +		dmas = <&apbdma 0>;
>>> +		dma-names = "fuse";
>>>  	};
>>>  
>>>  	pcie@80003000 {
>>>
>>
>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Actually, request-id '0' is a valid request. Does this work ok?
> 

It works fine, I have verified that reading on CPU == reading by DMA. The
REQ_SEL 0 is "Not Assigned" and seems acts as DRQ=1. I know that it is not
entirely correct, but APB DMA driver is hardcoded to the master mode, while we
need slave mode.
Jon Hunter Sept. 26, 2017, 10:10 p.m. | #4
On 26/09/17 22:54, Dmitry Osipenko wrote:
> On 27.09.2017 00:25, Jon Hunter wrote:
>>
>> On 26/09/17 21:54, Jon Hunter wrote:
>>>
>>> On 25/09/17 23:35, Dmitry Osipenko wrote:
>>>> Currently efuse driver requests DMA channel from an arbitrary DMA device,
>>>> it is not a problem since there is only one DMA provider for Tegra20 yet,
>>>> but it will become troublesome once another provider would be added.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  arch/arm/boot/dts/tegra20.dtsi | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>> index fb485a5e63d7..f1579c9a7ef4 100644
>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>> @@ -600,6 +600,8 @@
>>>>  		clock-names = "fuse";
>>>>  		resets = <&tegra_car 39>;
>>>>  		reset-names = "fuse";
>>>> +		dmas = <&apbdma 0>;
>>>> +		dma-names = "fuse";
>>>>  	};
>>>>  
>>>>  	pcie@80003000 {
>>>>
>>>
>>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>>
>> Actually, request-id '0' is a valid request. Does this work ok?
>>
> 
> It works fine, I have verified that reading on CPU == reading by DMA. The
> REQ_SEL 0 is "Not Assigned" and seems acts as DRQ=1. I know that it is not
> entirely correct, but APB DMA driver is hardcoded to the master mode, while we
> need slave mode.

Looking at the TRM I see that it is 'CNTR_REQ' so I am not sure if this
is a timer/counter that is driving this.

Jon
Dmitry Osipenko Sept. 26, 2017, 10:31 p.m. | #5
On 27.09.2017 01:10, Jon Hunter wrote:
> 
> On 26/09/17 22:54, Dmitry Osipenko wrote:
>> On 27.09.2017 00:25, Jon Hunter wrote:
>>>
>>> On 26/09/17 21:54, Jon Hunter wrote:
>>>>
>>>> On 25/09/17 23:35, Dmitry Osipenko wrote:
>>>>> Currently efuse driver requests DMA channel from an arbitrary DMA device,
>>>>> it is not a problem since there is only one DMA provider for Tegra20 yet,
>>>>> but it will become troublesome once another provider would be added.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> ---
>>>>>  arch/arm/boot/dts/tegra20.dtsi | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>>> index fb485a5e63d7..f1579c9a7ef4 100644
>>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>>> @@ -600,6 +600,8 @@
>>>>>  		clock-names = "fuse";
>>>>>  		resets = <&tegra_car 39>;
>>>>>  		reset-names = "fuse";
>>>>> +		dmas = <&apbdma 0>;
>>>>> +		dma-names = "fuse";
>>>>>  	};
>>>>>  
>>>>>  	pcie@80003000 {
>>>>>
>>>>
>>>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>>>
>>> Actually, request-id '0' is a valid request. Does this work ok?
>>>
>>
>> It works fine, I have verified that reading on CPU == reading by DMA. The
>> REQ_SEL 0 is "Not Assigned" and seems acts as DRQ=1. I know that it is not
>> entirely correct, but APB DMA driver is hardcoded to the master mode, while we
>> need slave mode.
> 
> Looking at the TRM I see that it is 'CNTR_REQ' so I am not sure if this
> is a timer/counter that is driving this.
> 

Oh, wow. Indeed it's TRIG_SEL is NA. Good catch! So it works because counter = 0.

Then APB DMA driver needs to be changed to support master mode. Seems that
should be simple to change, I'll look into it.
Dmitry Osipenko Sept. 26, 2017, 10:40 p.m. | #6
On 27.09.2017 01:31, Dmitry Osipenko wrote:
> On 27.09.2017 01:10, Jon Hunter wrote:
>>
>> On 26/09/17 22:54, Dmitry Osipenko wrote:
>>> On 27.09.2017 00:25, Jon Hunter wrote:
>>>>
>>>> On 26/09/17 21:54, Jon Hunter wrote:
>>>>>
>>>>> On 25/09/17 23:35, Dmitry Osipenko wrote:
>>>>>> Currently efuse driver requests DMA channel from an arbitrary DMA device,
>>>>>> it is not a problem since there is only one DMA provider for Tegra20 yet,
>>>>>> but it will become troublesome once another provider would be added.
>>>>>>
>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>> ---
>>>>>>  arch/arm/boot/dts/tegra20.dtsi | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>>>>>> index fb485a5e63d7..f1579c9a7ef4 100644
>>>>>> --- a/arch/arm/boot/dts/tegra20.dtsi
>>>>>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>>>>>> @@ -600,6 +600,8 @@
>>>>>>  		clock-names = "fuse";
>>>>>>  		resets = <&tegra_car 39>;
>>>>>>  		reset-names = "fuse";
>>>>>> +		dmas = <&apbdma 0>;
>>>>>> +		dma-names = "fuse";
>>>>>>  	};
>>>>>>  
>>>>>>  	pcie@80003000 {
>>>>>>
>>>>>
>>>>> Acked-by: Jon Hunter <jonathanh@nvidia.com>
>>>>
>>>> Actually, request-id '0' is a valid request. Does this work ok?
>>>>
>>>
>>> It works fine, I have verified that reading on CPU == reading by DMA. The
>>> REQ_SEL 0 is "Not Assigned" and seems acts as DRQ=1. I know that it is not
>>> entirely correct, but APB DMA driver is hardcoded to the master mode, while we
>>> need slave mode.
>>
>> Looking at the TRM I see that it is 'CNTR_REQ' so I am not sure if this
>> is a timer/counter that is driving this.
>>
> 
> Oh, wow. Indeed it's TRIG_SEL is NA. Good catch! So it works because counter = 0.
> 
> Then APB DMA driver needs to be changed to support master mode. Seems that
> should be simple to change, I'll look into it.
> 

s/support master/support slave/ of course. Thank you for the review ;)

Patch

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index fb485a5e63d7..f1579c9a7ef4 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -600,6 +600,8 @@ 
 		clock-names = "fuse";
 		resets = <&tegra_car 39>;
 		reset-names = "fuse";
+		dmas = <&apbdma 0>;
+		dma-names = "fuse";
 	};
 
 	pcie@80003000 {