Re: [PATCH 01/17] Add cc33xx.h, cc33xx_i.h

Nemanov, Michael
2024-08-06T16:57:09Z

On 5/22/2024 12:38 PM, Krzysztof Kozlowski wrote:
>
>> + struct core_status *core_status;
>> + u8 last_fw_rls_idx;
>> + u8 command_result[CC33XX_CMD_MAX_SIZE];
>> + u16 result_length;
>> + struct partial_rx_frame partial_rx;
>> +
>> + unsigned long flags;
>> +
>> + void *nvs_mac_addr;
>> + size_t nvs_mac_addr_len;
>> + struct cc33xx_fw_download *fw_download;
>> +
>> + struct mac_address addresses[CC33XX_NUM_MAC_ADDRESSES];
>> +
>> + unsigned long links_map[BITS_TO_LONGS(CC33XX_MAX_LINKS)];
>> + unsigned long roles_map[BITS_TO_LONGS(CC33XX_MAX_ROLES)];
>> + unsigned long roc_map[BITS_TO_LONGS(CC33XX_MAX_ROLES)];
>> + unsigned long rate_policies_map[BITS_TO_LONGS(CC33XX_MAX_RATE_POLICIES)];
>> +
>> + u8 session_ids[CC33XX_MAX_LINKS];
>> +
>> + struct list_head wlvif_list;
>> +
>> + u8 sta_count;
>> + u8 ap_count;
>> +
>> + struct cc33xx_acx_mem_map *target_mem_map;
>> +
>> + /* Accounting for allocated / available TX blocks on HW */
>> +
>> + u32 tx_blocks_available;
>> + u32 tx_allocated_blocks;
>> +
>> + /* Accounting for allocated / available Tx packets in HW */
>> +
>> + u32 tx_allocated_pkts[NUM_TX_QUEUES];
>> +
>> + /* Time-offset between host and chipset clocks */
>> +
>> + /* Frames scheduled for transmission, not handled yet */
>> + int tx_queue_count[NUM_TX_QUEUES];
>> + unsigned long queue_stop_reasons[NUM_TX_QUEUES * CC33XX_NUM_MAC_ADDRESSES];
>> +
>> + /* Frames received, not handled yet by mac80211 */
>> + struct sk_buff_head deferred_rx_queue;
>> +
>> + /* Frames sent, not returned yet to mac80211 */
>> + struct sk_buff_head deferred_tx_queue;
>> +
>> + struct work_struct tx_work;
>> + struct workqueue_struct *freezable_wq;
>> +
>> + /*freezable wq for netstack_work*/
>> + struct workqueue_struct *freezable_netstack_wq;
>> +
>> + /* Pending TX frames */
>> + unsigned long tx_frames_map[BITS_TO_LONGS(CC33XX_MAX_TX_DESCRIPTORS)];
>> + struct sk_buff *tx_frames[CC33XX_MAX_TX_DESCRIPTORS];
>> + int tx_frames_cnt;
>> +
>> + /* FW Rx counter */
>> + u32 rx_counter;
>> +
>> + /* Intermediate buffer, used for packet aggregation */
>> + u8 *aggr_buf;
>> + u32 aggr_buf_size;
>> + size_t max_transaction_len;
>> +
>> + /* Reusable dummy packet template */
>> + struct sk_buff *dummy_packet;
>> +
>> + /* Network stack work */
>> + struct work_struct netstack_work;
>> + /* FW log buffer */
>> + u8 *fwlog;
>> +
>> + /* Number of valid bytes in the FW log buffer */
>> + ssize_t fwlog_size;
>> +
>> + /* Hardware recovery work */
>> + struct work_struct recovery_work;
>> +
>> + struct work_struct irq_deferred_work;
>> +
>> + /* Reg domain last configuration */
>> + DECLARE_BITMAP(reg_ch_conf_last, 64);
>> + /* Reg domain pending configuration */
>> + DECLARE_BITMAP(reg_ch_conf_pending, 64);
>> +
>> + /* Lock-less list for deferred event handling */
>> + struct llist_head event_list;
>> + /* The mbox event mask */
>> + u32 event_mask;
>> + /* events to unmask only when ap interface is up */
>> + u32 ap_event_mask;
>> +
>> + /* Are we currently scanning */
>> + struct cc33xx_vif *scan_wlvif;
>> + struct cc33xx_scan scan;
>> + struct delayed_work scan_complete_work;
>> +
>> + struct ieee80211_vif *roc_vif;
>> + struct delayed_work roc_complete_work;
>> +
>> + struct cc33xx_vif *sched_vif;
>> +
>> + u8 mac80211_scan_stopped;
>> +
>> + /* The current band */
>> + enum nl80211_band band;
>> +
>> + /* in dBm */
>> + int power_level;
>> +
>> + struct cc33xx_stats stats;
>> +
>> + __le32 *buffer_32;
>> +
>> + /* Current chipset configuration */
>> + struct cc33xx_conf_file conf;
>> +
>> + bool enable_11a;
>> +
>> + /* bands supported by this instance of cc33xx */
>> + struct ieee80211_supported_band bands[CC33XX_NUM_BANDS];
>> +
>> + /* wowlan trigger was configured during suspend.
>> + * (currently, only "ANY" and "PATTERN" trigger is supported)
>> + */
>> +
>> + bool keep_device_power;
>> +
>> + /* AP-mode - links indexed by HLID. The global and broadcast links
>> + * are always active.
>> + */
>> + struct cc33xx_link links[CC33XX_MAX_LINKS];
>> +
>> + /* number of currently active links */
>> + int active_link_count;
>> +
>> + /* AP-mode - a bitmap of links currently in PS mode according to FW */
>> + unsigned long ap_fw_ps_map;
>> +
>> + /* AP-mode - a bitmap of links currently in PS mode in mac80211 */
>> + unsigned long ap_ps_map;
>> +
>> + /* Quirks of specific hardware revisions */
>> + unsigned int quirks;
>> +
>> + /* number of currently active RX BA sessions */
>> + int ba_rx_session_count;
>> +
>> + /* AP-mode - number of currently connected stations */
>> + int active_sta_count;
>> +
>> + /* last wlvif we transmitted from */
>> + struct cc33xx_vif *last_wlvif;
>> +
>> + /* work to fire when Tx is stuck */
>> + struct delayed_work tx_watchdog_work;
>> +
>> + /* HW HT (11n) capabilities */
>> + struct ieee80211_sta_ht_cap ht_cap[CC33XX_NUM_BANDS];
>> +
>> + /* the current dfs region */
>> + enum nl80211_dfs_regions dfs_region;
>> + bool radar_debug_mode;
>> +
>> + /* RX Data filter rule state - enabled/disabled */
>> + /* used in CONFIG PM AND W8 Code */
>> + unsigned long rx_filter_enabled[BITS_TO_LONGS(CC33XX_MAX_RX_FILTERS)];
>> +
>> + /* mutex for protecting the tx_flush function */
>> + struct mutex flush_mutex;
>> +
>> + /* sleep auth value currently configured to FW */
>> + int sleep_auth;
>> +
>> + /*ble_enable value - 1=enabled, 0=disabled. */
>> + int ble_enable;
>> +
>> + /* parameters for joining a TWT agreement */
>> + int min_wake_duration_usec;
>> + int min_wake_interval_mantissa;
>> + int min_wake_interval_exponent;
>> + int max_wake_interval_mantissa;
>> + int max_wake_interval_exponent;
>> +
>> + /* the number of allocated MAC addresses in this chip */
>> + int num_mac_addr;
>> +
>> + /* sta role index - if 0 - wlan0 primary station interface,
>> + * if 1 - wlan2 - secondary station interface
>> + */
>> + u8 sta_role_idx;
>> +
>> + u16 max_cmd_size;
>> +
>> + struct completion nvs_loading_complete;
>> + struct completion command_complete;
>> +
>> + /* dynamic fw traces */
>> + u32 dynamic_fw_traces;
>> +
>> + /* buffer for sending commands to FW */
>> + u8 cmd_buf[CC33XX_CMD_BUFFER_SIZE];
>> +
>> + /* number of keys requiring extra spare mem-blocks */
>> + int extra_spare_key_count;
>
> This entire struct is quite unmanageable...
>

From what I saw many wireless drivers hold their private members in a
single structure of roughly the same size. Most of these are unrelated
to each other so I doubt grouping them in sub-structures will improve
readability.