* [PATCH RFC 1/2 stable-6.1] dmaengine: at_hdmac: get next dma transfer from the right list
@ 2023-11-14 12:22 Thomas Pfaff
2023-11-24 12:53 ` Vinod Koul
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Pfaff @ 2023-11-14 12:22 UTC (permalink / raw)
To: ludovic.desroches, tudor.ambarus
Cc: dmaengine, vkoul, linux-arm-kernel, linux-kernel
From: Thomas Pfaff <tpfaff@pcs.com>
In kernel 6.1, atc_advance_work and atc_handle_error are checking for the
next dma transfer inside active list, but the descriptor is taken from the
queue instead.
Signed-off-by: Thomas Pfaff <tpfaff@pcs.com>
---
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 858bd64f1313..68c1bfbefc5c 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -490,6 +490,27 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
}
}
+/**
+ * atc_start_next - start next pending transaction if any
+ * @atchan: channel where the transaction ended
+ *
+ * Called with atchan->lock held
+ */
+static void atc_start_next(struct at_dma_chan *atchan)
+{
+ struct at_desc *desc = NULL;
+
+ if (!list_empty(&atchan->active_list))
+ desc = atc_first_active(atchan);
+ else if (!list_empty(&atchan->queue)) {
+ desc = atc_first_queued(atchan);
+ list_move_tail(&desc->desc_node, &atchan->active_list);
+ }
+
+ if (desc)
+ atc_dostart(atchan, desc);
+}
+
/**
* atc_advance_work - at the end of a transaction, move forward
* @atchan: channel where the transaction ended
@@ -513,11 +534,7 @@ static void atc_advance_work(struct at_dma_chan *atchan)
/* advance work */
spin_lock_irqsave(&atchan->lock, flags);
- if (!list_empty(&atchan->active_list)) {
- desc = atc_first_queued(atchan);
- list_move_tail(&desc->desc_node, &atchan->active_list);
- atc_dostart(atchan, desc);
- }
+ atc_start_next(atchan);
spin_unlock_irqrestore(&atchan->lock, flags);
}
@@ -529,7 +546,6 @@ static void atc_advance_work(struct at_dma_chan *atchan)
static void atc_handle_error(struct at_dma_chan *atchan)
{
struct at_desc *bad_desc;
- struct at_desc *desc;
struct at_desc *child;
unsigned long flags;
@@ -543,11 +559,7 @@ static void atc_handle_error(struct at_dma_chan *atchan)
list_del_init(&bad_desc->desc_node);
/* Try to restart the controller */
- if (!list_empty(&atchan->active_list)) {
- desc = atc_first_queued(atchan);
- list_move_tail(&desc->desc_node, &atchan->active_list);
- atc_dostart(atchan, desc);
- }
+ atc_start_next(atchan);
/*
* KERN_CRITICAL may seem harsh, but since this only happens
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH RFC 1/2 stable-6.1] dmaengine: at_hdmac: get next dma transfer from the right list
2023-11-14 12:22 [PATCH RFC 1/2 stable-6.1] dmaengine: at_hdmac: get next dma transfer from the right list Thomas Pfaff
@ 2023-11-24 12:53 ` Vinod Koul
2023-11-24 13:16 ` Thomas Pfaff
0 siblings, 1 reply; 3+ messages in thread
From: Vinod Koul @ 2023-11-24 12:53 UTC (permalink / raw)
To: Thomas Pfaff
Cc: tudor.ambarus, linux-kernel, ludovic.desroches, dmaengine,
linux-arm-kernel
On 14-11-23, 13:22, Thomas Pfaff wrote:
> From: Thomas Pfaff <tpfaff@pcs.com>
>
> In kernel 6.1, atc_advance_work and atc_handle_error are checking for the
> next dma transfer inside active list, but the descriptor is taken from the
> queue instead.
Sorry that is not how this works. Please send the patch for mainline and
add a stable tag to the patches. They will be backported to stable
kernels
Also, your patch threading is broken, they appear disjoint and not as a
series
Thanks
>
> Signed-off-by: Thomas Pfaff <tpfaff@pcs.com>
> ---
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index 858bd64f1313..68c1bfbefc5c 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -490,6 +490,27 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
> }
> }
>
> +/**
> + * atc_start_next - start next pending transaction if any
> + * @atchan: channel where the transaction ended
> + *
> + * Called with atchan->lock held
> + */
> +static void atc_start_next(struct at_dma_chan *atchan)
> +{
> + struct at_desc *desc = NULL;
> +
> + if (!list_empty(&atchan->active_list))
> + desc = atc_first_active(atchan);
> + else if (!list_empty(&atchan->queue)) {
> + desc = atc_first_queued(atchan);
> + list_move_tail(&desc->desc_node, &atchan->active_list);
> + }
> +
> + if (desc)
> + atc_dostart(atchan, desc);
> +}
> +
> /**
> * atc_advance_work - at the end of a transaction, move forward
> * @atchan: channel where the transaction ended
> @@ -513,11 +534,7 @@ static void atc_advance_work(struct at_dma_chan *atchan)
>
> /* advance work */
> spin_lock_irqsave(&atchan->lock, flags);
> - if (!list_empty(&atchan->active_list)) {
> - desc = atc_first_queued(atchan);
> - list_move_tail(&desc->desc_node, &atchan->active_list);
> - atc_dostart(atchan, desc);
> - }
> + atc_start_next(atchan);
> spin_unlock_irqrestore(&atchan->lock, flags);
> }
>
> @@ -529,7 +546,6 @@ static void atc_advance_work(struct at_dma_chan *atchan)
> static void atc_handle_error(struct at_dma_chan *atchan)
> {
> struct at_desc *bad_desc;
> - struct at_desc *desc;
> struct at_desc *child;
> unsigned long flags;
>
> @@ -543,11 +559,7 @@ static void atc_handle_error(struct at_dma_chan *atchan)
> list_del_init(&bad_desc->desc_node);
>
> /* Try to restart the controller */
> - if (!list_empty(&atchan->active_list)) {
> - desc = atc_first_queued(atchan);
> - list_move_tail(&desc->desc_node, &atchan->active_list);
> - atc_dostart(atchan, desc);
> - }
> + atc_start_next(atchan);
>
> /*
> * KERN_CRITICAL may seem harsh, but since this only happens
>
--
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH RFC 1/2 stable-6.1] dmaengine: at_hdmac: get next dma transfer from the right list
2023-11-24 12:53 ` Vinod Koul
@ 2023-11-24 13:16 ` Thomas Pfaff
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Pfaff @ 2023-11-24 13:16 UTC (permalink / raw)
To: Vinod Koul
Cc: tudor.ambarus, linux-kernel, ludovic.desroches, dmaengine,
linux-arm-kernel
Thank you for your answer.
I was marking the patches as RFC and for stable 6.1 only.
Between 6.1 and mainline, at_hdmac was ported to use virt-dma, there is
no possibility for my patches to go to mainline and being backported.
Most likely they are no longer needed in recent kernels.
Kind regards,
Thomas
On Fri, 24 Nov 2023, Vinod Koul wrote:
> On 14-11-23, 13:22, Thomas Pfaff wrote:
> > From: Thomas Pfaff <tpfaff@pcs.com>
> >
> > In kernel 6.1, atc_advance_work and atc_handle_error are checking for the
> > next dma transfer inside active list, but the descriptor is taken from the
> > queue instead.
>
> Sorry that is not how this works. Please send the patch for mainline and
> add a stable tag to the patches. They will be backported to stable
> kernels
>
> Also, your patch threading is broken, they appear disjoint and not as a
> series
>
> Thanks
>
> >
> > Signed-off-by: Thomas Pfaff <tpfaff@pcs.com>
> > ---
> > diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> > index 858bd64f1313..68c1bfbefc5c 100644
> > --- a/drivers/dma/at_hdmac.c
> > +++ b/drivers/dma/at_hdmac.c
> > @@ -490,6 +490,27 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc)
> > }
> > }
> >
> > +/**
> > + * atc_start_next - start next pending transaction if any
> > + * @atchan: channel where the transaction ended
> > + *
> > + * Called with atchan->lock held
> > + */
> > +static void atc_start_next(struct at_dma_chan *atchan)
> > +{
> > + struct at_desc *desc = NULL;
> > +
> > + if (!list_empty(&atchan->active_list))
> > + desc = atc_first_active(atchan);
> > + else if (!list_empty(&atchan->queue)) {
> > + desc = atc_first_queued(atchan);
> > + list_move_tail(&desc->desc_node, &atchan->active_list);
> > + }
> > +
> > + if (desc)
> > + atc_dostart(atchan, desc);
> > +}
> > +
> > /**
> > * atc_advance_work - at the end of a transaction, move forward
> > * @atchan: channel where the transaction ended
> > @@ -513,11 +534,7 @@ static void atc_advance_work(struct at_dma_chan *atchan)
> >
> > /* advance work */
> > spin_lock_irqsave(&atchan->lock, flags);
> > - if (!list_empty(&atchan->active_list)) {
> > - desc = atc_first_queued(atchan);
> > - list_move_tail(&desc->desc_node, &atchan->active_list);
> > - atc_dostart(atchan, desc);
> > - }
> > + atc_start_next(atchan);
> > spin_unlock_irqrestore(&atchan->lock, flags);
> > }
> >
> > @@ -529,7 +546,6 @@ static void atc_advance_work(struct at_dma_chan *atchan)
> > static void atc_handle_error(struct at_dma_chan *atchan)
> > {
> > struct at_desc *bad_desc;
> > - struct at_desc *desc;
> > struct at_desc *child;
> > unsigned long flags;
> >
> > @@ -543,11 +559,7 @@ static void atc_handle_error(struct at_dma_chan *atchan)
> > list_del_init(&bad_desc->desc_node);
> >
> > /* Try to restart the controller */
> > - if (!list_empty(&atchan->active_list)) {
> > - desc = atc_first_queued(atchan);
> > - list_move_tail(&desc->desc_node, &atchan->active_list);
> > - atc_dostart(atchan, desc);
> > - }
> > + atc_start_next(atchan);
> >
> > /*
> > * KERN_CRITICAL may seem harsh, but since this only happens
> >
>
> --
> ~Vinod
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-11-24 13:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-14 12:22 [PATCH RFC 1/2 stable-6.1] dmaengine: at_hdmac: get next dma transfer from the right list Thomas Pfaff
2023-11-24 12:53 ` Vinod Koul
2023-11-24 13:16 ` Thomas Pfaff
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox