2024-04-03 04:21:21

by Karthikeyan Periyasamy

[permalink] [raw]
Subject: [PATCH v2 0/2] [RESEND] wifi: ath12k: refactor the link capable flag

extend the link capable flag to accommodate inter-device capability.

v2:
- Rename the terminology from chip to device

Karthikeyan Periyasamy (2):
wifi: ath12k: extend the link capable flag
wifi: ath12k: fix link capable flags

drivers/net/wireless/ath/ath12k/core.c | 2 +-
drivers/net/wireless/ath/ath12k/core.h | 23 ++++++++++++++++++++---
drivers/net/wireless/ath/ath12k/mhi.c | 2 +-
drivers/net/wireless/ath/ath12k/qmi.c | 4 +++-
4 files changed, 25 insertions(+), 6 deletions(-)


base-commit: fe7e1b830cf3c0272aa4eaf367c4c7b29c169c3d
--
2.34.1



2024-04-03 04:21:27

by Karthikeyan Periyasamy

[permalink] [raw]
Subject: [PATCH v2 2/2] [RESEND] wifi: ath12k: fix link capable flags

By default driver supports intra-device SLO/MLO, the link capability flags
reflect this. When the QMI PHY capability learning fails driver not enable
the MLO parameter in the host capability QMI request message. In this case,
reset the device link capability flags to zero (SLO/MLO not support) to
accurately represent the capabilities.

Found in code review.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <[email protected]>
---
drivers/net/wireless/ath/ath12k/qmi.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 3f0d2b99127a..db8ba5fec2ae 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -2006,6 +2006,8 @@ static void ath12k_host_cap_parse_mlo(struct ath12k_base *ab,
int i;

if (!ab->qmi.num_radios || ab->qmi.num_radios == U8_MAX) {
+ ab->mlo_capable_flags = 0;
+
ath12k_dbg(ab, ATH12K_DBG_QMI,
"skip QMI MLO cap due to invalid num_radio %d\n",
ab->qmi.num_radios);
--
2.34.1


2024-04-03 04:21:26

by Karthikeyan Periyasamy

[permalink] [raw]
Subject: [PATCH v2 1/2] [RESEND] wifi: ath12k: extend the link capable flag

Link capability categorized as Single Link Operation (SLO) and Multi Link
Operation (MLO).

- Intra-device SLO/MLO refers to links present within a device
- Inter-device SLO/MLO refers to links present across multiple devices

Currently, driver uses a boolean variable to represent intra-device SLO/MLO
capability. To accommodate inter-device SLO/MLO capabilities within the
same variable, modify the existing variable name and type. Define a new
enumeration for the link capabilities to accommodate both intra-device
and inter-device scenarios. Populate the enum based on the supported
capabilities.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1

Signed-off-by: Karthikeyan Periyasamy <[email protected]>
---
drivers/net/wireless/ath/ath12k/core.c | 2 +-
drivers/net/wireless/ath/ath12k/core.h | 23 ++++++++++++++++++++---
drivers/net/wireless/ath/ath12k/mhi.c | 2 +-
drivers/net/wireless/ath/ath12k/qmi.c | 2 +-
4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 391b6fb2bd42..942a4274fda1 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1227,7 +1227,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev, size_t priv_size,
ab->dev = dev;
ab->hif.bus = bus;
ab->qmi.num_radios = U8_MAX;
- ab->slo_capable = true;
+ ab->mlo_capable_flags = ATH12k_INTRA_DEVICE_MLO_SUPPORT;

return ab;

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 97e5a0ccd233..e4eabd0f6e0e 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -688,6 +688,21 @@ struct ath12k_soc_dp_stats {
struct ath12k_soc_dp_tx_err_stats tx_err;
};

+/**
+ * enum ath12k_link_capable_flags - link capable flags
+ *
+ * Single/Multi link capability information
+ *
+ * @ATH12k_INTRA_DEVICE_MLO_SUPPORT: SLO/MLO form between the radio, where all
+ * the links (radios) present within a device.
+ * @ATH12k_INTER_DEVICE_MLO_SUPPORT: SLO/MLO form between the radio, where all
+ * the links (radios) present across the devices.
+ */
+enum ath12k_link_capable_flags {
+ ATH12k_INTRA_DEVICE_MLO_SUPPORT = BIT(0),
+ ATH12k_INTER_DEVICE_MLO_SUPPORT = BIT(1),
+};
+
/* Master structure to hold the hw data which may be used in core module */
struct ath12k_base {
enum ath12k_hw_rev hw_rev;
@@ -843,10 +858,12 @@ struct ath12k_base {

const struct hal_rx_ops *hal_rx_ops;

- /* slo_capable denotes if the single/multi link operation
- * is supported within the same chip (SoC).
+ /* mlo_capable_flags denotes the single/multi link operation
+ * capabilities of the Device.
+ *
+ * See enum ath12k_link_capable_flags
*/
- bool slo_capable;
+ u8 mlo_capable_flags;

/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
diff --git a/drivers/net/wireless/ath/ath12k/mhi.c b/drivers/net/wireless/ath/ath12k/mhi.c
index adb8c3ec1950..fd519c87ae24 100644
--- a/drivers/net/wireless/ath/ath12k/mhi.c
+++ b/drivers/net/wireless/ath/ath12k/mhi.c
@@ -385,7 +385,7 @@ int ath12k_mhi_register(struct ath12k_pci *ab_pci)
"failed to read board id\n");
} else if (board_id & OTP_VALID_DUALMAC_BOARD_ID_MASK) {
dualmac = true;
- ab->slo_capable = false;
+ ab->mlo_capable_flags = 0;
ath12k_dbg(ab, ATH12K_DBG_BOOT,
"dualmac fw selected for board id: %x\n", board_id);
}
diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 92845ffff44a..3f0d2b99127a 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -2124,7 +2124,7 @@ static void ath12k_qmi_phy_cap_send(struct ath12k_base *ab)
struct qmi_txn txn;
int ret;

- if (!ab->slo_capable)
+ if (!ab->mlo_capable_flags)
goto out;

ret = qmi_txn_init(&ab->qmi.handle, &txn,
--
2.34.1


2024-04-03 11:03:15

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] [RESEND] wifi: ath12k: refactor the link capable flag

Karthikeyan Periyasamy <[email protected]> writes:

> extend the link capable flag to accommodate inter-device capability.
>
> v2:
> - Rename the terminology from chip to device
>
> Karthikeyan Periyasamy (2):
> wifi: ath12k: extend the link capable flag
> wifi: ath12k: fix link capable flags

Why the resend? I'm not really a fan of these "RESEND" versions. If you
need to resend something simply just increase the version number (in
this case to v3) and explain in the change log why you sent v3. That's a
lot more clear for everyone than having "v2", "v2 RESEND", "v2 RESEND
RESEND" and what not. It's not like we are running out of version
numbers ;)

But no need to resend because of this :)

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

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

2024-04-03 11:30:00

by Karthikeyan Periyasamy

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] [RESEND] wifi: ath12k: refactor the link capable flag



On 4/3/2024 4:08 PM, Kalle Valo wrote:
> Karthikeyan Periyasamy <[email protected]> writes:
>
>> extend the link capable flag to accommodate inter-device capability.
>>
>> v2:
>> - Rename the terminology from chip to device
>>
>> Karthikeyan Periyasamy (2):
>> wifi: ath12k: extend the link capable flag
>> wifi: ath12k: fix link capable flags
>
> Why the resend?

I missed the version log in the commit message.

I'm not really a fan of these "RESEND" versions. If you
> need to resend something simply just increase the version number (in
> this case to v3) and explain in the change log why you sent v3. That's a
> lot more clear for everyone than having "v2", "v2 RESEND", "v2 RESEND
> RESEND" and what not. It's not like we are running out of version
> numbers ;)
>
> But no need to resend because of this :)
>

Sure, i got it.

--
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி

2024-04-03 15:30:58

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] [RESEND] wifi: ath12k: refactor the link capable flag

On 4/3/2024 4:21 AM, Karthikeyan Periyasamy wrote:
> On 4/3/2024 4:08 PM, Kalle Valo wrote:
>> Why the resend?
>
> I missed the version log in the commit message.

In that case in the future, just reply to your cover letter and say you missed
including the versioning info, and put it in the reply.


2024-04-03 19:29:25

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] [RESEND] wifi: ath12k: fix link capable flags

On 4/2/2024 9:20 PM, Karthikeyan Periyasamy wrote:
> By default driver supports intra-device SLO/MLO, the link capability flags
> reflect this. When the QMI PHY capability learning fails driver not enable
> the MLO parameter in the host capability QMI request message. In this case,
> reset the device link capability flags to zero (SLO/MLO not support) to
> accurately represent the capabilities.
>
> Found in code review.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Acked-by: Jeff Johnson <[email protected]>



2024-04-03 20:02:15

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] [RESEND] wifi: ath12k: extend the link capable flag

On 4/2/2024 9:20 PM, Karthikeyan Periyasamy wrote:
> Link capability categorized as Single Link Operation (SLO) and Multi Link
> Operation (MLO).
>
> - Intra-device SLO/MLO refers to links present within a device
> - Inter-device SLO/MLO refers to links present across multiple devices
>
> Currently, driver uses a boolean variable to represent intra-device SLO/MLO
> capability. To accommodate inter-device SLO/MLO capabilities within the
> same variable, modify the existing variable name and type. Define a new
> enumeration for the link capabilities to accommodate both intra-device
> and inter-device scenarios. Populate the enum based on the supported
> capabilities.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
Acked-by: Jeff Johnson <[email protected]>



2024-04-05 12:58:06

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] [RESEND] wifi: ath12k: extend the link capable flag

Karthikeyan Periyasamy <[email protected]> writes:

> Link capability categorized as Single Link Operation (SLO) and Multi Link
> Operation (MLO).
>
> - Intra-device SLO/MLO refers to links present within a device
> - Inter-device SLO/MLO refers to links present across multiple devices
>
> Currently, driver uses a boolean variable to represent intra-device SLO/MLO
> capability. To accommodate inter-device SLO/MLO capabilities within the
> same variable, modify the existing variable name and type. Define a new
> enumeration for the link capabilities to accommodate both intra-device
> and inter-device scenarios. Populate the enum based on the supported
> capabilities.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>

[...]

> +enum ath12k_link_capable_flags {
> + ATH12k_INTRA_DEVICE_MLO_SUPPORT = BIT(0),
> + ATH12k_INTER_DEVICE_MLO_SUPPORT = BIT(1),
> +};

I did s/ATH12k/ATH12K/ on this patch:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=c15ec7e3397e7a400b2a6a3cb232f95a94c95989

We have also another similar typo in ath12k:

dp.c:1350: spt_idx = u32_get_bits(cookie, ATH12k_DP_CC_COOKIE_SPT);
dp.c:1368: spt_idx = u32_get_bits(cookie, ATH12k_DP_CC_COOKIE_SPT);
dp.h:248:#define ATH12k_DP_CC_COOKIE_SPT GENMASK(8, 0)

Patches welcome.

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

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

2024-04-08 14:38:20

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] [RESEND] wifi: ath12k: extend the link capable flag

Karthikeyan Periyasamy <[email protected]> wrote:

> Link capability categorized as Single Link Operation (SLO) and Multi Link
> Operation (MLO).
>
> - Intra-device SLO/MLO refers to links present within a device
> - Inter-device SLO/MLO refers to links present across multiple devices
>
> Currently, driver uses a boolean variable to represent intra-device SLO/MLO
> capability. To accommodate inter-device SLO/MLO capabilities within the
> same variable, modify the existing variable name and type. Define a new
> enumeration for the link capabilities to accommodate both intra-device
> and inter-device scenarios. Populate the enum based on the supported
> capabilities.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Karthikeyan Periyasamy <[email protected]>
> Acked-by: Jeff Johnson <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

2 patches applied to ath-next branch of ath.git, thanks.

58e8ecda692e wifi: ath12k: extend the link capable flag
b34389c3978c wifi: ath12k: fix link capable flags

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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