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

[Roopni Devanathan]

On 1/30/2025 7:01 AM, Jeff Johnson wrote:
> 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.
>  */
> 
> 
Thanks for clarifying Jeff. I've sent a v6 addressing all these comments.

- Roopni.