Received-SPF: pass (google.com: domain of linux-kernel+bounces-207503-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sun, 9 Jun 2024 22:33:56 +0200
From: Olivier Dautricourt <olivierdautricourt@gmail.com>
To: Markus Elfring <Markus.Elfring@web.de>
Cc: dmaengine@vger.kernel.org, Stefan Roese <sr@denx.de>,
	Vinod Koul <vkoul@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	Eric Schwarz <eas@sw-optimization.com>
Subject: Re: [PATCH v2 2/3] dmaengine: altera-msgdma: cleanup after
 completing all descriptors
Message-ID: <ZmYRtMlJheCp7i-E@olivier-manjaro>
References: <20240608213216.25087-2-olivierdautricourt@gmail.com>
 <0e439d73-aff5-4d56-9a3c-b29867132db1@web.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0e439d73-aff5-4d56-9a3c-b29867132db1@web.de>

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