Re: [PATCH v5 1/2] wifi: ath12k: Add Support to Parse TPC Event from Firmware

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 1/28/2025 3:10 AM, Roopni Devanathan wrote:
...
> +struct wmi_max_reg_power_allowed_params {

this seems to be misnamed. per the wmi naming convention _params are structs
passed from firmware. you cannot pass a pointer to an array from firmware to
host, you can only pass the array contents. hence seems this is a host-only
struct and should be named _arg

> +	struct wmi_max_reg_power_fixed_params tpc_reg_pwr;
> +	s16 *reg_pwr_array;
> +};
> +
> +struct wmi_tpc_rates_array_fixed_params {
> +	__le32 rate_array_type;
> +	__le32 rate_array_len;
> +} __packed;
> +
> +struct wmi_tpc_rates_array_params {

again seems this should be _arg

> +	struct wmi_tpc_rates_array_fixed_params tpc_rates_array;
> +	s16 *rate_array;
> +};
> +
> +struct wmi_tpc_ctl_pwr_fixed_params {
> +	__le32 ctl_array_type;
> +	__le32 ctl_array_len;
> +	__le32 end_of_ctl_pwr;
> +	__le32 ctl_pwr_count;
> +	__le32 d1;
> +	__le32 d2;
> +	__le32 d3;
> +	__le32 d4;
> +} __packed;
> +
> +struct wmi_tpc_ctl_pwr_table_params {

again seems this should be _arg

> +	struct wmi_tpc_ctl_pwr_fixed_params tpc_ctl_pwr;
> +	s8 *ctl_pwr_table;
> +};
> +
> +struct wmi_tpc_stats_info {

same here

> +	u32 pdev_id;
> +	u32 event_count;
> +	u32 end_of_event;
> +	u32 tlvs_rcvd;
> +	struct wmi_max_reg_power_allowed_params max_reg_allowed_power;
> +	struct wmi_tpc_rates_array_params rates_array1;
> +	struct wmi_tpc_rates_array_params rates_array2;
> +	struct wmi_tpc_config_params tpc_config;
> +	struct wmi_tpc_ctl_pwr_table_params ctl_array;
> +};

reminder from the top of the file:
/* Naming conventions for structures:
 *
 * _cmd means that this is a firmware command sent from host to firmware.
 *
 * _event means that this is a firmware event sent from firmware to host
 *
 * _params is a structure which is embedded either into _cmd or _event (or
 * both), it is not sent individually.
 *
 * _arg is used inside the host, the firmware does not see that at all.
 */