Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
Hi Florian, On 24/09/2018 21:17, Florian Fainelli wrote: > On 09/24/2018 12:04 PM, Corentin Labbe wrote: >> This patch convert meson stmmac glue driver to use all xxxsetbits32 >> functions. >> >> Signed-off-by: Corentin Labbe >> --- >> .../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 >> #include >> #include >> +#include >> >> #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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
On 09/24/2018 12:04 PM, Corentin Labbe wrote: > This patch convert meson stmmac glue driver to use all xxxsetbits32 functions. > > Signed-off-by: Corentin Labbe > --- > .../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 > #include > #include > +#include > > #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. -- Florian ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
On 24/09/2018 21:04, Corentin Labbe wrote: > This patch convert meson stmmac glue driver to use all xxxsetbits32 functions. > > Signed-off-by: Corentin Labbe > --- > .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 56 > +- > 1 file changed, 22 insertions(+), 34 deletions(-) > [...] Reviewed-by: Neil Armstrong ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel