On Sun, Jun 09, 2024 at 07:20:49PM +0200, Markus Elfring wrote:
> …
> > This fixes a Sparse warning because we first take the lock in
> > msgdma_tasklet.
> …
>
> Can the tag “Fixes” become relevant for the proposed change?
I can add a Fixes tag, it will target only the first commit introducing
this driver.
>
>
> …
> > +++ b/drivers/dma/altera-msgdma.c
> > @@ -585,6 +585,8 @@ static void msgdma_chan_desc_cleanup(struct msgdma_device *mdev)
> > struct msgdma_sw_desc *desc, *next;
> > unsigned long irqflags;
> >
> > + spin_lock_irqsave(&mdev->lock, irqflags);
> > +
> > list_for_each_entry_safe(desc, next, &mdev->done_list, node) {
> > struct dmaengine_desc_callback cb;
> >
> > @@ -600,6 +602,8 @@ static void msgdma_chan_desc_cleanup(struct msgdma_device *mdev)
> > /* Run any dependencies, then free the descriptor */
> > msgdma_free_descriptor(mdev, desc);
> > }
> > +
> > + spin_unlock_irqrestore(&mdev->lock, irqflags);
> > }
> …
>
> Would you become interested to apply the guard “spinlock_irqsave”?
> https://elixir.bootlin.com/linux/v6.10-rc2/source/include/linux/spinlock.h#L574
I could but arent these type of things to be integrated in a more global patch serie
targeting (let say) all drivers in one subsystem ?
Currently it seems only one driver uses a guard in the dmaengine subsystem.
Kr,
Olivier
