Re: [PATCH 1/2] iio: adc: Add driver for Marvell 88PM886 PMIC ADC

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 30 Aug 2025 18:05:06 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Aug 30, 2025 at 4:07 PM Duje Mihanović <duje@dujemihanovic.xyz> wrote:
> > On Saturday, 30 August 2025 06:41:58 Central European Summer Time Andy
> > Shevchenko wrote:  
> > > On Sat, Aug 30, 2025 at 7:37 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:  
> > > > On Fri, Aug 29, 2025 at 2:41 AM David Lechner <dlechner@baylibre.com>  
> > wrote:  
> > > > > On 8/28/25 5:17 PM, Duje Mihanović wrote:  
> 
> ...
> 
> > > > > > +     ret = regmap_bulk_read(*map, regs[chan], buf, 2);  
> > >
> > > On top, please drop a double pointer and use map directly. That's
> > > already a pointer, what's the issue with it to begin with?  
> >
> > struct regmap is only defined in a regmap-internal header, so it has to
> > be a double pointer or a struct containing a regmap pointer. I went
> > with David's advice and created this struct.  
> 
> I might have missed something... So, the root of this is how we
> allocate memory for the data structure and what we keep in the priv
> member there. Indeed, it keeps the pointer to the field in the
> allocated memory, so if we allocate a memory just to keep one pointer
> it should be doubled (independently on the possibility to access the
> data type we are using to keep in priv).
> 

To avoid confusion of layers of pointers I'd spin a structure for iio_priv

struct pm886_data {
//nothing else for now.
	struct regmap *map;
};

Then there will be no double pointers visible and this will looks like
most other drivers where this is at least one other bit of state to store.

Very high chance something else will need to go in there at some point anyway!

Jonathan