2023-02-07 10:33:43

by Youghandhar Chintala

[permalink] [raw]
Subject: [PATCH v2 0/2] dt: bindings: add dt entry for XO calibration support

Add dt binding to get XO calibration data support for Wi-Fi RF clock.

Signed-off-by: Youghandhar Chintala <[email protected]>

Changes from v1:
- Sending the series to right list

Youghandhar Chintala (2):
dt: bindings: add dt entry for XO calibration support
wifi: ath11k: PMIC XO cal data support

.../bindings/net/wireless/qcom,ath11k.yaml | 4 ++++
drivers/net/wireless/ath/ath11k/ahb.c | 8 +++++++
drivers/net/wireless/ath/ath11k/core.h | 3 +++
drivers/net/wireless/ath/ath11k/qmi.c | 24 +++++++++++++++++++
drivers/net/wireless/ath/ath11k/qmi.h | 4 +++-
5 files changed, 42 insertions(+), 1 deletion(-)

--
2.38.0



2023-02-07 10:34:18

by Youghandhar Chintala

[permalink] [raw]
Subject: [PATCH v3 1/2] dt: bindings: net: ath11k: add dt entry for XO calibration support

Add dt binding to get XO calibration data support for Wi-Fi RF clock.

Retrieve the XO trim offset via system firmware (e.g., device tree),
especially in the case where the device doesn't have a useful EEPROM
on which to store the calibrated XO offset.
Calibrated XO offset is sent to firmware, which compensate the RF clock
drift by programing the XO trim register.

Signed-off-by: Youghandhar Chintala <[email protected]>
---
.../devicetree/bindings/net/wireless/qcom,ath11k.yaml | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
index f7cf135aa37f..4745251e70d4 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
@@ -41,6 +41,10 @@ properties:
* reg
* reg-names

+ xo-cal-data:
+ description:
+ XO frequency offset to program the XO trim register
+
qcom,ath11k-calibration-variant:
$ref: /schemas/types.yaml#/definitions/string
description:
--
2.38.0


2023-02-07 10:34:19

by Youghandhar Chintala

[permalink] [raw]
Subject: [PATCH v3 2/2] wifi: ath11k: PMIC XO cal data support

PMIC XO is the clock source for Wi-Fi RF clock in integrated Wi-Fi
chipset ex: WCN6750. Due to board layout errors XO frequency drifts
can cause Wi-Fi RF clock inaccuracy.
XO calibration test tree in Factory Test Mode is used to find the
best frequency offset(for example +/-2 KHz )by programming XO trim
register. This ensure system clock stays within required 20 ppm
WLAN RF clock.

Retrieve the XO trim offset via system firmware (e.g., device tree),
especially in the case where the device doesn't have a useful EEPROM
on which to store the calibrated XO offset (e.g., for integrated Wi-Fi).
Calibrated XO offset is sent to firmware, which compensate the clock drift
by programing the XO trim register.

Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1

Signed-off-by: Youghandhar Chintala <[email protected]>
---
drivers/net/wireless/ath/ath11k/ahb.c | 8 ++++++++
drivers/net/wireless/ath/ath11k/core.h | 3 +++
drivers/net/wireless/ath/ath11k/qmi.c | 24 ++++++++++++++++++++++++
drivers/net/wireless/ath/ath11k/qmi.h | 4 +++-
4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
index d34a4d6325b2..89580b4e47a9 100644
--- a/drivers/net/wireless/ath/ath11k/ahb.c
+++ b/drivers/net/wireless/ath/ath11k/ahb.c
@@ -1039,6 +1039,14 @@ static int ath11k_ahb_fw_resources_init(struct ath11k_base *ab)
ab_ahb->fw.iommu_domain = iommu_dom;
of_node_put(node);

+ ret = of_property_read_u32(pdev->dev.of_node, "xo-cal-data", &ab->xo_cal_data);
+ if (ret) {
+ ath11k_dbg(ab, ATH11K_DBG_AHB, "failed to get xo-cal-data property\n");
+ return 0;
+ }
+ ab->xo_cal_supported = true;
+ ath11k_dbg(ab, ATH11K_DBG_AHB, "xo cal data 0x%x\n", ab->xo_cal_data);
+
return 0;

err_iommu_unmap:
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 22460b0abf03..783398e98915 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -969,6 +969,9 @@ struct ath11k_base {
const struct ath11k_pci_ops *ops;
} pci;

+ bool xo_cal_supported;
+ u32 xo_cal_data;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 145f20a681bd..67f386b001ab 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -1451,6 +1451,24 @@ static struct qmi_elem_info qmi_wlanfw_wlan_mode_req_msg_v01_ei[] = {
.offset = offsetof(struct qmi_wlanfw_wlan_mode_req_msg_v01,
hw_debug),
},
+ {
+ .data_type = QMI_OPT_FLAG,
+ .elem_len = 1,
+ .elem_size = sizeof(u8),
+ .array_type = NO_ARRAY,
+ .tlv_type = 0x11,
+ .offset = offsetof(struct qmi_wlanfw_wlan_mode_req_msg_v01,
+ xo_cal_data_valid),
+ },
+ {
+ .data_type = QMI_UNSIGNED_1_BYTE,
+ .elem_len = 1,
+ .elem_size = sizeof(u8),
+ .array_type = NO_ARRAY,
+ .tlv_type = 0x11,
+ .offset = offsetof(struct qmi_wlanfw_wlan_mode_req_msg_v01,
+ xo_cal_data),
+ },
{
.data_type = QMI_EOTI,
.array_type = NO_ARRAY,
@@ -2610,6 +2628,12 @@ static int ath11k_qmi_wlanfw_mode_send(struct ath11k_base *ab,
req.hw_debug_valid = 1;
req.hw_debug = 0;

+ if (ab->xo_cal_supported) {
+ req.xo_cal_data_valid = 1;
+ req.xo_cal_data = ab->xo_cal_data;
+ }
+ ath11k_dbg(ab, ATH11K_DBG_QMI, "xo_cal_supported %d\n", ab->xo_cal_supported);
+
ret = qmi_txn_init(&ab->qmi.handle, &txn,
qmi_wlanfw_wlan_mode_resp_msg_v01_ei, &resp);
if (ret < 0)
diff --git a/drivers/net/wireless/ath/ath11k/qmi.h b/drivers/net/wireless/ath/ath11k/qmi.h
index 2ec56a34fa81..db61ce0d5689 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.h
+++ b/drivers/net/wireless/ath/ath11k/qmi.h
@@ -450,7 +450,7 @@ struct qmi_wlanfw_m3_info_resp_msg_v01 {
struct qmi_response_type_v01 resp;
};

-#define QMI_WLANFW_WLAN_MODE_REQ_MSG_V01_MAX_LEN 11
+#define QMI_WLANFW_WLAN_MODE_REQ_MSG_V01_MAX_LEN 17
#define QMI_WLANFW_WLAN_MODE_RESP_MSG_V01_MAX_LEN 7
#define QMI_WLANFW_WLAN_CFG_REQ_MSG_V01_MAX_LEN 803
#define QMI_WLANFW_WLAN_CFG_RESP_MSG_V01_MAX_LEN 7
@@ -470,6 +470,8 @@ struct qmi_wlanfw_wlan_mode_req_msg_v01 {
u32 mode;
u8 hw_debug_valid;
u8 hw_debug;
+ u8 xo_cal_data_valid;
+ u8 xo_cal_data;
};

struct qmi_wlanfw_wlan_mode_resp_msg_v01 {
--
2.38.0


2023-02-07 10:37:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt: bindings: net: ath11k: add dt entry for XO calibration support

On 07/02/2023 11:32, Youghandhar Chintala wrote:
> Add dt binding to get XO calibration data support for Wi-Fi RF clock.

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Several comments were ignored.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

Subject: drop second/last, redundant "dt entry for". The "dt-bindings"
prefix is already stating that these are bindings.

dt-bindings: net: qcom,ath11k: add XO frequency offset

>
> Retrieve the XO trim offset via system firmware (e.g., device tree),
> especially in the case where the device doesn't have a useful EEPROM
> on which to store the calibrated XO offset.
> Calibrated XO offset is sent to firmware, which compensate the RF clock
> drift by programing the XO trim register.
>
> Signed-off-by: Youghandhar Chintala <[email protected]>
> ---
> .../devicetree/bindings/net/wireless/qcom,ath11k.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> index f7cf135aa37f..4745251e70d4 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> @@ -41,6 +41,10 @@ properties:
> * reg
> * reg-names
>
> + xo-cal-data:
> + description:
> + XO frequency offset to program the XO trim register

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

(missing type/ref, vendor prefix).


Best regards,
Krzysztof


2023-02-07 10:38:41

by Robert Marko

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] wifi: ath11k: PMIC XO cal data support

On Tue, Feb 7, 2023 at 11:33 AM Youghandhar Chintala
<[email protected]> wrote:
>
> PMIC XO is the clock source for Wi-Fi RF clock in integrated Wi-Fi
> chipset ex: WCN6750. Due to board layout errors XO frequency drifts
> can cause Wi-Fi RF clock inaccuracy.
> XO calibration test tree in Factory Test Mode is used to find the
> best frequency offset(for example +/-2 KHz )by programming XO trim
> register. This ensure system clock stays within required 20 ppm
> WLAN RF clock.
>
> Retrieve the XO trim offset via system firmware (e.g., device tree),
> especially in the case where the device doesn't have a useful EEPROM
> on which to store the calibrated XO offset (e.g., for integrated Wi-Fi).
> Calibrated XO offset is sent to firmware, which compensate the clock drift
> by programing the XO trim register.

This is still a bit too vague for me and the DT binding is allowing
for pretty much
anything to be passed.

This XO trim offset is offset from which register base?
And, what is supposed to populate it?

Regards,
Robert
>
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1
>
> Signed-off-by: Youghandhar Chintala <[email protected]>
> ---
> drivers/net/wireless/ath/ath11k/ahb.c | 8 ++++++++
> drivers/net/wireless/ath/ath11k/core.h | 3 +++
> drivers/net/wireless/ath/ath11k/qmi.c | 24 ++++++++++++++++++++++++
> drivers/net/wireless/ath/ath11k/qmi.h | 4 +++-
> 4 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
> index d34a4d6325b2..89580b4e47a9 100644
> --- a/drivers/net/wireless/ath/ath11k/ahb.c
> +++ b/drivers/net/wireless/ath/ath11k/ahb.c
> @@ -1039,6 +1039,14 @@ static int ath11k_ahb_fw_resources_init(struct ath11k_base *ab)
> ab_ahb->fw.iommu_domain = iommu_dom;
> of_node_put(node);
>
> + ret = of_property_read_u32(pdev->dev.of_node, "xo-cal-data", &ab->xo_cal_data);
> + if (ret) {
> + ath11k_dbg(ab, ATH11K_DBG_AHB, "failed to get xo-cal-data property\n");
> + return 0;
> + }
> + ab->xo_cal_supported = true;
> + ath11k_dbg(ab, ATH11K_DBG_AHB, "xo cal data 0x%x\n", ab->xo_cal_data);
> +
> return 0;
>
> err_iommu_unmap:
> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
> index 22460b0abf03..783398e98915 100644
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -969,6 +969,9 @@ struct ath11k_base {
> const struct ath11k_pci_ops *ops;
> } pci;
>
> + bool xo_cal_supported;
> + u32 xo_cal_data;
> +
> /* must be last */
> u8 drv_priv[] __aligned(sizeof(void *));
> };
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 145f20a681bd..67f386b001ab 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -1451,6 +1451,24 @@ static struct qmi_elem_info qmi_wlanfw_wlan_mode_req_msg_v01_ei[] = {
> .offset = offsetof(struct qmi_wlanfw_wlan_mode_req_msg_v01,
> hw_debug),
> },
> + {
> + .data_type = QMI_OPT_FLAG,
> + .elem_len = 1,
> + .elem_size = sizeof(u8),
> + .array_type = NO_ARRAY,
> + .tlv_type = 0x11,
> + .offset = offsetof(struct qmi_wlanfw_wlan_mode_req_msg_v01,
> + xo_cal_data_valid),
> + },
> + {
> + .data_type = QMI_UNSIGNED_1_BYTE,
> + .elem_len = 1,
> + .elem_size = sizeof(u8),
> + .array_type = NO_ARRAY,
> + .tlv_type = 0x11,
> + .offset = offsetof(struct qmi_wlanfw_wlan_mode_req_msg_v01,
> + xo_cal_data),
> + },
> {
> .data_type = QMI_EOTI,
> .array_type = NO_ARRAY,
> @@ -2610,6 +2628,12 @@ static int ath11k_qmi_wlanfw_mode_send(struct ath11k_base *ab,
> req.hw_debug_valid = 1;
> req.hw_debug = 0;
>
> + if (ab->xo_cal_supported) {
> + req.xo_cal_data_valid = 1;
> + req.xo_cal_data = ab->xo_cal_data;
> + }
> + ath11k_dbg(ab, ATH11K_DBG_QMI, "xo_cal_supported %d\n", ab->xo_cal_supported);
> +
> ret = qmi_txn_init(&ab->qmi.handle, &txn,
> qmi_wlanfw_wlan_mode_resp_msg_v01_ei, &resp);
> if (ret < 0)
> diff --git a/drivers/net/wireless/ath/ath11k/qmi.h b/drivers/net/wireless/ath/ath11k/qmi.h
> index 2ec56a34fa81..db61ce0d5689 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.h
> +++ b/drivers/net/wireless/ath/ath11k/qmi.h
> @@ -450,7 +450,7 @@ struct qmi_wlanfw_m3_info_resp_msg_v01 {
> struct qmi_response_type_v01 resp;
> };
>
> -#define QMI_WLANFW_WLAN_MODE_REQ_MSG_V01_MAX_LEN 11
> +#define QMI_WLANFW_WLAN_MODE_REQ_MSG_V01_MAX_LEN 17
> #define QMI_WLANFW_WLAN_MODE_RESP_MSG_V01_MAX_LEN 7
> #define QMI_WLANFW_WLAN_CFG_REQ_MSG_V01_MAX_LEN 803
> #define QMI_WLANFW_WLAN_CFG_RESP_MSG_V01_MAX_LEN 7
> @@ -470,6 +470,8 @@ struct qmi_wlanfw_wlan_mode_req_msg_v01 {
> u32 mode;
> u8 hw_debug_valid;
> u8 hw_debug;
> + u8 xo_cal_data_valid;
> + u8 xo_cal_data;
> };
>
> struct qmi_wlanfw_wlan_mode_resp_msg_v01 {
> --
> 2.38.0
>
>
> --
> ath11k mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath11k



--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: [email protected]
Web: http://www.sartura.hr