[PATCH 1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean

[PATCH 1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean

[Holger Brunck]

Unmapping of uf_regs is done from ucc_fast_free and doesn't need to be
done explicitly. If already unmapped ucc_fast_free will crash.

Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 3bd57527b1be..8155e92af14e 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -773,11 +773,6 @@ static void uhdlc_memclean(struct ucc_hdlc_private *priv)
 	kfree(priv->tx_skbuff);
 	priv->tx_skbuff = NULL;
 
-	if (priv->uf_regs) {
-		iounmap(priv->uf_regs);
-		priv->uf_regs = NULL;
-	}
-
 	if (priv->uccf) {
 		ucc_fast_free(priv->uccf);
 		priv->uccf = NULL;
-- 
2.52.0.120.gb31ab939fe

[PATCH 2/2] net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove

[Holger Brunck]

If the driver is used in a non tdm mode priv->utdm is a NULL pointer.
Therefore we need to check this pointer first before checking si_regs.

Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
---
 drivers/net/wan/fsl_ucc_hdlc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 8155e92af14e..15bfb78381d4 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1250,12 +1250,12 @@ static void ucc_hdlc_remove(struct platform_device *pdev)
 
 	uhdlc_memclean(priv);
 
-	if (priv->utdm->si_regs) {
+	if (priv->utdm && priv->utdm->si_regs) {
 		iounmap(priv->utdm->si_regs);
 		priv->utdm->si_regs = NULL;
 	}
 
-	if (priv->utdm->siram) {
+	if (priv->utdm && priv->utdm->siram) {
 		iounmap(priv->utdm->siram);
 		priv->utdm->siram = NULL;
 	}
-- 
2.52.0.120.gb31ab939fe

Re: [PATCH 2/2] net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove

[Simon Horman]

On Wed, Apr 29, 2026 at 01:42:08PM +0200, Holger Brunck wrote:
> If the driver is used in a non tdm mode priv->utdm is a NULL pointer.
> Therefore we need to check this pointer first before checking si_regs.
> 
> Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>

Hi Holger,

I have the same feedback as for patch 1/2:

As a fix for code present in net, this warrants a fixes tag
that cites the commit that introduced this bug.
Perhaps this one is appropriate.

Fixes: ca20e191eed0 ("net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove")

Also, as a fix for net it would be best to explicitly target that branch,
like this:

Subject: [PATCH net 2/2] ...

I don't think it's necessary to repost to just to address either of the
above.  But for reference more information on the Netdev development
process can be found here:

https://docs.kernel.org/process/maintainer-netdev.html

The above not withstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

FTR, an AI generated review of this patch is available on sashiko.dev.
All the issues flagged there seem to be pre-existing and I do not
believe they should block progress of this patch. But you may want to
look into then in the context of follow-up.

...

Re: [PATCH 1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean

[Simon Horman]

On Wed, Apr 29, 2026 at 01:42:07PM +0200, Holger Brunck wrote:
> Unmapping of uf_regs is done from ucc_fast_free and doesn't need to be
> done explicitly. If already unmapped ucc_fast_free will crash.
> 
> Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>

As a fix for code present in net, this warrants a fixes tag
that cites the commit that introduced this bug.
Perhaps this one is appropriate.

Fixes: ca20e191eed0 ("net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove")

Also, as a fix for net it would be best to explicitly target that branch,
like this:

Subject: [PATCH net 1/2] ...

I don't think it's necessary to repost to just to address either of the
above.  But for reference more information on the Netdev development
process can be found here:

https://docs.kernel.org/process/maintainer-netdev.html

The above not withstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

FTR, an AI generated review of this patch is available on sashiko.dev.
All the issues flagged there seem to be pre-existing and I do not
believe they should block progress of this patch. But you may want to
look into then in the context of follow-up.

...

Re: [PATCH 1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean

[Jakub Kicinski]

On Fri, 1 May 2026 13:27:56 +0100 Simon Horman wrote:
> Fixes: ca20e191eed0 ("net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove")

This appears to be a SHA of the second patch in some local tree.
Supposedly the correct tag is:

Fixes: c19b6d246a35 ("drivers/net: support hdlc function for QE-UCC")

RE: [PATCH 1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean

[Holger Brunck]

> 
> On Wed, Apr 29, 2026 at 01:42:07PM +0200, Holger Brunck wrote:
> > Unmapping of uf_regs is done from ucc_fast_free and doesn't need to be
> > done explicitly. If already unmapped ucc_fast_free will crash.
> >
> > Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
> 
> As a fix for code present in net, this warrants a fixes tag that cites the commit
> that introduced this bug.
> Perhaps this one is appropriate.
> 
> Fixes: ca20e191eed0 ("net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove")
> 
> Also, as a fix for net it would be best to explicitly target that branch, like this:
> 
> Subject: [PATCH net 1/2] ...
> 
> I don't think it's necessary to repost to just to address either of the above.  But for
> reference more information on the Netdev development process can be found
> here:
> 

ok I will take care of this next time.

> 
> The above not withstanding, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> FTR, an AI generated review of this patch is available on sashiko.dev.
> All the issues flagged there seem to be pre-existing and I do not believe they
> should block progress of this patch. But you may want to look into then in the
> context of follow-up.
>

Thanks, I will have a look at this.

Best regards
Holger

Re: [PATCH 1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 29 Apr 2026 13:42:07 +0200 you wrote:
> Unmapping of uf_regs is done from ucc_fast_free and doesn't need to be
> done explicitly. If already unmapped ucc_fast_free will crash.
> 
> Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>
> ---
>  drivers/net/wan/fsl_ucc_hdlc.c | 5 -----
>  1 file changed, 5 deletions(-)

Here is the summary with links:
  - [1/2] net: wan: fsl_ucc_hdlc: fix uhdlc_memclean
    https://git.kernel.org/netdev/net/c/1a57efe250a1
  - [2/2] net: wan: fsl_ucc_hdlc: fix ucc_hdlc_remove
    https://git.kernel.org/netdev/net/c/851bba8068d1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html