Re: [PATCH V3 1/2] Documentation: DT: Add binding documentation for NVIDIA ADMA

[Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]

On 19/10/15 17:33, Stephen Warren wrote:
> On 10/19/2015 05:22 AM, Jon Hunter wrote:
>>
>> On 16/10/15 17:09, Stephen Warren wrote:
>>> On 10/16/2015 01:35 AM, Jon Hunter wrote:
>>>> Add device-tree binding documentation for the Tegra210 Audio DMA
>>>> controller.
>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/tegra210-adma.txt
>>>> b/Documentation/devicetree/bindings/dma/tegra210-adma.txt
>>>
>>>> +Required properties:
>>>
>>>> +- interrupt-parent: Phandle to the interrupt parent controller.
>>>
>>> Nit: Since that is more of a "system level"/standard property, it's
>>> typical not to document it. The property is not actually required if the
>>> inherited value is already correct. Still, it's obvious enough what this
>>> means, so I'd only suggest fixing this if you have to respin for some
>>> other reason.
>>
>> Ok.
>>
>>>> +- clocks: Must contain one entry for the ADMA module clock,
>>>> "adma_ape".
>>>> +- clock-names: Must contain the entry "adma_ape".
>>>
>>> Which clock is this in the CAR? I don't see any adma_ape clock
>>> documented in the TRM.
>>
>> Darn. I thought I had checked this. Yes it should be the AHUB clock and
>> looking at the current t210 clock patches for mainline this is
>> TEGRA210_CLK_D_AUDIO. Ok will fix this.
>>
>>> Is there a dedicated reset signal for this module? If so, we should
>>> require a resets property.
>>
>> No there does not appear to be. Looking at the documentation the ADMA
>> would be reset by the main APE reset. Seems to be one reset that resets
>> most modules in the APE.
>>
>> That raises another issue, the ADMA is in the audio power partition and
>> currently our downstream driver assumes that this is on. I should at
>> least check this.
> 
> IIRC the situation downstream w.r.t. the audio power partition is
> something like:
> 
> The AGIC is in that power partition. The Linux kernel AGIC driver gets
> probed before the power domain driver for that domain. (I think)
> deferred probe doesn't work for the AGIC driver for some reason (perhaps
> this is just a bug or oversight in our downstream AGIC driver?). So, the
> AGIC driver can't turn on the audio power domain. So, there's a hack in
> the bootloader to turn the power domain on. So, everything in the kernel
> assumes that power domain is on at boot and so drivers don't have to
> explicitly control/request that power domain.
> 
> We should fix this properly so that upstream doesn't make the assumption
> that the audio power domain is magically on before the kernel boots.

Yes, I agree. Ok, I will look at that some more.

Jon