[PATCH v2 0/2] Add xo calibration support for wifi rf clock

[PATCH v2 0/2] Add xo calibration support for wifi rf clock

[Govind Singh]

PMIC XO is the clock source for wifi rf clock in integrated wifi
chipset ex: WCN3990. Due to board layout errors XO frequency drifts
can cause wifi rf clock inaccuracy.
XO calibration test tree in Factory Test Mode is used to find the
best frequency offset(for example +/-2KHz )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 Wifi).
Calibrated XO offset is sent to fw, which compensate the clock drift
by programing the XO trim register.

Testing:
        Tested on QCS404 platform(WCN3990 HW)
        Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1

change since v1:
        Added return check for case where xo cal dt is not populated.

Govind Singh (2):
  dt: bindings: add dt entry for XO calibration support
  ath10k: Add xo calibration support for wifi rf clock

 .../devicetree/bindings/net/wireless/qcom,ath10k.txt |  1 +
 drivers/net/wireless/ath/ath10k/qmi.c                | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/snoc.c               | 11 +++++++++++
 drivers/net/wireless/ath/ath10k/snoc.h               |  2 ++
 4 files changed, 26 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH v2 0/2] Add xo calibration support for wifi rf clock

[Govind Singh]

PMIC XO is the clock source for wifi rf clock in integrated wifi
chipset ex: WCN3990. Due to board layout errors XO frequency drifts
can cause wifi rf clock inaccuracy.
XO calibration test tree in Factory Test Mode is used to find the
best frequency offset(for example +/-2KHz )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 Wifi).
Calibrated XO offset is sent to fw, which compensate the clock drift
by programing the XO trim register.

Testing:
        Tested on QCS404 platform(WCN3990 HW)
        Tested FW: WLAN.HL.3.1-00959-QCAHLSWMTPLZ-1

change since v1:
        Added return check for case where xo cal dt is not populated.

Govind Singh (2):
  dt: bindings: add dt entry for XO calibration support
  ath10k: Add xo calibration support for wifi rf clock

 .../devicetree/bindings/net/wireless/qcom,ath10k.txt |  1 +
 drivers/net/wireless/ath/ath10k/qmi.c                | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/snoc.c               | 11 +++++++++++
 drivers/net/wireless/ath/ath10k/snoc.h               |  2 ++
 4 files changed, 26 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Govind Singh]

Add dt binding to get xo calibration data support for wifi rf clock.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
 Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
index ae661e65354e..ab8042866e83 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
@@ -81,6 +81,7 @@ Optional properties:
 	Definition: Name of external front end module used. Some valid FEM names
 		    for example: "microsemi-lx5586", "sky85703-11"
 		    and "sky85803" etc.
+- xo-cal-data: xo cal offset to be configured in xo trim register.
 
 Example (to supply PCI based wifi block details):
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Govind Singh]

Add dt binding to get xo calibration data support for wifi rf clock.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
 Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
index ae661e65354e..ab8042866e83 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
@@ -81,6 +81,7 @@ Optional properties:
 	Definition: Name of external front end module used. Some valid FEM names
 		    for example: "microsemi-lx5586", "sky85703-11"
 		    and "sky85803" etc.
+- xo-cal-data: xo cal offset to be configured in xo trim register.
 
 Example (to supply PCI based wifi block details):
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Brian Norris]

On Tue, Mar 19, 2019 at 9:48 PM Govind Singh <govinds@codeaurora.org> wrote:
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
> @@ -81,6 +81,7 @@ Optional properties:
>         Definition: Name of external front end module used. Some valid FEM names
>                     for example: "microsemi-lx5586", "sky85703-11"
>                     and "sky85803" etc.
> +- xo-cal-data: xo cal offset to be configured in xo trim register.

https://media.readthedocs.org/pdf/devicetree-specification/stable/devicetree-specification.pdf

"2.2.4 Properties
...
Nonstandard property names should specify a unique string prefix, such
as a stock ticker symbol, identifying the name of the company or
organization that defined the property. Examples:
  fsl,channel-fifo-len ibm,ppc-interrupt-server#s
  linux,network-index"

In other words, like most other custom properties in this file, you
should probably have a "qcom," prefix.

Brian

>  Example (to supply PCI based wifi block details):

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Brian Norris]

On Tue, Mar 19, 2019 at 9:48 PM Govind Singh <govinds@codeaurora.org> wrote:
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
> @@ -81,6 +81,7 @@ Optional properties:
>         Definition: Name of external front end module used. Some valid FEM names
>                     for example: "microsemi-lx5586", "sky85703-11"
>                     and "sky85803" etc.
> +- xo-cal-data: xo cal offset to be configured in xo trim register.

https://media.readthedocs.org/pdf/devicetree-specification/stable/devicetree-specification.pdf

"2.2.4 Properties
...
Nonstandard property names should specify a unique string prefix, such
as a stock ticker symbol, identifying the name of the company or
organization that defined the property. Examples:
  fsl,channel-fifo-len ibm,ppc-interrupt-server#s
  linux,network-index"

In other words, like most other custom properties in this file, you
should probably have a "qcom," prefix.

Brian

>  Example (to supply PCI based wifi block details):

[PATCH v2 2/2] ath10k: Add xo calibration support for wifi rf clock

[Govind Singh]

PMIC XO is the clock source for wifi rf clock in integrated wifi
chipset ex: WCN3990. Due to board layout errors XO frequency drifts
can cause wifi rf clock inaccuracy.
XO calibration test tree in Factory Test Mode is used to find the
best frequency offset(for example +/-2KHz )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 Wifi).
Calibrated XO offset is sent to fw, which compensate the clock drift
by programing the XO trim register.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/qmi.c  | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/snoc.c | 11 +++++++++++
 drivers/net/wireless/ath/ath10k/snoc.h |  2 ++
 3 files changed, 25 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index 37b3bd629f48..99c140a6628d 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -302,10 +302,16 @@ static int ath10k_qmi_send_cal_report_req(struct ath10k_qmi *qmi)
 	struct wlfw_cal_report_resp_msg_v01 resp = {};
 	struct wlfw_cal_report_req_msg_v01 req = {};
 	struct ath10k *ar = qmi->ar;
+	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
 	struct qmi_txn txn;
 	int i, j = 0;
 	int ret;
 
+	if (ar_snoc->xo_cal_supported) {
+		req.xo_cal_data_valid = 1;
+		req.xo_cal_data = ar_snoc->xo_cal_data;
+	}
+
 	ret = qmi_txn_init(&qmi->qmi_hdl, &txn, wlfw_cal_report_resp_msg_v01_ei,
 			   &resp);
 	if (ret < 0)
@@ -636,6 +642,7 @@ ath10k_qmi_ind_register_send_sync_msg(struct ath10k_qmi *qmi)
 	struct wlfw_ind_register_resp_msg_v01 resp = {};
 	struct wlfw_ind_register_req_msg_v01 req = {};
 	struct ath10k *ar = qmi->ar;
+	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
 	struct qmi_txn txn;
 	int ret;
 
@@ -646,6 +653,11 @@ ath10k_qmi_ind_register_send_sync_msg(struct ath10k_qmi *qmi)
 	req.msa_ready_enable_valid = 1;
 	req.msa_ready_enable = 1;
 
+	if (ar_snoc->xo_cal_supported) {
+		req.xo_cal_enable_valid = 1;
+		req.xo_cal_enable = 1;
+	}
+
 	ret = qmi_txn_init(&qmi->qmi_hdl, &txn,
 			   wlfw_ind_register_resp_msg_v01_ei, &resp);
 	if (ret < 0)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 54efe6be8f1d..7cfd17beb7b0 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 
 #include "ce.h"
@@ -1189,6 +1190,16 @@ static int ath10k_snoc_resource_init(struct ath10k *ar)
 		ar_snoc->ce_irqs[i].irq_line = res->start;
 	}
 
+	ret = device_property_read_u32(&pdev->dev, "xo-cal-data",
+				       &ar_snoc->xo_cal_data);
+	ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc xo-cal-data return %d\n", ret);
+	if (ret == 0) {
+		ar_snoc->xo_cal_supported = true;
+		ath10k_dbg(ar, ATH10K_DBG_SNOC, "xo cal data %x\n",
+			   ar_snoc->xo_cal_data);
+	}
+	ret = 0;
+
 out:
 	return ret;
 }
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index 2b2f23cf7c5d..25383de8f17d 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -91,6 +91,8 @@ struct ath10k_snoc {
 	struct ath10k_clk_info *clk;
 	struct ath10k_qmi *qmi;
 	unsigned long int flags;
+	bool xo_cal_supported;
+	u32 xo_cal_data;
 };
 
 static inline struct ath10k_snoc *ath10k_snoc_priv(struct ath10k *ar)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH v2 2/2] ath10k: Add xo calibration support for wifi rf clock

[Govind Singh]

PMIC XO is the clock source for wifi rf clock in integrated wifi
chipset ex: WCN3990. Due to board layout errors XO frequency drifts
can cause wifi rf clock inaccuracy.
XO calibration test tree in Factory Test Mode is used to find the
best frequency offset(for example +/-2KHz )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 Wifi).
Calibrated XO offset is sent to fw, which compensate the clock drift
by programing the XO trim register.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/qmi.c  | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/snoc.c | 11 +++++++++++
 drivers/net/wireless/ath/ath10k/snoc.h |  2 ++
 3 files changed, 25 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index 37b3bd629f48..99c140a6628d 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -302,10 +302,16 @@ static int ath10k_qmi_send_cal_report_req(struct ath10k_qmi *qmi)
 	struct wlfw_cal_report_resp_msg_v01 resp = {};
 	struct wlfw_cal_report_req_msg_v01 req = {};
 	struct ath10k *ar = qmi->ar;
+	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
 	struct qmi_txn txn;
 	int i, j = 0;
 	int ret;
 
+	if (ar_snoc->xo_cal_supported) {
+		req.xo_cal_data_valid = 1;
+		req.xo_cal_data = ar_snoc->xo_cal_data;
+	}
+
 	ret = qmi_txn_init(&qmi->qmi_hdl, &txn, wlfw_cal_report_resp_msg_v01_ei,
 			   &resp);
 	if (ret < 0)
@@ -636,6 +642,7 @@ ath10k_qmi_ind_register_send_sync_msg(struct ath10k_qmi *qmi)
 	struct wlfw_ind_register_resp_msg_v01 resp = {};
 	struct wlfw_ind_register_req_msg_v01 req = {};
 	struct ath10k *ar = qmi->ar;
+	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
 	struct qmi_txn txn;
 	int ret;
 
@@ -646,6 +653,11 @@ ath10k_qmi_ind_register_send_sync_msg(struct ath10k_qmi *qmi)
 	req.msa_ready_enable_valid = 1;
 	req.msa_ready_enable = 1;
 
+	if (ar_snoc->xo_cal_supported) {
+		req.xo_cal_enable_valid = 1;
+		req.xo_cal_enable = 1;
+	}
+
 	ret = qmi_txn_init(&qmi->qmi_hdl, &txn,
 			   wlfw_ind_register_resp_msg_v01_ei, &resp);
 	if (ret < 0)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 54efe6be8f1d..7cfd17beb7b0 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 
 #include "ce.h"
@@ -1189,6 +1190,16 @@ static int ath10k_snoc_resource_init(struct ath10k *ar)
 		ar_snoc->ce_irqs[i].irq_line = res->start;
 	}
 
+	ret = device_property_read_u32(&pdev->dev, "xo-cal-data",
+				       &ar_snoc->xo_cal_data);
+	ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc xo-cal-data return %d\n", ret);
+	if (ret == 0) {
+		ar_snoc->xo_cal_supported = true;
+		ath10k_dbg(ar, ATH10K_DBG_SNOC, "xo cal data %x\n",
+			   ar_snoc->xo_cal_data);
+	}
+	ret = 0;
+
 out:
 	return ret;
 }
diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h
index 2b2f23cf7c5d..25383de8f17d 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.h
+++ b/drivers/net/wireless/ath/ath10k/snoc.h
@@ -91,6 +91,8 @@ struct ath10k_snoc {
 	struct ath10k_clk_info *clk;
 	struct ath10k_qmi *qmi;
 	unsigned long int flags;
+	bool xo_cal_supported;
+	u32 xo_cal_data;
 };
 
 static inline struct ath10k_snoc *ath10k_snoc_priv(struct ath10k *ar)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 0/2] Add xo calibration support for wifi rf clock

[Sven Eckelmann]

[-- Attachment #1.1: Type: text/plain, Size: 2658 bytes --]

On Wednesday, 20 March 2019 05:45:09 CET Govind Singh wrote:
> PMIC XO is the clock source for wifi rf clock in integrated wifi
> chipset ex: WCN3990. Due to board layout errors XO frequency drifts
> can cause wifi rf clock inaccuracy.
> XO calibration test tree in Factory Test Mode is used to find the
> best frequency offset(for example +/-2KHz )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 Wifi).
> Calibrated XO offset is sent to fw, which compensate the clock drift
> by programing the XO trim register.

Who is responsible to fill in this values in the device-tree? On other 
products, the correct XTAL capacitor registers values are calibrated on 
different devices (in the same product line) separately to ensure that each 
device has a minimal inaccuracy. During the boot of the device, the two u8 
taken from params_for_tuning_caps (inside the EEPROM) are just written to the 
AR_CH0_XTAL register (mapped to the correct the INDAC and OUTDAC region).

Your patch here seems to be doing something similar (you may correct me if I 
misinterpret something) but you are already saying that these devices don't 
have an EEPROM. This is already quite odd because then we also wouldn't have 
temperature compensation (also stored in per device EEPROM/precal data for 
other devices).

So you move it to the device tree. By default, this device tree is most likely 
a static thing which is shipped with the rest of the firmware. So no per 
device data is stored in this DTB on the flash. To include device specific 
information (mac addresses, calibration data, ...), you could also have the 
bootloader (u-boot for example) change the device tree during the boot process 
and let it inject the device specific XO trim register data.

How is this planned to work? Is the bootloader expected to modify the device 
tree during the boot to provide the device specific xo-cal-data. If yes, where 
is it getting the information from? And is there already support for QDART for 
it?

If you do this, why aren't you using the data from qcom,ath10k-pre-
calibration-data. At least for other ath10k devices, it includes the 
previously mentioned tuning caps. It is the first time I heard about an XO 
trim register and thus it might be something different than what I expect.


Last question: why is it an u32 when the message with xo_cal_data can only 
transport an u8?

Kind regards,
	Sven

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 146 bytes --]

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 0/2] Add xo calibration support for wifi rf clock

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2658 bytes --]

On Wednesday, 20 March 2019 05:45:09 CET Govind Singh wrote:
> PMIC XO is the clock source for wifi rf clock in integrated wifi
> chipset ex: WCN3990. Due to board layout errors XO frequency drifts
> can cause wifi rf clock inaccuracy.
> XO calibration test tree in Factory Test Mode is used to find the
> best frequency offset(for example +/-2KHz )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 Wifi).
> Calibrated XO offset is sent to fw, which compensate the clock drift
> by programing the XO trim register.

Who is responsible to fill in this values in the device-tree? On other 
products, the correct XTAL capacitor registers values are calibrated on 
different devices (in the same product line) separately to ensure that each 
device has a minimal inaccuracy. During the boot of the device, the two u8 
taken from params_for_tuning_caps (inside the EEPROM) are just written to the 
AR_CH0_XTAL register (mapped to the correct the INDAC and OUTDAC region).

Your patch here seems to be doing something similar (you may correct me if I 
misinterpret something) but you are already saying that these devices don't 
have an EEPROM. This is already quite odd because then we also wouldn't have 
temperature compensation (also stored in per device EEPROM/precal data for 
other devices).

So you move it to the device tree. By default, this device tree is most likely 
a static thing which is shipped with the rest of the firmware. So no per 
device data is stored in this DTB on the flash. To include device specific 
information (mac addresses, calibration data, ...), you could also have the 
bootloader (u-boot for example) change the device tree during the boot process 
and let it inject the device specific XO trim register data.

How is this planned to work? Is the bootloader expected to modify the device 
tree during the boot to provide the device specific xo-cal-data. If yes, where 
is it getting the information from? And is there already support for QDART for 
it?

If you do this, why aren't you using the data from qcom,ath10k-pre-
calibration-data. At least for other ath10k devices, it includes the 
previously mentioned tuning caps. It is the first time I heard about an XO 
trim register and thus it might be something different than what I expect.


Last question: why is it an u32 when the message with xo_cal_data can only 
transport an u8?

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] Add xo calibration support for wifi rf clock

[Govind Singh]

Hi Sven,

On 2019-03-20 14:07, Sven Eckelmann wrote:
> On Wednesday, 20 March 2019 05:45:09 CET Govind Singh wrote:
>> PMIC XO is the clock source for wifi rf clock in integrated wifi
>> chipset ex: WCN3990. Due to board layout errors XO frequency drifts
>> can cause wifi rf clock inaccuracy.
>> XO calibration test tree in Factory Test Mode is used to find the
>> best frequency offset(for example +/-2KHz )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 
>> Wifi).
>> Calibrated XO offset is sent to fw, which compensate the clock drift
>> by programing the XO trim register.
> 
> Who is responsible to fill in this values in the device-tree?

This is populated via boot-loader/system fw(for chrome-OS its coreboot).
Post calibration QDART writes to non-volatile/persist region and
during boot up boot loader fills this value in dt node as there is
no otp region or EPROM available.

> On other
> products, the correct XTAL capacitor registers values are calibrated on
> different devices (in the same product line) separately to ensure that 
> each
> device has a minimal inaccuracy. During the boot of the device, the two 
> u8
> taken from params_for_tuning_caps (inside the EEPROM) are just written 
> to the
> AR_CH0_XTAL register (mapped to the correct the INDAC and OUTDAC 
> region).
> 
> Your patch here seems to be doing something similar (you may correct me 
> if I
> misinterpret something) but you are already saying that these devices 
> don't
> have an EEPROM. This is already quite odd because then we also wouldn't 
> have
> temperature compensation (also stored in per device EEPROM/precal data 
> for
> other devices).
> 
> So you move it to the device tree. By default, this device tree is most 
> likely
> a static thing which is shipped with the rest of the firmware. So no 
> per
> device data is stored in this DTB on the flash. To include device 
> specific
> information (mac addresses, calibration data, ...), you could also have 
> the
> bootloader (u-boot for example) change the device tree during the boot 
> process
> and let it inject the device specific XO trim register data.
> 
> How is this planned to work? Is the bootloader expected to modify the 
> device
> tree during the boot to provide the device specific xo-cal-data. If 
> yes, where
> is it getting the information from? And is there already support for 
> QDART for
> it?
> 
Per device data will be stored in NOR flash by QDART and dt entry will 
be populated during boot by bootloader.
Here we are trying to set the xo trim register of PMIC xtal, which is 
the base clk source of wifi rf clk.

> If you do this, why aren't you using the data from qcom,ath10k-pre-
> calibration-data. At least for other ath10k devices, it includes the
> previously mentioned tuning caps. It is the first time I heard about an 
> XO
> trim register and thus it might be something different than what I 
> expect.
> 
No, Integrated chip set ex:WCN3990 does not use 
ath10k-pre-calibration-data.

> 
> Last question: why is it an u32 when the message with xo_cal_data can 
> only
> transport an u8?
> 

Yes, this i will fix in next version.

> Kind regards,
> 	Sven

BR,
Govind

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 0/2] Add xo calibration support for wifi rf clock

[Govind Singh]

Hi Sven,

On 2019-03-20 14:07, Sven Eckelmann wrote:
> On Wednesday, 20 March 2019 05:45:09 CET Govind Singh wrote:
>> PMIC XO is the clock source for wifi rf clock in integrated wifi
>> chipset ex: WCN3990. Due to board layout errors XO frequency drifts
>> can cause wifi rf clock inaccuracy.
>> XO calibration test tree in Factory Test Mode is used to find the
>> best frequency offset(for example +/-2KHz )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 
>> Wifi).
>> Calibrated XO offset is sent to fw, which compensate the clock drift
>> by programing the XO trim register.
> 
> Who is responsible to fill in this values in the device-tree?

This is populated via boot-loader/system fw(for chrome-OS its coreboot).
Post calibration QDART writes to non-volatile/persist region and
during boot up boot loader fills this value in dt node as there is
no otp region or EPROM available.

> On other
> products, the correct XTAL capacitor registers values are calibrated on
> different devices (in the same product line) separately to ensure that 
> each
> device has a minimal inaccuracy. During the boot of the device, the two 
> u8
> taken from params_for_tuning_caps (inside the EEPROM) are just written 
> to the
> AR_CH0_XTAL register (mapped to the correct the INDAC and OUTDAC 
> region).
> 
> Your patch here seems to be doing something similar (you may correct me 
> if I
> misinterpret something) but you are already saying that these devices 
> don't
> have an EEPROM. This is already quite odd because then we also wouldn't 
> have
> temperature compensation (also stored in per device EEPROM/precal data 
> for
> other devices).
> 
> So you move it to the device tree. By default, this device tree is most 
> likely
> a static thing which is shipped with the rest of the firmware. So no 
> per
> device data is stored in this DTB on the flash. To include device 
> specific
> information (mac addresses, calibration data, ...), you could also have 
> the
> bootloader (u-boot for example) change the device tree during the boot 
> process
> and let it inject the device specific XO trim register data.
> 
> How is this planned to work? Is the bootloader expected to modify the 
> device
> tree during the boot to provide the device specific xo-cal-data. If 
> yes, where
> is it getting the information from? And is there already support for 
> QDART for
> it?
> 
Per device data will be stored in NOR flash by QDART and dt entry will 
be populated during boot by bootloader.
Here we are trying to set the xo trim register of PMIC xtal, which is 
the base clk source of wifi rf clk.

> If you do this, why aren't you using the data from qcom,ath10k-pre-
> calibration-data. At least for other ath10k devices, it includes the
> previously mentioned tuning caps. It is the first time I heard about an 
> XO
> trim register and thus it might be something different than what I 
> expect.
> 
No, Integrated chip set ex:WCN3990 does not use 
ath10k-pre-calibration-data.

> 
> Last question: why is it an u32 when the message with xo_cal_data can 
> only
> transport an u8?
> 

Yes, this i will fix in next version.

> Kind regards,
> 	Sven

BR,
Govind

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

[Youghandhar Chintala]

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

Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>

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


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

[PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Youghandhar Chintala]

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

Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
---
 .../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..205ee949daba 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 cal offset to be configured in XO trim register
+
   qcom,ath11k-calibration-variant:
     $ref: /schemas/types.yaml#/definitions/string
     description:
-- 
2.38.0


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

[PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Youghandhar Chintala]

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

Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
---
 .../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..205ee949daba 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 cal offset to be configured in XO trim register
+
   qcom,ath11k-calibration-variant:
     $ref: /schemas/types.yaml#/definitions/string
     description:
-- 
2.38.0

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Krzysztof Kozlowski]

On 31/01/2023 15:03, Youghandhar Chintala wrote:
> Add dt binding to get XO calibration data support for Wi-Fi RF clock.

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).
Hint: dt-bindings: net: qcom,ath11k:

> 
> Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
> ---
>  .../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..205ee949daba 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 cal offset to be configured in XO trim register

Missing type. I also do not understand what's this and why some register
offset should be stored in DT. Please give us some justification why
this is suitable for DT.

Best regards,
Krzysztof


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Krzysztof Kozlowski]

On 31/01/2023 15:03, Youghandhar Chintala wrote:
> Add dt binding to get XO calibration data support for Wi-Fi RF clock.

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).
Hint: dt-bindings: net: qcom,ath11k:

> 
> Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
> ---
>  .../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..205ee949daba 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 cal offset to be configured in XO trim register

Missing type. I also do not understand what's this and why some register
offset should be stored in DT. Please give us some justification why
this is suitable for DT.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Rob Herring]

On Tue, Jan 31, 2023 at 07:02:16PM +0100, Krzysztof Kozlowski wrote:
> On 31/01/2023 15:03, Youghandhar Chintala wrote:
> > Add dt binding to get XO calibration data support for Wi-Fi RF clock.
> 
> 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).
> Hint: dt-bindings: net: qcom,ath11k:
> 
> > 
> > Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
> > ---
> >  .../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..205ee949daba 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 cal offset to be configured in XO trim register
> 
> Missing type. I also do not understand what's this and why some register
> offset should be stored in DT. Please give us some justification why
> this is suitable for DT.

I think that's a voltage offset or something, but you are right, we 
shouldn't have to guess.

It needs a vendor prefix, too.

Rob

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Rob Herring]

On Tue, Jan 31, 2023 at 07:02:16PM +0100, Krzysztof Kozlowski wrote:
> On 31/01/2023 15:03, Youghandhar Chintala wrote:
> > Add dt binding to get XO calibration data support for Wi-Fi RF clock.
> 
> 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).
> Hint: dt-bindings: net: qcom,ath11k:
> 
> > 
> > Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
> > ---
> >  .../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..205ee949daba 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 cal offset to be configured in XO trim register
> 
> Missing type. I also do not understand what's this and why some register
> offset should be stored in DT. Please give us some justification why
> this is suitable for DT.

I think that's a voltage offset or something, but you are right, we 
shouldn't have to guess.

It needs a vendor prefix, too.

Rob

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Youghandhar Chintala (Temp)]

On 1/31/2023 11:32 PM, Krzysztof Kozlowski wrote:
> On 31/01/2023 15:03, Youghandhar Chintala wrote:
>> Add dt binding to get XO calibration data support for Wi-Fi RF clock.
> 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).
> Hint: dt-bindings: net: qcom,ath11k:
>
>> Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
>> ---
>>   .../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..205ee949daba 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 cal offset to be configured in XO trim register
> Missing type. I also do not understand what's this and why some register
> offset should be stored in DT. Please give us some justification why
> this is suitable for DT.
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,

I will address you comments in next version of patch.

Regards,

Youghandhar



-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH v2 1/2] dt: bindings: add dt entry for XO calibration support

[Youghandhar Chintala (Temp)]

On 1/31/2023 11:32 PM, Krzysztof Kozlowski wrote:
> On 31/01/2023 15:03, Youghandhar Chintala wrote:
>> Add dt binding to get XO calibration data support for Wi-Fi RF clock.
> 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).
> Hint: dt-bindings: net: qcom,ath11k:
>
>> Signed-off-by: Youghandhar Chintala <quic_youghand@quicinc.com>
>> ---
>>   .../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..205ee949daba 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 cal offset to be configured in XO trim register
> Missing type. I also do not understand what's this and why some register
> offset should be stored in DT. Please give us some justification why
> this is suitable for DT.
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,

I will address you comments in next version of patch.

Regards,

Youghandhar