* [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow
@ 2022-07-04 11:13 Vaishnav Achath
2022-07-09 6:20 ` Péter Ujfalusi
2022-07-21 13:32 ` Vinod Koul
0 siblings, 2 replies; 4+ messages in thread
From: Vaishnav Achath @ 2022-07-04 11:13 UTC (permalink / raw)
To: peter.ujfalusi, vkoul, dmaengine, linux-kernel
Cc: nm, vigneshr, p.yadav, vaishnav.a, j-keerthy, m-khayami,
stanley_liu
UDMA_CHAN_RT_*BCNT_REG stores the real-time channel bytecount statistics.
These registers are 32-bit hardware counters and the driver uses these
counters to monitor the operational progress status for a channel, when
transferring more than 4GB of data it was observed that these counters
overflow and completion calculation of a operation gets affected and the
transfer hangs indefinitely.
This commit adds changes to decrease the byte count for every complete
transaction so that these registers never overflow and the proper byte
count statistics is maintained for ongoing transaction by the RT counters.
Earlier uc->bcnt used to maintain a count of the completed bytes at driver
side, since the RT counters maintain the statistics of current transaction
now, the maintenance of uc->bcnt is not necessary.
Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---
drivers/dma/ti/k3-udma.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 2f0d2c68c93c..0f91a3e47c19 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -300,8 +300,6 @@ struct udma_chan {
struct udma_tx_drain tx_drain;
- u32 bcnt; /* number of bytes completed since the start of the channel */
-
/* Channel configuration parameters */
struct udma_chan_config config;
@@ -757,6 +755,22 @@ static void udma_reset_rings(struct udma_chan *uc)
}
}
+static void udma_decrement_byte_counters(struct udma_chan *uc, u32 val)
+{
+ if (uc->tchan) {
+ udma_tchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
+ udma_tchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
+ if (!uc->bchan)
+ udma_tchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
+ }
+
+ if (uc->rchan) {
+ udma_rchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
+ udma_rchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
+ udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
+ }
+}
+
static void udma_reset_counters(struct udma_chan *uc)
{
u32 val;
@@ -790,8 +804,6 @@ static void udma_reset_counters(struct udma_chan *uc)
val = udma_rchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
}
-
- uc->bcnt = 0;
}
static int udma_reset_chan(struct udma_chan *uc, bool hard)
@@ -1115,8 +1127,8 @@ static void udma_check_tx_completion(struct work_struct *work)
if (uc->desc) {
struct udma_desc *d = uc->desc;
- uc->bcnt += d->residue;
udma_start(uc);
+ udma_decrement_byte_counters(uc, d->residue);
vchan_cookie_complete(&d->vd);
break;
}
@@ -1168,8 +1180,8 @@ static irqreturn_t udma_ring_irq_handler(int irq, void *data)
vchan_cyclic_callback(&d->vd);
} else {
if (udma_is_desc_really_done(uc, d)) {
- uc->bcnt += d->residue;
udma_start(uc);
+ udma_decrement_byte_counters(uc, d->residue);
vchan_cookie_complete(&d->vd);
} else {
schedule_delayed_work(&uc->tx_drain.work,
@@ -1204,7 +1216,7 @@ static irqreturn_t udma_udma_irq_handler(int irq, void *data)
vchan_cyclic_callback(&d->vd);
} else {
/* TODO: figure out the real amount of data */
- uc->bcnt += d->residue;
+ udma_decrement_byte_counters(uc, d->residue);
udma_start(uc);
vchan_cookie_complete(&d->vd);
}
@@ -3809,7 +3821,6 @@ static enum dma_status udma_tx_status(struct dma_chan *chan,
bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_BCNT_REG);
}
- bcnt -= uc->bcnt;
if (bcnt && !(bcnt % uc->desc->residue))
residue = 0;
else
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow
2022-07-04 11:13 [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow Vaishnav Achath
@ 2022-07-09 6:20 ` Péter Ujfalusi
2022-07-27 14:46 ` Vaishnav Achath
2022-07-21 13:32 ` Vinod Koul
1 sibling, 1 reply; 4+ messages in thread
From: Péter Ujfalusi @ 2022-07-09 6:20 UTC (permalink / raw)
To: Vaishnav Achath, vkoul, dmaengine, linux-kernel
Cc: nm, vigneshr, p.yadav, j-keerthy, m-khayami, stanley_liu
On 7/4/22 14:13, Vaishnav Achath wrote:
> UDMA_CHAN_RT_*BCNT_REG stores the real-time channel bytecount statistics.
> These registers are 32-bit hardware counters and the driver uses these
> counters to monitor the operational progress status for a channel, when
> transferring more than 4GB of data it was observed that these counters
> overflow and completion calculation of a operation gets affected and the
> transfer hangs indefinitely.
>
> This commit adds changes to decrease the byte count for every complete
> transaction so that these registers never overflow and the proper byte
> count statistics is maintained for ongoing transaction by the RT counters.
>
> Earlier uc->bcnt used to maintain a count of the completed bytes at driver
> side, since the RT counters maintain the statistics of current transaction
> now, the maintenance of uc->bcnt is not necessary.
Thanks for the patch,
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> ---
> drivers/dma/ti/k3-udma.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index 2f0d2c68c93c..0f91a3e47c19 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -300,8 +300,6 @@ struct udma_chan {
>
> struct udma_tx_drain tx_drain;
>
> - u32 bcnt; /* number of bytes completed since the start of the channel */
> -
> /* Channel configuration parameters */
> struct udma_chan_config config;
>
> @@ -757,6 +755,22 @@ static void udma_reset_rings(struct udma_chan *uc)
> }
> }
>
> +static void udma_decrement_byte_counters(struct udma_chan *uc, u32 val)
> +{
> + if (uc->tchan) {
> + udma_tchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
> + udma_tchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
> + if (!uc->bchan)
> + udma_tchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
> + }
> +
> + if (uc->rchan) {
> + udma_rchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
> + udma_rchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
> + udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
> + }
In case of MEM_TO_MEM (or the not implemented DEV_TO_DEV) we use the
tchan's counter for position tracking, but we have the pair anyways (UDMA).
if ((uc->desc->dir == DMA_DEV_TO_MEM)
rchan bcnt reset
else
tchan bcnt reset
> +}
> +
> static void udma_reset_counters(struct udma_chan *uc)
> {
> u32 val;
> @@ -790,8 +804,6 @@ static void udma_reset_counters(struct udma_chan *uc)
> val = udma_rchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
> udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
> }
> -
> - uc->bcnt = 0;
> }
>
> static int udma_reset_chan(struct udma_chan *uc, bool hard)
> @@ -1115,8 +1127,8 @@ static void udma_check_tx_completion(struct work_struct *work)
> if (uc->desc) {
> struct udma_desc *d = uc->desc;
>
> - uc->bcnt += d->residue;
> udma_start(uc);
> + udma_decrement_byte_counters(uc, d->residue);
Why not before udma_start()?
> vchan_cookie_complete(&d->vd);
> break;
> }
> @@ -1168,8 +1180,8 @@ static irqreturn_t udma_ring_irq_handler(int irq, void *data)
> vchan_cyclic_callback(&d->vd);
> } else {
> if (udma_is_desc_really_done(uc, d)) {
> - uc->bcnt += d->residue;
> udma_start(uc);
> + udma_decrement_byte_counters(uc, d->residue);
> vchan_cookie_complete(&d->vd);
> } else {
> schedule_delayed_work(&uc->tx_drain.work,
> @@ -1204,7 +1216,7 @@ static irqreturn_t udma_udma_irq_handler(int irq, void *data)
> vchan_cyclic_callback(&d->vd);
> } else {
> /* TODO: figure out the real amount of data */
> - uc->bcnt += d->residue;
> + udma_decrement_byte_counters(uc, d->residue);
> udma_start(uc);
> vchan_cookie_complete(&d->vd);
> }
> @@ -3809,7 +3821,6 @@ static enum dma_status udma_tx_status(struct dma_chan *chan,
> bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_BCNT_REG);
> }
>
> - bcnt -= uc->bcnt;
> if (bcnt && !(bcnt % uc->desc->residue))
> residue = 0;
> else
--
Péter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow
2022-07-09 6:20 ` Péter Ujfalusi
@ 2022-07-27 14:46 ` Vaishnav Achath
0 siblings, 0 replies; 4+ messages in thread
From: Vaishnav Achath @ 2022-07-27 14:46 UTC (permalink / raw)
To: Péter Ujfalusi, vkoul, dmaengine, linux-kernel
Cc: nm, vigneshr, p.yadav, j-keerthy, m-khayami, stanley_liu
Hi Peter, Vinod,
On 09/07/22 11:50, Péter Ujfalusi wrote:
>
>
> On 7/4/22 14:13, Vaishnav Achath wrote:
>> UDMA_CHAN_RT_*BCNT_REG stores the real-time channel bytecount statistics.
>> These registers are 32-bit hardware counters and the driver uses these
>> counters to monitor the operational progress status for a channel, when
>> transferring more than 4GB of data it was observed that these counters
>> overflow and completion calculation of a operation gets affected and the
>> transfer hangs indefinitely.
>>
>> This commit adds changes to decrease the byte count for every complete
>> transaction so that these registers never overflow and the proper byte
>> count statistics is maintained for ongoing transaction by the RT counters.
>>
>> Earlier uc->bcnt used to maintain a count of the completed bytes at driver
>> side, since the RT counters maintain the statistics of current transaction
>> now, the maintenance of uc->bcnt is not necessary.
>
> Thanks for the patch,
>
>> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
>> ---
>> drivers/dma/ti/k3-udma.c | 27 +++++++++++++++++++--------
>> 1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
>> index 2f0d2c68c93c..0f91a3e47c19 100644
>> --- a/drivers/dma/ti/k3-udma.c
>> +++ b/drivers/dma/ti/k3-udma.c
>> @@ -300,8 +300,6 @@ struct udma_chan {
>>
>> struct udma_tx_drain tx_drain;
>>
>> - u32 bcnt; /* number of bytes completed since the start of the channel */
>> -
>> /* Channel configuration parameters */
>> struct udma_chan_config config;
>>
>> @@ -757,6 +755,22 @@ static void udma_reset_rings(struct udma_chan *uc)
>> }
>> }
>>
>> +static void udma_decrement_byte_counters(struct udma_chan *uc, u32 val)
>> +{
>> + if (uc->tchan) {
>> + udma_tchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
>> + udma_tchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
>> + if (!uc->bchan)
>> + udma_tchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
>> + }
>> +
>> + if (uc->rchan) {
>> + udma_rchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
>> + udma_rchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
>> + udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
>> + }
>
> In case of MEM_TO_MEM (or the not implemented DEV_TO_DEV) we use the
> tchan's counter for position tracking, but we have the pair anyways (UDMA).
> if ((uc->desc->dir == DMA_DEV_TO_MEM)
> rchan bcnt reset
> else
> tchan bcnt reset
>
>> +}
>> +
>> static void udma_reset_counters(struct udma_chan *uc)
>> {
>> u32 val;
>> @@ -790,8 +804,6 @@ static void udma_reset_counters(struct udma_chan *uc)
>> val = udma_rchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
>> udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
>> }
>> -
>> - uc->bcnt = 0;
>> }
>>
>> static int udma_reset_chan(struct udma_chan *uc, bool hard)
>> @@ -1115,8 +1127,8 @@ static void udma_check_tx_completion(struct work_struct *work)
>> if (uc->desc) {
>> struct udma_desc *d = uc->desc;
>>
>> - uc->bcnt += d->residue;
>> udma_start(uc);
>> + udma_decrement_byte_counters(uc, d->residue);
>
> Why not before udma_start()?
Thank you for your review and feedback, I have updated the addressed items in
V2, Sorry for the delay in responding.
V2: https://patchwork.kernel.org/project/linux-dmaengine/patch/20220727140837.25877-1-vaishnav.a@ti.com/
>
>> vchan_cookie_complete(&d->vd);
>> break;
>> }
>> @@ -1168,8 +1180,8 @@ static irqreturn_t udma_ring_irq_handler(int irq, void *data)
>> vchan_cyclic_callback(&d->vd);
>> } else {
>> if (udma_is_desc_really_done(uc, d)) {
>> - uc->bcnt += d->residue;
>> udma_start(uc);
>> + udma_decrement_byte_counters(uc, d->residue);
>> vchan_cookie_complete(&d->vd);
>> } else {
>> schedule_delayed_work(&uc->tx_drain.work,
>> @@ -1204,7 +1216,7 @@ static irqreturn_t udma_udma_irq_handler(int irq, void *data)
>> vchan_cyclic_callback(&d->vd);
>> } else {
>> /* TODO: figure out the real amount of data */
>> - uc->bcnt += d->residue;
>> + udma_decrement_byte_counters(uc, d->residue);
>> udma_start(uc);
>> vchan_cookie_complete(&d->vd);
>> }
>> @@ -3809,7 +3821,6 @@ static enum dma_status udma_tx_status(struct dma_chan *chan,
>> bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_BCNT_REG);
>> }
>>
>> - bcnt -= uc->bcnt;
>> if (bcnt && !(bcnt % uc->desc->residue))
>> residue = 0;
>> else
>
--
Thanks and Regards,
Vaishnav
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow
2022-07-04 11:13 [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow Vaishnav Achath
2022-07-09 6:20 ` Péter Ujfalusi
@ 2022-07-21 13:32 ` Vinod Koul
1 sibling, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2022-07-21 13:32 UTC (permalink / raw)
To: Vaishnav Achath
Cc: peter.ujfalusi, dmaengine, linux-kernel, nm, vigneshr, p.yadav,
j-keerthy, m-khayami, stanley_liu
On 04-07-22, 16:43, Vaishnav Achath wrote:
> UDMA_CHAN_RT_*BCNT_REG stores the real-time channel bytecount statistics.
> These registers are 32-bit hardware counters and the driver uses these
> counters to monitor the operational progress status for a channel, when
> transferring more than 4GB of data it was observed that these counters
> overflow and completion calculation of a operation gets affected and the
> transfer hangs indefinitely.
>
> This commit adds changes to decrease the byte count for every complete
> transaction so that these registers never overflow and the proper byte
> count statistics is maintained for ongoing transaction by the RT counters.
>
> Earlier uc->bcnt used to maintain a count of the completed bytes at driver
> side, since the RT counters maintain the statistics of current transaction
> now, the maintenance of uc->bcnt is not necessary.
Along with Peter's comment, pls fix the subsystem tag!
>
> Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
> ---
> drivers/dma/ti/k3-udma.c | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index 2f0d2c68c93c..0f91a3e47c19 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -300,8 +300,6 @@ struct udma_chan {
>
> struct udma_tx_drain tx_drain;
>
> - u32 bcnt; /* number of bytes completed since the start of the channel */
> -
> /* Channel configuration parameters */
> struct udma_chan_config config;
>
> @@ -757,6 +755,22 @@ static void udma_reset_rings(struct udma_chan *uc)
> }
> }
>
> +static void udma_decrement_byte_counters(struct udma_chan *uc, u32 val)
> +{
> + if (uc->tchan) {
> + udma_tchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
> + udma_tchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
> + if (!uc->bchan)
> + udma_tchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
> + }
> +
> + if (uc->rchan) {
> + udma_rchanrt_write(uc, UDMA_CHAN_RT_BCNT_REG, val);
> + udma_rchanrt_write(uc, UDMA_CHAN_RT_SBCNT_REG, val);
> + udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
> + }
> +}
> +
> static void udma_reset_counters(struct udma_chan *uc)
> {
> u32 val;
> @@ -790,8 +804,6 @@ static void udma_reset_counters(struct udma_chan *uc)
> val = udma_rchanrt_read(uc, UDMA_CHAN_RT_PEER_BCNT_REG);
> udma_rchanrt_write(uc, UDMA_CHAN_RT_PEER_BCNT_REG, val);
> }
> -
> - uc->bcnt = 0;
> }
>
> static int udma_reset_chan(struct udma_chan *uc, bool hard)
> @@ -1115,8 +1127,8 @@ static void udma_check_tx_completion(struct work_struct *work)
> if (uc->desc) {
> struct udma_desc *d = uc->desc;
>
> - uc->bcnt += d->residue;
> udma_start(uc);
> + udma_decrement_byte_counters(uc, d->residue);
> vchan_cookie_complete(&d->vd);
> break;
> }
> @@ -1168,8 +1180,8 @@ static irqreturn_t udma_ring_irq_handler(int irq, void *data)
> vchan_cyclic_callback(&d->vd);
> } else {
> if (udma_is_desc_really_done(uc, d)) {
> - uc->bcnt += d->residue;
> udma_start(uc);
> + udma_decrement_byte_counters(uc, d->residue);
> vchan_cookie_complete(&d->vd);
> } else {
> schedule_delayed_work(&uc->tx_drain.work,
> @@ -1204,7 +1216,7 @@ static irqreturn_t udma_udma_irq_handler(int irq, void *data)
> vchan_cyclic_callback(&d->vd);
> } else {
> /* TODO: figure out the real amount of data */
> - uc->bcnt += d->residue;
> + udma_decrement_byte_counters(uc, d->residue);
> udma_start(uc);
> vchan_cookie_complete(&d->vd);
> }
> @@ -3809,7 +3821,6 @@ static enum dma_status udma_tx_status(struct dma_chan *chan,
> bcnt = udma_tchanrt_read(uc, UDMA_CHAN_RT_BCNT_REG);
> }
>
> - bcnt -= uc->bcnt;
> if (bcnt && !(bcnt % uc->desc->residue))
> residue = 0;
> else
> --
> 2.17.1
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-27 14:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-04 11:13 [PATCH] dma: ti: k3-udma: Reset UDMA_CHAN_RT byte counters to prevent overflow Vaishnav Achath
2022-07-09 6:20 ` Péter Ujfalusi
2022-07-27 14:46 ` Vaishnav Achath
2022-07-21 13:32 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox