Hi Florian,

On 24/09/2018 21:17, Florian Fainelli wrote:

On 09/24/2018 12:04 PM, Corentin Labbe wrote:

quoted

This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 56 +++++++++-------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c5979569fd60..abcf65588576 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c

@@ -23,6 +23,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
 #include <linux/stmmac.h>
+#include <linux/setbits.h>
 
 #include "stmmac_platform.h"
 

@@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
 	struct clk_gate		rgmii_tx_en;
 };
 
-static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
-				    u32 mask, u32 value)
-{
-	u32 data;
-
-	data = readl(dwmac->regs + reg);
-	data &= ~mask;
-	data |= (value & mask);
-
-	writel(data, dwmac->regs + reg);
-}

Why not make mseon8b_dwmac_mask_bits() a wrapper around
clrsetbits_le32() whose purpose is only to dereference dwmac->regs and
pass it to clrsetbits_le32()? That would be far less changes to review
and audit for correctness, same goes with every other patch in this
series touching the meson drivers.

Personally, I'll prefer dropping my custom writel_bits_relaxed() with something
more future proof (I also use it in spi-meson-spicc and ao-cec),
and I think the same for dwmac-meson8b.c

Neil