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

[Andrew Lunn <andrew@lunn.ch>]

>  static struct regmap_config qca8k_regmap_config = {
> -	.reg_bits = 16,
> +	.reg_bits = 32,

Does this change really allow you to access more registers? 

>  	.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.

>  static int qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
>  {
> -	u32 reg[3];
> +	u32 reg[QCA8K_ATU_TABLE_SIZE];
>  	int ret;
>  
>  	/* load the ARL table into an array */
> -	ret = qca8k_bulk_read(priv, QCA8K_REG_ATU_DATA0, reg, sizeof(reg));
> +	ret = regmap_bulk_read(priv->regmap, QCA8K_REG_ATU_DATA0, reg,
> +			       QCA8K_ATU_TABLE_SIZE);
>  	if (ret)
>  		return ret;

Please split the 3 -> QCA8K_ATU_TABLE_SIZE change out into a patch of
its own.

Ideally you want lots of small, obviously correct patches. A change
which replaces 3 for QCA8K_ATU_TABLE_SIZE should be small and
obviously correct, meaning it is quick and easy to review, and makes
the more complex to review change smaller and also simpler to review.

    Andrew