Re: [PATCH net-next v3 09/12] net: stmmac: dwmac4: Allow platforms to specify some DMA/MTL offsets

Simon Horman
2023-04-01T14:59:20Z

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@redhat.com>
> ---
>
> This patch (and the prior patch) are replacements for
> https://lore.kernel.org/netdev/20230320204153.21736840@kernel.org/
> 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.