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 <clabbe@xxxxxxxxxxxx>
>> ---
>> .../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
