Re: [PATCH v2 4/9] wifi: rtw88: Add rtw8703b.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 02.03.24 um 01:33 schrieb Ping-Ke Shih:
On Fri, 2024-03-01 at 17:35 +0100, Fiona Klute wrote:

Am 01.03.24 um 03:09 schrieb Ping-Ke Shih:
+++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
+/* Copyright Fiona Klute <fiona.klute@xxxxxx> */
+
+#ifndef __RTW8703B_H__
+#define __RTW8703B_H__
+
+#include <linux/types.h>
+#include <linux/compiler_attributes.h>

Removing these two headers can still compiled pass in my side. Please check why
you need them.

If I remove them whether the code compiles depends on the order of
#includes. If some other header that includes those two is included
before rtw8703b.h it works, otherwise it will break. I don't think
that's desirable, though other rtw88 headers already behave that way
(e.g. main.h must be included before the others). Also, clangd will
complain about undefined types (u8, s8), which is less important but
still annoying when working on the code. So I'd prefer to keep the includes.

By the way, rtw8723d.h does
     #include "rtw8723x.h"

Can we use the same stuff?

I don't think so. The vendor driver has different code paths: With 8723D
it takes one for "PHYSTS_2ND_TYPE_IC" [1], with 8703B the one for
"ODM_IC_11N_SERIES" a few lines below. For those "2nd type" ICs there
are different structs depending on the page number
(phy_sts_rpt_jgr2_type[0-2]), while the "11N series" ones always use
phy_status_rpt_8192cd (all defined in [2]).

However, the mainline rtlwifi driver has an equivalent struct called
"phy_status_rpt" in
drivers/net/wireless/realtek/rtlwifi/rtl8723be/trx.h. I don't know if
sharing such definitions between different drivers is desirable. If yes,
please let me know where the header should go.

[1]
https://xff.cz/git/linux/tree/drivers/staging/rtl8723cs/hal/phydm/phydm_phystatus.c?h=orange-pi-6.7#n3142
[2]
https://xff.cz/git/linux/tree/drivers/staging/rtl8723cs/hal/phydm/phydm_phystatus.h?h=orange-pi-6.7


Sorry for confusing. I meant to remove
    #include <linux/types.h>
    #include <linux/compiler_attributes.h>
and to add
    #include "rtw8723x.h"
like rtw8723d.h does after this patchset, not reference to vendor driver.

Oh yes, that makes sense. Thanks for the clarification, I'll do that.

Best regards,
Fiona






[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux