linux-kernel - Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Sep 2018 09:53:22 +0200
From:   Neil Armstrong <narmstrong@...libre.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Corentin Labbe <clabbe@...libre.com>, Gilles.Muller@...6.fr,
        Julia.Lawall@...6.fr, agust@...x.de, airlied@...ux.ie,
        alexandre.torgue@...com, alistair@...ple.id.au,
        benh@...nel.crashing.org, carlo@...one.org, davem@...emloft.net,
        galak@...nel.crashing.org, joabreu@...opsys.com,
        khilman@...libre.com, maxime.ripard@...tlin.com,
        michal.lkml@...kovi.net, mpe@...erman.id.au,
        mporter@...nel.crashing.org, nicolas.palix@...g.fr,
        oss@...error.net, paulus@...ba.org, peppe.cavallaro@...com,
        tj@...nel.org, vitb@...nel.crashing.org, wens@...e.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-ide@...r.kernel.org,
        linux-amlogic@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
        cocci@...teme.lip6.fr, linux-arm-kernel@...ts.infradead.org
Subject: 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 <clabbe@...libre.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