[net-next,1/2] net: stmmac: fix unsafe MTL DMA macro

Message ID 20221021114711.1610797-1-junxiao.chang@intel.com
State New
Headers
Series [net-next,1/2] net: stmmac: fix unsafe MTL DMA macro |

Commit Message

Chang, Junxiao Oct. 21, 2022, 11:47 a.m. UTC
  Macro like "#define abc(x) (x, x)" is unsafe which might introduce
side effects. Each MTL RxQ DMA channel mask is 4 bits, so using
(0xf << chan) instead of GENMASK(x + 3, x) to avoid unsafe macro.

Fixes: d43042f4da3e ("net: stmmac: mapping mtl rx to dma channel")
Signed-off-by: Junxiao Chang <junxiao.chang@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Jakub Kicinski Oct. 25, 2022, 4 a.m. UTC | #1
On Fri, 21 Oct 2022 19:47:10 +0800 Junxiao Chang wrote:
> Macro like "#define abc(x) (x, x)" is unsafe which might introduce
> side effects. Each MTL RxQ DMA channel mask is 4 bits, so using
> (0xf << chan) instead of GENMASK(x + 3, x) to avoid unsafe macro.
> 
> Fixes: d43042f4da3e ("net: stmmac: mapping mtl rx to dma channel")

You need to point out an existing usage where this is causing problems,
otherwise this is not a fix.

And squash the two patches together, it's going to be easier to review.
  
Chang, Junxiao Oct. 25, 2022, 4:58 a.m. UTC | #2
Thank you for reviewing it. There is no real issue if this macro only be used in stmmac driver and parameter is not function or "i++".

I will squash these two patches.

Regards,
Junxiao

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org> 
Sent: Tuesday, October 25, 2022 12:00 PM
To: Chang, Junxiao <junxiao.chang@intel.com>
Cc: peppe.cavallaro@st.com; alexandre.torgue@foss.st.com; joabreu@synopsys.com; davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; mcoquelin.stm32@gmail.com; Joao.Pinto@synopsys.com; netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 1/2] net: stmmac: fix unsafe MTL DMA macro

On Fri, 21 Oct 2022 19:47:10 +0800 Junxiao Chang wrote:
> Macro like "#define abc(x) (x, x)" is unsafe which might introduce 
> side effects. Each MTL RxQ DMA channel mask is 4 bits, so using (0xf 
> << chan) instead of GENMASK(x + 3, x) to avoid unsafe macro.
> 
> Fixes: d43042f4da3e ("net: stmmac: mapping mtl rx to dma channel")

You need to point out an existing usage where this is causing problems, otherwise this is not a fix.

And squash the two patches together, it's going to be easier to review.
  

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 71dad409f78b0..3c1490408a1c3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -333,7 +333,7 @@  enum power_event {
 #define MTL_RXQ_DMA_MAP1		0x00000c34 /* queue 4 to 7 */
 #define MTL_RXQ_DMA_Q04MDMACH_MASK	GENMASK(3, 0)
 #define MTL_RXQ_DMA_Q04MDMACH(x)	((x) << 0)
-#define MTL_RXQ_DMA_QXMDMACH_MASK(x)	GENMASK(11 + (8 * ((x) - 1)), 8 * (x))
+#define MTL_RXQ_DMA_QXMDMACH_MASK(x)	(0xf << 8 * (x))
 #define MTL_RXQ_DMA_QXMDMACH(chan, q)	((chan) << (8 * (q)))
 
 #define MTL_CHAN_BASE_ADDR		0x00000d00