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

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

Posted by Nathan Chancellor 1 month, 1 week ago

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:

  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.

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

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

Posted by Théo Lebrun 1 month, 1 week ago

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

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

Posted by Nathan Chancellor 1 month, 1 week ago

Hi Théo,

On Wed, Aug 19, 2026 at 08:06:48PM +0200, Théo Lebrun wrote:
> 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

Oh correct, I misread macb_init_tieoff() as macb_alloc_tieoff() so
considered it a "move" but "placed" would definitely be more accurate.

> location where I created them (argh I hate #if/ifdef blocks, especially
> long ones).

Yeah, I had to grep for the pairs because this one was so big.

> 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

Yup, same exact reproducer I used.

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

I am always in favor of dropping #ifdefs to make it easier to avoid
compile issues like this. If that is the route we want to go, please
feel free to send a patch for it (or I can if you prefer).

-- 
Cheers,
Nathan

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

Posted by Théo Lebrun 1 month, 1 week ago

On Wed Aug 19, 2026 at 8:54 PM CEST, Nathan Chancellor wrote:
> On Wed, Aug 19, 2026 at 08:06:48PM +0200, Théo Lebrun wrote:
>> 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
>
> Oh correct, I misread macb_init_tieoff() as macb_alloc_tieoff() so
> considered it a "move" but "placed" would definitely be more accurate.
>
>> location where I created them (argh I hate #if/ifdef blocks, especially
>> long ones).
>
> Yeah, I had to grep for the pairs because this one was so big.
>
>> 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
>
> Yup, same exact reproducer I used.
>
>> 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?
>
> I am always in favor of dropping #ifdefs to make it easier to avoid
> compile issues like this. If that is the route we want to go, please
> feel free to send a patch for it (or I can if you prefer).

Done. Just sent now and noticed I forgot CCing you, sorry!
https://lore.kernel.org/netdev/20260820-macb-fix-x86-v1-1-b2e7c902104e@bootlin.com/

Thanks,

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

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

Posted by Nicolai Buchwitz 1 month, 1 week ago

Hi Nathan

On 19.8.2026 03:14, 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:
> 
>   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.

Sorry about that, I reviewed the original and missed it. Thanks for 
fixing it!

> [...]

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

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

Posted by luoxuanqiang 1 month, 1 week ago

在 2026/8/19 09:14, Nathan Chancellor 写道:
> 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:
>
>    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

nit: s/from its original positive/from its original position/

> be necessary, so move it and macb_free_tieoff() back out of the
> CONFIG_OF block to clear up the error.
>
> Fixes: 5262eab9462a ("net: macb: allocate tieoff descriptor once across device lifetime")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Tested-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Thanks,
Xuanqiang

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

Posted by Nathan Chancellor 1 month, 1 week ago

On Wed, Aug 19, 2026 at 03:10:32PM +0800, luoxuanqiang wrote:
> 在 2026/8/19 09:14, Nathan Chancellor 写道:
> > 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:
> > 
> >    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
> 
> nit: s/from its original positive/from its original position/

Whoops, good catch! I can send a v2 later unless one of the maintainers
don't mind fixing that during application time. Thanks for taking a
look.

-- 
Cheers,
Nathan