Re: [net-next PATCH v2] net: dsa: qca8k: convert to regmap read/write API

[Andrew Lunn]

On Sat, Aug 27, 2022 at 03:48:34PM +0200, Christian Marangi wrote:
> On Sat, Aug 27, 2022 at 04:00:40PM +0200, Andrew Lunn wrote:
> > >  static struct regmap_config qca8k_regmap_config = {
> > > -	.reg_bits = 16,
> > > +	.reg_bits = 32,
> > 
> > Does this change really allow you to access more registers? 
> >
> 
> I could be confused but I think the value was wrong from the start. (the
> driver is a bit old and the regmap config struct was very wrong at
> times)
> This should declare how wide is each address right?
> 
> If this is the case then at times who declared the regmap config was
> confused by the fact that the mdio is limited to 16 bits and require
> special handling.
> 
> This is not problematic for the bits ops but is problematic for the bulk
> ops as they base the calculation on these values.
> 
> Or I could be totally wrong... Anyway without this change the wrong
> address is passed to the bulk ops so it's necessary (and for the said
> reason, the value was wrong from the start)

So please figure it out, maybe use git blame to see who added it, and
ask etc. And then submit a patch which changes just this, including
and explanation why it should be changed.
 
> > >  	.val_bits = 32,
> > >  	.reg_stride = 4,
> > >  	.max_register = 0x16ac, /* end MIB - Port6 range */
> > > -	.reg_read = qca8k_regmap_read,
> > > -	.reg_write = qca8k_regmap_write,
> > > +	.read = qca8k_bulk_read,
> > > +	.write = qca8k_bulk_write,
> > >  	.reg_update_bits = qca8k_regmap_update_bits,
> > >  	.rd_table = &qca8k_readable_table,
> > >  	.disable_locking = true, /* Locking is handled by qca8k read/write */
> > >  	.cache_type = REGCACHE_NONE, /* Explicitly disable CACHE */
> > > +	.max_raw_read = 16, /* mgmt eth can read/write up to 4 bytes at times */
> > > +	.max_raw_write = 16,
> > 
> > I think the word 'bytes' in the comment is wrong. I assume you can
> > access 4 registers, each register is one 32-bit work in size.
> > 
> 
> Yes you are right. Any suggestion on how to improve?

s/bytes/words/

	Andrew