Received-SPF: pass (google.com: domain of linux-wireless+bounces-3133-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Kalle Valo <kvalo@kernel.org>
To: Jeff Johnson <quic_jjohnson@quicinc.com>
Cc: Kang Yang <quic_kangyang@quicinc.com>,  <ath12k@lists.infradead.org>,
  <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v6 04/11] wifi: ath12k: add P2P IE in beacon template
References: <20240130040303.370590-1-quic_kangyang@quicinc.com>
	<20240130040303.370590-5-quic_kangyang@quicinc.com>
	<f947f804-6fd1-41eb-8b6f-c562d51e5441@quicinc.com>
Date: Mon, 05 Feb 2024 18:24:04 +0200
In-Reply-To: <f947f804-6fd1-41eb-8b6f-c562d51e5441@quicinc.com> (Jeff
	Johnson's message of "Tue, 30 Jan 2024 09:02:54 -0800")
Message-ID: <877cjiswuz.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 1/29/2024 8:02 PM, Kang Yang wrote:
>
>> P2P Element is a necessary component of P2P protocol communication.
>> It contains the Vendor Specific Information Element which includes
>> the WFA OUI and an OUI Type indicating P2P.
>> 
>> Add P2P IE in beacon template, and implement WMI interface for it.
>> 
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> 
>> Signed-off-by: Kang Yang <quic_kangyang@quicinc.com>

[...]

>> +int ath12k_wmi_p2p_go_bcn_ie(struct ath12k *ar, u32 vdev_id,
>> +			     const u8 *p2p_ie)
>> +{
> ...
>> +	ptr = skb->data;
>> +	cmd = ptr;
>> +	cmd->tlv_header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_P2P_GO_SET_BEACON_IE,
>> +						 sizeof(*cmd));
>> +	cmd->vdev_id = cpu_to_le32(vdev_id);
>> +	cmd->ie_buf_len = cpu_to_le32(p2p_ie_len);
>> +
>> +	ptr = skb->data + sizeof(*cmd);
>
> IMO better would be: ptr += sizeof(*cmd);
>
> Kalle can probably adjust this in the pending branch

Yeah, I changed that.

But I think even better would be to avoid pointer arightmetic and do something
like this:

struct wmi_p2p_go_set_beacon_ie_cmd {
	__le32 tlv_header;
	__le32 vdev_id;
	__le32 ie_buf_len;
        u8 tlv[];
} __packed;

tlv = cmd->tlv;
tlv->header = ath12k_wmi_tlv_cmd_hdr(WMI_TAG_ARRAY_BYTE,
					     aligned_len);

But that's separate cleanup, I see pointer arithmetic used quite a lot
in wmi.c.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches