Re: [PATCH 17/17] spi: dw: Use regset32 DebugFS method to create a registers file

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Wed, May 13, 2020 at 03:44:22PM +0300, Serge Semin wrote:
> On Fri, May 08, 2020 at 10:30:27PM +0300, Andy Shevchenko wrote:
> > On Fri, May 08, 2020 at 04:29:42PM +0300, Serge Semin wrote:
> > > DebugFS kernel interface provides a dedicated method to create the
> > > registers dump file. Use it instead of creating a generic DebugFS
> > > file with manually written read callback function.

> > > +#define DW_SPI_DBGFS_REG(_name, _off)	\
> > > +{					\
> > > +	.name = _name,			\
> > > +	.offset = _off			\
> > 
> > Leave comma here.
> 
> don't see a point.

It will help in case if this getting extended. Also slightly better to
distinguish between terminator type of members vs. data structures.

> > >  }

> > > +	struct debugfs_regset32 regset;
> > 
> > I'm wondering why we need it here and not simple on the stack?
> 
> Please see the way the DebugFS regset work. A prompt: how does the DebugFS
> core get to know what is a base address of the registers? =)

If they have a member in the struct which passed thru private pointer of inode.
But I see your point.

-- 
With Best Regards,
Andy Shevchenko