Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <61f00e3c-06ea-20a6-4b76-2bddf986f074@gmail.com>
Date:   Tue, 8 Feb 2022 18:49:28 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v3 2/2] ath9k: htc: clean up *STAT_* macros
Content-Language: en-US
To:     =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@toke.dk>,
        Jeff Johnson <quic_jjohnson@quicinc.com>,
        ath9k-devel@qca.qualcomm.com, kvalo@kernel.org,
        davem@davemloft.net, kuba@kernel.org, linville@tuxdriver.com
Cc:     linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <80962aae265995d1cdb724f5362c556d494c7566.1644265120.git.paskripkin@gmail.com>
 <28c83b99b8fea0115ad7fbda7cc93a86468ec50d.1644265120.git.paskripkin@gmail.com>
 <258ac12b-9ca3-9b24-30df-148f9df51582@quicinc.com> <87ee4d9xxe.fsf@toke.dk>
From:   Pavel Skripkin <paskripkin@gmail.com>
In-Reply-To: <87ee4d9xxe.fsf@toke.dk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Toke,

On 2/8/22 18:32, Toke Høiland-Jørgensen wrote:
>> It seems that these macros (both the original and the new) aren't 
>> following the guidance from the Coding Style which tells us under 
>> "Things to avoid when using macros" that we should avoid "macros that 
>> depend on having a local variable with a magic name". Wouldn't these 
>> macros be "better" is they included the hif_dev/priv as arguments rather 
>> than being "magic"?
> 
> Hmm, yeah, that's a good point; looks like the non-HTC ath9k stats
> macros have already been converted to take the container as a parameter,
> so taking this opportunity to fix these macros is not a bad idea. While
> we're at it, let's switch to the do{} while(0) syntax the other macros
> are using instead of that weird usage of ?:. And there's not really any
> reason for the duplication between ADD/INC either. So I'm thinking
> something like:
> 
> #define __STAT_SAVE(_priv, _member, _n) do { if (_priv) (_priv)->_member += (_n); } while(0)
> 
> #define TX_STAT_ADD(_priv, _c, _a) __STAT_SAVE(_priv, debug.tx_stats._c, _a)
> #define TX_STAT_INC(_priv, _c) TX_STAT_ADD(_priv, _c, 1)
> 

Good point, thank you. Will redo these macros in v4


With regards,
Pavel Skripkin