On 17/04/2024 10:33, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>
>> A few of the shared functions need small changes for the USB driver:
>> - firmware loading
>> - efuse reading
>> - rate mask updating
>> - rf register reading
>> - initial gain for scanning
>>
>> Also, add a few macros to wifi.h, add some new members to struct
>> rtl_priv, and initialise rtlhal.interfaceindex for USB devices.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>> ---
>> v5:
>> - No change.
>>
>> v4:
>> - Update the commit message.
>> - Move _rtl92d_pci_lock and _rtl92d_pci_unlock to phy_common.h to
>> silence a checkpatch warning.
>> - Move rate_mask declaration higher (sorted by length).
>> - Add some bit definitions to reg.h.
>> - Add to struct rtl_priv the things shared by the two USB interfaces
>> of a dual MAC RTL8192DU:
>> u32 *curveindex_2g;
>> u32 *curveindex_5g;
>> struct mutex *mutex_for_power_on_off;
>> struct mutex *mutex_for_hw_init;
>>
>> v3:
>> - Silence sparse warnings about locks in phy_common.c by using helper
>> functions _rtl92d_pci_lock() and _rtl92d_pci_unlock().
>>
>> v2:
>> - Delete the macro IS_NORMAL_CHIP. It wasn't actually used.
>> - Add RTL_USB_DELAY_FACTOR macro.
>> - Use FEN_CPUEN macro.
>> - Make params_pci and params_usb arrays static const, and make
>> rtl_get_hwinfo() take a const pointer.
>> - Put the rate mask, macid, etc. in a struct in
>> rtl92de_update_hal_rate_mask().
>> - Use sizeof(rtlpriv->rate_mask) in memcpy instead of hardcoded 5.
>> - Rename temp variable to phy_ctrl.
>> - Define _TXDMA_*_MAP and _HPQ, _LPQ, _PUBQ macros using GENMASK and
>> remove the leading underscore.
>> - Delete _NPQ macro because it's not very useful.
>
> [...]
>
>
>> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
>> b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
>> index 33b8ba363ca2..3d994238985f 100644
>> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
>> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192d/fw_common.h
>> @@ -25,6 +25,16 @@
>> #define GET_FIRMWARE_HDR_SUB_VER(__fwhdr) \
>> le32_get_bits(*(__le32 *)((__fwhdr) + 4), GENMASK(23, 16))
>>
>> +#define RAID_MASK GENMASK(31, 28)
>> +#define RATE_MASK_MASK GENMASK(27, 0)
>> +#define SHORT_GI_MASK BIT(5)
>> +#define MACID_MASK GENMASK(4, 0)
>> +
>> +struct rtl92d_rate_mask_h2c {
>> + __le32 rate_mask_and_raid;
>> + u8 macid_and_short_gi;
>> +} __packed;
>> +
>
> You asked me to drop a patch that looks very like this. Is this part correct?
>
Yes, it is correct in rtl8192d-common. This is what rtl92d_fill_h2c_cmd()
expects. It was not correct in rtl8xxxu because rtl8xxxu_gen1_h2c_cmd()
expects something else.
> [...]
>
>> @@ -2768,6 +2773,12 @@ struct rtl_priv {
>> */
>> bool use_new_trx_flow;
>>
>> + /* For dual MAC RTL8192DU, things shared by the 2 USB interfaces */
>> + u32 *curveindex_2g;
>> + u32 *curveindex_5g;
>> + struct mutex *mutex_for_power_on_off; /* for power on/off */
>> + struct mutex *mutex_for_hw_init; /* for hardware init */
>
> It would be better to introduce these locks along with consumers. Then, I will
> not miss to review them carefully.
>
Okay, I will remove them from this patch set.
> By the way, I don't review remaining patches of v4 until this new (smaller)
> patchset gets merged.
>
That makes sense.
