| Message ID | 20220707102725.41383-2-akhilrajeev@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add compatible for Tegra234 GPCDMA | expand |
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. > + > static void tegra_dma_disable(struct tegra_dma_channel *tdc) > { > u32 csr, status; > @@ -1292,6 +1303,14 @@ static const struct tegra_dma_chip_data tegra194_dma_chip_data = { > .terminate = tegra_dma_pause, > }; > > +static const struct tegra_dma_chip_data tegra234_dma_chip_data = { > + .nr_channels = 31, > + .channel_reg_size = SZ_64K, > + .max_dma_count = SZ_1G, > + .hw_support_pause = true, > + .terminate = tegra_dma_pause_noerr, > +}; > + > static const struct of_device_id tegra_dma_of_match[] = { > { > .compatible = "nvidia,tegra186-gpcdma", > @@ -1299,6 +1318,9 @@ static const struct of_device_id tegra_dma_of_match[] = { > }, { > .compatible = "nvidia,tegra194-gpcdma", > .data = &tegra194_dma_chip_data, > + }, { > + .compatible = "nvidia,tegra234-gpcdma", > + .data = &tegra234_dma_chip_data, > }, { > }, > };
> 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. Regards, Akhil -- nvpublic
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
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"; 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; +} + static void tegra_dma_disable(struct tegra_dma_channel *tdc) { u32 csr, status; @@ -1292,6 +1303,14 @@ static const struct tegra_dma_chip_data tegra194_dma_chip_data = { .terminate = tegra_dma_pause, }; +static const struct tegra_dma_chip_data tegra234_dma_chip_data = { + .nr_channels = 31, + .channel_reg_size = SZ_64K, + .max_dma_count = SZ_1G, + .hw_support_pause = true, + .terminate = tegra_dma_pause_noerr, +}; + static const struct of_device_id tegra_dma_of_match[] = { { .compatible = "nvidia,tegra186-gpcdma", @@ -1299,6 +1318,9 @@ static const struct of_device_id tegra_dma_of_match[] = { }, { .compatible = "nvidia,tegra194-gpcdma", .data = &tegra194_dma_chip_data, + }, { + .compatible = "nvidia,tegra234-gpcdma", + .data = &tegra234_dma_chip_data, }, { }, };
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(-)