From:   Kalle Valo <kvalo@kernel.org>
To:     Ping-Ke Shih <pkshih@realtek.com>
Cc:     <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH] wifi: rtw89: add counters of register-based H2C/C2H
References: <20230310035943.49020-1-pkshih@realtek.com>
Date:   Wed, 15 Mar 2023 11:52:22 +0200
In-Reply-To: <20230310035943.49020-1-pkshih@realtek.com> (Ping-Ke Shih's
        message of "Fri, 10 Mar 2023 11:59:43 +0800")
Message-ID: <87r0tqmje1.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Ping-Ke Shih <pkshih@realtek.com> writes:

> The register-based H2C/C2H are used to exchange information between driver
> and firmware, but only apply to narrow area because its data size is
> smaller than regular packet-based H2C/C2H.
>
> This kind of H2C/C2H must be paired. To identify if any H2C/C2H is missing,
> update counters to help diagnose this kind of problems.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

[...]

> +static inline void
> +rtw89_write8_mask_add(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u8 add)
> +{
> +	u32 shift;
> +	u8 orig, set;
> +	u8 data;
> +
> +	mask &= 0xff;
> +	shift = __ffs(mask);
> +
> +	orig = rtw89_read8(rtwdev, addr);
> +	data = ((orig & mask) >> shift) + add;
> +	set = (orig & ~mask) | ((data << shift) & mask);
> +	rtw89_write8(rtwdev, addr, set);
> +}

This function has a lot of shifting etc which feels like reinventing the
wheel, doesn't linux/bitfield.h contain what you need? For example,
u32_get_bits() and u32_replace_bits()?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches