Re: [PATCH v2 net-next 11/15] net: ethernet: mtk_eth_soc: introduce device register map

From: Jakub Kicinski <kuba@kernel.org>
To: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: netdev@vger.kernel.org, nbd@nbd.name, john@phrozen.org,
	sean.wang@mediatek.com, Mark-MC.Lee@mediatek.com,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	Sam.Shih@mediatek.com, linux-mediatek@lists.infradead.org,
	devicetree@vger.kernel.org, robh@kernel.org,
	lorenzo.bianconi@redhat.com
Subject: Re: [PATCH v2 net-next 11/15] net: ethernet: mtk_eth_soc: introduce device register map
Date: Wed, 18 May 2022 08:44:31 -0700	[thread overview]
Message-ID: <20220518084431.66aa1737@kernel.org> (raw)
In-Reply-To: <YoTA+5gLC4zhoQ0F@lore-desk>

On Wed, 18 May 2022 11:48:43 +0200 Lorenzo Bianconi wrote:
> > On Mon, 16 May 2022 18:06:38 +0200 Lorenzo Bianconi wrote:  
> > >  /* PDMA RX Base Pointer Register */
> > > -#define MTK_PRX_BASE_PTR0	0x900
> > > +#define MTK_PRX_BASE_PTR0	(eth->soc->reg_map[MTK_PDMA_BASE] + 0x100)
> > >  #define MTK_PRX_BASE_PTR_CFG(x)	(MTK_PRX_BASE_PTR0 + (x * 0x10))  
> > 
> > Implicit macro arguments are really unpleasant for people doing
> > tree-wide changes or otherwise unfamiliar with the driver.
> > 
> > Nothing we can do to avoid this?  
> 
> I used this approach in order to have just few changes in the codebase. I guess the best
> option would be to explicitly add eth parameter to the register macros, what do you think?

I don't think there's a best known practice, you'll have to exercise
your judgment. Taking a look at a random example of MTK_PDMA_INT_STATUS.
Looks like that one is already assigned to eth->tx_int_status_reg.
Maybe that can be generalized? Personally I'd forgo the macros
completely and just use eth->soc->register_name in the code.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek