| Message ID | 1444980919-20331-2-git-send-email-jonathanh@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
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. > +- 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. Is there a dedicated reset signal for this module? If so, we should require a resets property. -- 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
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. Jon -- 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
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. -- 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
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 -- 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 --git a/Documentation/devicetree/bindings/dma/tegra210-adma.txt b/Documentation/devicetree/bindings/dma/tegra210-adma.txt new file mode 100644 index 000000000000..5e4eda1d1918 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/tegra210-adma.txt @@ -0,0 +1,50 @@ +* NVIDIA Tegra Audio DMA (ADMA) controller + +Required properties: +- compatible: Must be "nvidia,tegra210-adma". +- reg: Should contain DMA registers location and length. This should be + a single entry that includes all of the per-channel registers in one + contiguous bank. +- interrupt-parent: Phandle to the interrupt parent controller. +- interrupts: Should contain all of the per-channel DMA interrupts in + ascending order with respect to the DMA channel index. +- clocks: Must contain one entry for the ADMA module clock, "adma_ape". +- clock-names: Must contain the entry "adma_ape". +- #dma-cells : Must be 1. The first cell denotes the receive/transmit + request number and should be between 1 and the maximum number of + requests supported. This value corresponds to the RX/TX_REQUEST_SELECT + fields in the ADMA_CHn_CTRL register. + + +Example: + +adma: adma@702e2000 { + compatible = "nvidia,tegra210-adma"; + reg = <0x0 0x702e2000 0x0 0x2000>; + interrupt-parent = <&tegra_agic>; + interrupts = <GIC_SPI INT_ADMA_EOT0 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT1 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT2 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT3 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT4 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT5 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT6 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT7 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT8 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT9 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT10 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT11 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT12 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT13 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT14 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT15 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT16 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT17 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT18 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT19 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT20 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI INT_ADMA_EOT21 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA210_CLK_ADMA_APE>; + clock-names = "adma_ape"; + #dma-cells = <1>; +};
Add device-tree binding documentation for the Tegra210 Audio DMA controller. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- .../devicetree/bindings/dma/tegra210-adma.txt | 50 ++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/tegra210-adma.txt