On Friday 1 October 2021 11:22:08 CEST Kalle Valo wrote:

Jerome Pouiller [off-list ref] writes:

quoted

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>

[...]

quoted

+/* The device needs data about the antenna configuration. This information in
+ * provided by PDS (Platform Data Set, this is the wording used in WF200
+ * documentation) files. For hardware integrators, the full process to create
+ * PDS files is described here:
+ *   https:github.com/SiliconLabs/wfx-firmware/blob/master/PDS/README.md
+ *
+ * So this function aims to send PDS to the device. However, the PDS file is
+ * often bigger than Rx buffers of the chip, so it has to be sent in multiple
+ * parts.
+ *
+ * In add, the PDS data cannot be split anywhere. The PDS files contains tree
+ * structures. Braces are used to enter/leave a level of the tree (in a JSON
+ * fashion). PDS files can only been split between root nodes.
+ */
+int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
+{
+     int ret;
+     int start, brace_level, i;
+
+     start = 0;
+     brace_level = 0;
+     if (buf[0] != '{') {
+ dev_err(wdev->dev, "valid PDS start with '{'. Did you forget to
compress it?\n");
+             return -EINVAL;
+     }
+     for (i = 1; i < len - 1; i++) {
+             if (buf[i] == '{')
+                     brace_level++;
+             if (buf[i] == '}')
+                     brace_level--;
+             if (buf[i] == '}' && !brace_level) {
+                     i++;
+                     if (i - start + 1 > WFX_PDS_MAX_SIZE)
+                             return -EFBIG;
+                     buf[start] = '{';
+                     buf[i] = 0;
+                     dev_dbg(wdev->dev, "send PDS '%s}'\n", buf + start);
+                     buf[i] = '}';
+                     ret = hif_configuration(wdev, buf + start,
+                                             i - start + 1);
+                     if (ret > 0) {
+ dev_err(wdev->dev, "PDS bytes %d to %d: invalid data (unsupported
options?)\n",
+                                     start, i);
+                             return -EINVAL;
+                     }
+                     if (ret == -ETIMEDOUT) {
+ dev_err(wdev->dev, "PDS bytes %d to %d: chip didn't reply (corrupted
file?)\n",
+                                     start, i);
+                             return ret;
+                     }
+                     if (ret) {
+ dev_err(wdev->dev, "PDS bytes %d to %d: chip returned an unknown
error\n",
+                                     start, i);
+                             return -EIO;
+                     }
+                     buf[i] = ',';
+                     start = i;
+             }
+     }
+     return 0;
+}

I'm not really fond of having this kind of ASCII based parser in the
kernel. Do you have an example compressed file somewhere?

An example of uncompressed configuration file can be found here[1]. Once
compressed with [2], you get:

    {a:{a:4,b:1},b:{a:{a:4,b:0,c:0,d:0,e:A},b:{a:4,b:0,c:0,d:0,e:B},c:{a:4,b:0,c:0,d:0,e:C},d:{a:4,b:0,c:0,d:0,e:D},e:{a:4,b:0,c:0,d:0,e:E},f:{a:4,b:0,c:0,d:0,e:F},g:{a:4,b:0,c:0,d:0,e:G},h:{a:4,b:0,c:0,d:0,e:H},i:{a:4,b:0,c:0,d:0,e:I},j:{a:4,b:0,c:0,d:0,e:J},k:{a:4,b:0,c:0,d:0,e:K},l:{a:4,b:0,c:0,d:1,e:L},m:{a:4,b:0,c:0,d:1,e:M}},c:{a:{a:4},b:{a:6},c:{a:6,c:0},d:{a:6},e:{a:6},f:{a:6}},e:{b:0,c:1},h:{e:0,a:50,b:0,d:0,c:[{a:1,b:[0,0,0,0,0,0]},{a:2,b:[0,0,0,0,0,0]},{a:[3,9],b:[0,0,0,0,0,0]},{a:A,b:[0,0,0,0,0,0]},{a:B,b:[0,0,0,0,0,0]},{a:[C,D],b:[0,0,0,0,0,0]},{a:E,b:[0,0,0,0,0,0]}]},j:{a:0,b:0}}


[1]: https://github.com/SiliconLabs/wfx-pds/blob/API4.1/BRD4001A_Rev_A01.pds.in
[2]: https://github.com/SiliconLabs/wfx-linux-tools/blob/SD4/pds_compress

Does the device still work without these PDS files? I ask because my
suggestion is to remove this part altogether and revisit after the
initial driver is moved to drivers/net/wireless. A lot simpler to review
complex features separately.

I think we will be able to communicate with the chip. However, the chip won't
have any antenna parameters. Thus, I think the RF won't work properly.


-- 
Jérôme Pouiller