Re: [PATCH net] net: macb: Move macb_{alloc,free}_tieoff() out of CONFIG_OF block

["Théo Lebrun" <theo.lebrun@bootlin.com>]

Hello Nathan,

On Wed Aug 19, 2026 at 3:14 AM CEST, Nathan Chancellor wrote:
> Commit 5262eab9462a ("net: macb: allocate tieoff descriptor once across
> device lifetime") moved macb_alloc_tieoff() and macb_free_tieoff() into
> a CONFIG_OF block, breaking the build when it is disabled:

That commit message is somewhat off: the commit created the two
functions, it didn't move them around. But indeed I was wrong in the
location where I created them (argh I hate #if/ifdef blocks, especially
long ones).

Can reproduce using my host toolchain easily:

   ⟩ unset ARCH CROSS_COMPILE
   ⟩ make mrproper
   ⟩ make defconfig
   ⟩ ./scripts/config -e COMMON_CLK -e MACB
   ⟩ make olddefconfig
   ⟩ make drivers/net/ethernet/cadence/
   ⟩ echo $?
   2

Here is an alternative fix proposal: let's drop the #ifdef. It will
avoid any future error. Almost all drivers have no #ifdef as such:

⟩ git grep -l MODULE_DEVICE_TABLE drivers/ | wc -l
7458
⟩ git grep -l MODULE_DEVICE_TABLE drivers/ | xargs git grep -l CONFIG_OF | wc -l
391

Yes, I know this means the match table land in the module in !OF case.
Almost everyone on the ML I've seen seem to consider that it's trivial.
Apparently it's even useful because some ACPI can match on compatible,
somehow.

Opinions?

>   drivers/net/ethernet/cadence/macb_main.c: In function 'macb_probe':
>   drivers/net/ethernet/cadence/macb_main.c:5951:15: error: implicit declaration of function 'macb_alloc_tieoff' [-Wimplicit-function-declaration]
>    5951 |         err = macb_alloc_tieoff(bp);
>         |               ^~~~~~~~~~~~~~~~~
>   drivers/net/ethernet/cadence/macb_main.c:5973:9: error: implicit declaration of function 'macb_free_tieoff' [-Wimplicit-function-declaration]
>    5973 |         macb_free_tieoff(bp);
>         |         ^~~~~~~~~~~~~~~~
>
> Moving macb_alloc_tieoff() from its original positive does not appear to
> be necessary, so move it and macb_free_tieoff() back out of the
> CONFIG_OF block to clear up the error.

Apart from the typo already reported, I'm guessing the original position
is the one from previous macb_init_tieoff() functions. Yes I don't see
a reason for the move.

> Fixes: 5262eab9462a ("net: macb: allocate tieoff descriptor once across device lifetime")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> This does not apply to net currently but I assume net will be fast
> forwarded when net-next-7.3 is merged.
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 64 ++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 1476bce77f34..96a7e7777e62 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2776,6 +2776,38 @@ static int macb_alloc(struct macb *bp)
>  	return -ENOMEM;
>  }
>  
> +static int macb_alloc_tieoff(struct macb *bp)
> +{
> +	/* Tieoff is a workaround in case HW cannot disable queues, for PM. */
> +	if (bp->caps & MACB_CAPS_QUEUE_DISABLE)
> +		return 0;
> +
> +	bp->rx_ring_tieoff = dma_alloc_coherent(&bp->pdev->dev,
> +						macb_dma_desc_get_size(bp),
> +						&bp->rx_ring_tieoff_dma,
> +						GFP_KERNEL);
> +	if (!bp->rx_ring_tieoff)
> +		return -ENOMEM;
> +
> +	macb_set_addr(bp, bp->rx_ring_tieoff,
> +		      MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED));
> +
> +	bp->rx_ring_tieoff->ctrl = 0;
> +
> +	return 0;
> +}
> +
> +static void macb_free_tieoff(struct macb *bp)
> +{
> +	if (!bp->rx_ring_tieoff)
> +		return;
> +
> +	dma_free_coherent(&bp->pdev->dev, macb_dma_desc_get_size(bp),
> +			  bp->rx_ring_tieoff,
> +			  bp->rx_ring_tieoff_dma);
> +	bp->rx_ring_tieoff = NULL;
> +}
> +
>  static void gem_init_rx_ring(struct macb_queue *queue)
>  {
>  	queue->rx_tail = 0;
> @@ -5507,38 +5539,6 @@ static int eyeq5_init(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int macb_alloc_tieoff(struct macb *bp)
> -{
> -	/* Tieoff is a workaround in case HW cannot disable queues, for PM. */
> -	if (bp->caps & MACB_CAPS_QUEUE_DISABLE)
> -		return 0;
> -
> -	bp->rx_ring_tieoff = dma_alloc_coherent(&bp->pdev->dev,
> -						macb_dma_desc_get_size(bp),
> -						&bp->rx_ring_tieoff_dma,
> -						GFP_KERNEL);
> -	if (!bp->rx_ring_tieoff)
> -		return -ENOMEM;
> -
> -	macb_set_addr(bp, bp->rx_ring_tieoff,
> -		      MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED));
> -
> -	bp->rx_ring_tieoff->ctrl = 0;
> -
> -	return 0;
> -}
> -
> -static void macb_free_tieoff(struct macb *bp)
> -{
> -	if (!bp->rx_ring_tieoff)
> -		return;
> -
> -	dma_free_coherent(&bp->pdev->dev, macb_dma_desc_get_size(bp),
> -			  bp->rx_ring_tieoff,
> -			  bp->rx_ring_tieoff_dma);
> -	bp->rx_ring_tieoff = NULL;
> -}
> -
>  static const struct macb_usrio_config mpfs_usrio = {
>  	.tsu_source = 0,
>  };
>
> ---
> base-commit: 61eb236c41c2a4717015dff18016a75a5eb90052
> change-id: 20260818-macb-fix-no-of-build-eee72c7cf20d
>
> Best regards,
> --  
> Cheers,
> Nathan



Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com