Messages in this thread | | | Date | Thu, 7 Jul 2022 15:11:04 +0100 | Subject | Re: [PATCH 1/2] dmaengine: tegra: Add terminate() for Tegra234 | From | Jon Hunter <> |
| |
On 07/07/2022 11:47, Akhil R wrote: >> On 07/07/2022 11:27, Akhil R wrote: >>> In certain cases where the DMA client bus gets corrupted or if the end >>> device ceases to send/receive data, DMA can wait indefinitely for the >>> data to be received/sent. Attempting to terminate the transfer will >>> put the DMA in pause flush mode and it remains there. >>> >>> The channel is irrecoverable once this pause times out in Tegra194 and >>> earlier chips. Whereas, from Tegra234, it can be recovered by >>> disabling the channel and reprograming it. >>> >>> Hence add a new terminate() function that ignores the outcome of >>> dma_pause() and disables the channel. >>> >>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com> >>> --- >>> arch/arm64/boot/dts/nvidia/tegra234.dtsi | 5 +++-- >>> drivers/dma/tegra186-gpc-dma.c | 26 ++++++++++++++++++++++-- >>> 2 files changed, 27 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> b/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> index cf611eff7f6b..83d1ad7d3c8c 100644 >>> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> @@ -22,8 +22,9 @@ >>> ranges = <0x0 0x0 0x0 0x40000000>; >>> >>> gpcdma: dma-controller@2600000 { >>> - compatible = "nvidia,tegra194-gpcdma", >>> - "nvidia,tegra186-gpcdma"; >>> + compatible = "nvidia,tegra234-gpcdma", >>> + "nvidia,tegra194-gpcdma", >>> + "nvidia,tegra186-gpcdma"; >>> reg = <0x2600000 0x210000>; >>> resets = <&bpmp TEGRA234_RESET_GPCDMA>; >>> reset-names = "gpcdma"; >> >> I think that this should be split into two patches. >> >>> diff --git a/drivers/dma/tegra186-gpc-dma.c >>> b/drivers/dma/tegra186-gpc-dma.c index 05cd451f541d..fa9bda4a2bc6 >>> 100644 >>> --- a/drivers/dma/tegra186-gpc-dma.c >>> +++ b/drivers/dma/tegra186-gpc-dma.c >>> @@ -157,8 +157,8 @@ >>> * If any burst is in flight and DMA paused then this is the time to complete >>> * on-flight burst and update DMA status register. >>> */ >>> -#define TEGRA_GPCDMA_BURST_COMPLETE_TIME 20 >>> -#define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT 100 >>> +#define TEGRA_GPCDMA_BURST_COMPLETE_TIME 10 >>> +#define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT 5000 /* 5 >> msec */ >>> >>> /* Channel base address offset from GPCDMA base address */ >>> #define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET 0x20000 >>> @@ -432,6 +432,17 @@ static int tegra_dma_device_resume(struct >> dma_chan *dc) >>> return 0; >>> } >>> >>> +static inline int tegra_dma_pause_noerr(struct tegra_dma_channel >>> +*tdc) { >>> + /* Return 0 irrespective of PAUSE status. >>> + * This is useful to recover channels that can exit out of flush >>> + * state when the channel is disabled. >>> + */ >>> + >>> + tegra_dma_pause(tdc); >>> + return 0; >>> +} >> >> The commit message says that "add a new terminate() function that ignores the >> outcome of dma_pause() and disables the channel". But I only see pause being >> done here. > The function is set as .terminate() function in chip_data and is called during > terminate_all(). Since this return 0, tegra_dma_terminate_all() will proceed > and calls tegra_dma_disable() in the next step.
That's fine, but it is not clear from the commit message.
Jon
-- nvpublic
| |