Re: [PATCH] iio: amplifiers: ada4250: use DMA-safe memory for regmap_bulk_read()

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 21 Apr 2025 14:56:06 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 21 Apr 2025 14:00:30 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Mon, 2025-04-21 at 11:57 +0100, Jonathan Cameron wrote:  
> > > On Sat, 19 Apr 2025 13:07:25 -0500
> > > David Lechner <dlechner@baylibre.com> wrote:
> > >     
> > > > On 4/19/25 10:03 AM, Andy Shevchenko wrote:    
> > > > > On Fri, Apr 18, 2025 at 02:09:04PM -0500, David Lechner wrote:      
> > > > > > On 4/18/25 1:58 PM, Andy Shevchenko wrote:      
> > > > > > > пʼятниця, 18 квітня 2025 р. David Lechner
> > > > > > > <dlechner@baylibre.com <mailto:dlechner@baylibre.com>> пише:
> > > > > > > 
> > > > > > >     Use DMA-safe memory instead of stack-allocated memory for the call
> > > > > > > to
> > > > > > >     regmap_bulk_read() in the ada4250_init() function as this could be
> > > > > > > used
> > > > > > >     directly by a SPI controller.
> > > > > > > 
> > > > > > > Sorry, but can you elaborate more on this? If driver doesn’t override
> > > > > > > the
> > > > > > > callbacks the regmap SPI uses spi_write_then_read() which is supposed
> > > > > > > to be
> > > > > > > dma safe.       
> > > > > > 
> > > > > > Ah, I didn't dig that far down. Will send a new patch that just cleans
> > > > > > up the
> > > > > > unnecessary alignment and unaligned call.      
> > > > > 
> > > > > But do you have a real life issue with that? Coincidentally we discussed
> > > > > similar case in another driver with colleague of mine, and he insists that
> > > > > it
> > > > > might be still broken (somewhere).
> > > > >       
> > > > 
> > > > Some drivers that use regmap with SPI implement their own read and write
> > > > callbacks if the default one doesn't work, so maybe something like that?
> > > >     
> > > 
> > > I asked Mark about this long ago. Maybe the guidance has changed, but the
> > > request
> > > if my memory is correct was to treat bulk regmap accesses with the same care
> > > wrt to
> > > DMA safety that we treat direct use of the underlying bus accessors.
> > > (unfortunately I have no idea what thread this was in!)
> > > 
> > > The reasoning I think was that it was an implementation detail whether
> > > regmap copied the data or not for bulk accesses.  Back then I chased through
> > > the SPI case and couldn't find a path that didn't bounce the data (though some
> > > paths could have been made zero copy).
> > >     
> > 
> > Looking on the implementation, indeed it looks like that everything should be
> > fine... However, IMO, David's reasoning about potential drivers using their own
> > regmap_bus (or just overriding read and write) is perhaps justification enough
> > for doing it.  
> 
> That's in a specific driver which is the same one allocating the buffer.
> Doing it defensively in other drivers is probably a step too far if we don't
> still want to do it everwhere regmap and SPI are combined.

Given we don't believe this to be an active bug and no reply from Mark
on any change on regmap, I'm going to queue this up the slow way and
see if anyone objects.  Queued up for 6.17.

I dropped the fixes tag and added a bit of commentary on why.

Jonathan

> 
> Jonathan
> 
> 
> > 
> > - Nuno Sá 
> >   
> > > Anyhow, +CC Mark as maybe I remembered that wrong or his view has
> > > changed!
> > > 
> > > Jonathan    
> 
>