Skip to contentSkip to conversation

wifi: ath10k: Drop of_get_property() call

PATCH In review Rob Herring (Arm) 3 participants 4 messages Mar 14, 2025 – Jun 6, 2025 Qualcomm Atheros ATH10K wireless driver

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 wifi: ath10k: Drop of_get_property() call

There's no need to check the property presence and length before calling of_property_read_u8_array() as it will return an error if the property is missing or the length is too small. The return errno doesn't matter to the caller, so no change in behavior there.

Change of_property_read_u8_array() to of_property_read_variable_u8_array() as the former allows properties to be longer than the requested length. Now the property has to be the exact length requested as the removed check required.

This part of a larger effort to remove DT functions like of_get_property() and of_find_property() which return raw DT data having no reference counting.

---
v2:
- Add check that cal_data_len is non-zero

---
 drivers/net/wireless/ath/ath10k/core.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)
drivers/net/wireless/ath/ath10k/core.c +7 -17
index b3294287bce1..47206b171aa5 100644
@@ -1889,39 +1889,29 @@ static int ath10k_download_cal_file(struct ath10k *ar,
 static int ath10k_download_cal_dt(struct ath10k *ar, const char *dt_name)
 {
 	struct device_node *node;
-	int data_len;
+	int data_len = ar->hw_params.cal_data_len;
 	void *data;
 	int ret;
 
 	node = ar->dev->of_node;
-	if (!node)
+	if (!node || !data_len)
 		/* Device Tree is optional, don't print any warnings if
 		 * there's no node for ath10k.
 		 */
 		return -ENOENT;
 
-	if (!of_get_property(node, dt_name, &data_len)) {
-		/* The calibration data node is optional */
-		return -ENOENT;
-	}
-
-	if (data_len != ar->hw_params.cal_data_len) {
-		ath10k_warn(ar, "invalid calibration data length in DT: %d\n",
-			    data_len);
-		ret = -EMSGSIZE;
-		goto out;
-	}
-
 	data = kmalloc(data_len, GFP_KERNEL);
 	if (!data) {
 		ret = -ENOMEM;
 		goto out;
 	}
 
-	ret = of_property_read_u8_array(node, dt_name, data, data_len);
+	ret = of_property_read_variable_u8_array(node, dt_name, data, data_len, data_len);
 	if (ret) {
-		ath10k_warn(ar, "failed to read calibration data from DT: %d\n",
-			    ret);
+		/* Don't warn if optional property not found  */
+		if (ret != -EINVAL)
+			ath10k_warn(ar, "failed to read calibration data from DT: %d\n",
+				    ret);
 		goto out_free;
 	}
 
Signature · 1 line
2.47.2
#1 lore raw

Hi,

Quoted 55 lines · Rob Herring (Arm)

There's no need to check the property presence and length before calling of_property_read_u8_array() as it will return an error if the property is missing or the length is too small. The return errno doesn't matter to the caller, so no change in behavior there.

Change of_property_read_u8_array() to of_property_read_variable_u8_array() as the former allows properties to be longer than the requested length. Now the property has to be the exact length requested as the removed check required.

This part of a larger effort to remove DT functions like of_get_property() and of_find_property() which return raw DT data having no reference counting.

---
v2:
- Add check that cal_data_len is non-zero

---
 drivers/net/wireless/ath/ath10k/core.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)
drivers/net/wireless/ath/ath10k/core.c +2 -13
index b3294287bce1..47206b171aa5 100644
@@ -1889,39 +1889,29 @@ static int ath10k_download_cal_file(struct ath10k *ar,
 static int ath10k_download_cal_dt(struct ath10k *ar, const char *dt_name)
 {
        struct device_node *node;
-       int data_len;
+       int data_len = ar->hw_params.cal_data_len;
        void *data;
        int ret;
 
        node = ar->dev->of_node;
-       if (!node)
+       if (!node || !data_len)
                /* Device Tree is optional, don't print any warnings if
                 * there's no node for ath10k.
                 */
                return -ENOENT;
 
-       if (!of_get_property(node, dt_name, &data_len)) {
-               /* The calibration data node is optional */
-               return -ENOENT;
-       }
-
-       if (data_len != ar->hw_params.cal_data_len) {
-               ath10k_warn(ar, "invalid calibration data length in DT: %d\n",
-                           data_len);
-               ret = -EMSGSIZE;
-               goto out;
-       }

drivers/net/wireless/ath/ath10k/core.c static int ath10k_download_cal_file(struct ath10k *ar, · line 1910

Quoted 10 lines
drivers/net/wireless/ath/ath10k/core.c +1 -2
-
        data = kmalloc(data_len, GFP_KERNEL);
        if (!data) {
                ret = -ENOMEM;
                goto out;
        }
 
-       ret = of_property_read_u8_array(node, dt_name, data, data_len);
+       ret = of_property_read_variable_u8_array(node, dt_name, data, data_len, data_len);
        if (ret) {

of_property_read_u8_array() returns 0 on success, but of_property_read_variable_u8_array() returns the number of elements read on success, so this check needs to be ret < 0 now.

drivers/net/wireless/ath/ath10k/core.c static int ath10k_download_cal_file(struct ath10k *ar, · line 1914

Quoted 6 lines
drivers/net/wireless/ath/ath10k/core.c +4 -2
-               ath10k_warn(ar, "failed to read calibration data from DT: %d\n",
-                           ret);
+               /* Don't warn if optional property not found  */
+               if (ret != -EINVAL)
+                       ath10k_warn(ar, "failed to read calibration data from DT: %d\n",
+                                   ret);

Best regards,
Jonas

#2 lore raw
Quoted 66 lines · Jonas Gorski

Hi,

Quoted 62 lines · Rob Herring (Arm)

There's no need to check the property presence and length before calling of_property_read_u8_array() as it will return an error if the property is missing or the length is too small. The return errno doesn't matter to the caller, so no change in behavior there.

Change of_property_read_u8_array() to of_property_read_variable_u8_array() as the former allows properties to be longer than the requested length. Now the property has to be the exact length requested as the removed check required.

This part of a larger effort to remove DT functions like of_get_property() and of_find_property() which return raw DT data having no reference counting.

---
v2:
- Add check that cal_data_len is non-zero

---
 drivers/net/wireless/ath/ath10k/core.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)
drivers/net/wireless/ath/ath10k/core.c +2 -14
index b3294287bce1..47206b171aa5 100644
@@ -1889,39 +1889,29 @@ static int ath10k_download_cal_file(struct ath10k *ar,
 static int ath10k_download_cal_dt(struct ath10k *ar, const char *dt_name)
 {
        struct device_node *node;
-       int data_len;
+       int data_len = ar->hw_params.cal_data_len;
        void *data;
        int ret;
 
        node = ar->dev->of_node;
-       if (!node)
+       if (!node || !data_len)
                /* Device Tree is optional, don't print any warnings if
                 * there's no node for ath10k.
                 */
                return -ENOENT;
 
-       if (!of_get_property(node, dt_name, &data_len)) {
-               /* The calibration data node is optional */
-               return -ENOENT;
-       }
-
-       if (data_len != ar->hw_params.cal_data_len) {
-               ath10k_warn(ar, "invalid calibration data length in DT: %d\n",
-                           data_len);
-               ret = -EMSGSIZE;
-               goto out;
-       }
-
        data = kmalloc(data_len, GFP_KERNEL);
        if (!data) {
                ret = -ENOMEM;
                goto out;
        }
 
Quoted 7 lines

drivers/net/wireless/ath/ath10k/core.c static int ath10k_download_cal_file(struct ath10k *ar, · line 1910

Quoted 3 lines
drivers/net/wireless/ath/ath10k/core.c +1 -1
-       ret = of_property_read_u8_array(node, dt_name, data, data_len);
+       ret = of_property_read_variable_u8_array(node, dt_name, data, data_len, data_len);
        if (ret) {

of_property_read_u8_array() returns 0 on success, but of_property_read_variable_u8_array() returns the number of elements read on success, so this check needs to be ret < 0 now.

Indeed. Thanks for catching that.

Rob

#3 lore raw
Quoted 6 lines · Rob Herring
Quoted 3 lines · Jonas Gorski

of_property_read_u8_array() returns 0 on success, but of_property_read_variable_u8_array() returns the number of elements read on success, so this check needs to be ret < 0 now.

Indeed. Thanks for catching that.

cleaning up patchwork. was there ever a v3?