On Sat, Apr 01, 2023 at 04:58:59PM +0200, Simon Horman wrote: > On Fri, Mar 31, 2023 at 04:45:46PM -0500, Andrew Halaney wrote: > > Some platforms have dwmac4 implementations that have a different > > address space layout than the default, resulting in the need to define > > their own DMA/MTL offsets. > > > > Extend the functions to allow a platform driver to indicate what its > > addresses are, overriding the defaults. > > > > Signed-off-by: Andrew Halaney <ahalaney@xxxxxxxxxx> > > --- > > > > This patch (and the prior patch) are replacements for > > https://lore.kernel.org/netdev/20230320204153.21736840@xxxxxxxxxx/ > > as was requested. Hopefully I was understanding the intent correctly :) > > > > I'm pretty sure further refinement will be requested for this one, but > > it is the best I could come up with myself! Specifically some of the > > naming, dealing with spacing in some older spots of dwmac4, > > where the addresses should live in the structure hierarchy, etc are > > things I would not be surprised to have to rework if this is still > > preferred over the wrapper approach. > > > > Changes since v2: > > * New, replacing old wrapper approach > > > > drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 91 ++++++++-- > > .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 36 ++-- > > .../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 157 ++++++++++-------- > > .../net/ethernet/stmicro/stmmac/dwmac4_dma.h | 51 +++--- > > .../net/ethernet/stmicro/stmmac/dwmac4_lib.c | 67 +++++--- > > include/linux/stmmac.h | 19 +++ > > 6 files changed, 279 insertions(+), 142 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h > > index ccd49346d3b3..a0c0ee1dc13f 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h > > @@ -336,14 +336,23 @@ enum power_event { > > > > #define MTL_CHAN_BASE_ADDR 0x00000d00 > > #define MTL_CHAN_BASE_OFFSET 0x40 > > -#define MTL_CHANX_BASE_ADDR(x) (MTL_CHAN_BASE_ADDR + \ > > - (x * MTL_CHAN_BASE_OFFSET)) > > - > > -#define MTL_CHAN_TX_OP_MODE(x) MTL_CHANX_BASE_ADDR(x) > > -#define MTL_CHAN_TX_DEBUG(x) (MTL_CHANX_BASE_ADDR(x) + 0x8) > > -#define MTL_CHAN_INT_CTRL(x) (MTL_CHANX_BASE_ADDR(x) + 0x2c) > > -#define MTL_CHAN_RX_OP_MODE(x) (MTL_CHANX_BASE_ADDR(x) + 0x30) > > -#define MTL_CHAN_RX_DEBUG(x) (MTL_CHANX_BASE_ADDR(x) + 0x38) > > +#define MTL_CHANX_BASE_ADDR(addrs, x) \ > > +({ \ > > + const struct dwmac4_addrs *__addrs = addrs; \ > > + const u32 __x = x; \ > > + u32 __addr; \ > > + if (__addrs) \ > > + __addr = __addrs->mtl_chan + (__x * __addrs->mtl_chan_offset); \ > > + else \ > > + __addr = MTL_CHAN_BASE_ADDR + (__x * MTL_CHAN_BASE_OFFSET); \ > > + __addr; \ > > +}) > > Could this and similar macros added by this patch be functions? > From my pov a benefit would be slightly more type safety. > And as a bonus there wouldn't be any need to handle aliasing of input. > Sure, to be honest I'll be much more comfortable coding that up anyways. I don't do a ton of macro programming and had to refamiliarize myself of the pitfalls that comes with it when doing this. Thanks, Andrew
