Re: [PATCH v7 10/24] wfx: add fwio.c/fwio.h

On Thursday 7 October 2021 10:08:53 CEST Kalle Valo wrote:

Jérôme Pouiller [off-list ref] writes:

quoted

On Friday 1 October 2021 13:58:38 CEST Kalle Valo wrote:

quoted

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

+static int get_firmware(struct wfx_dev *wdev, u32 keyset_chip,
+                     const struct firmware **fw, int *file_offset)
+{
+     int keyset_file;
+     char filename[256];
+     const char *data;
+     int ret;
+
+     snprintf(filename, sizeof(filename), "%s_%02X.sec",
+              wdev->pdata.file_fw, keyset_chip);
+     ret = firmware_request_nowarn(fw, filename, wdev->dev);
+     if (ret) {
+             dev_info(wdev->dev, "can't load %s, falling back to %s.sec\n",
+                      filename, wdev->pdata.file_fw);
+             snprintf(filename, sizeof(filename), "%s.sec",
+                      wdev->pdata.file_fw);
+             ret = request_firmware(fw, filename, wdev->dev);
+             if (ret) {
+                     dev_err(wdev->dev, "can't load %s\n", filename);
+                     *fw = NULL;
+                     return ret;
+             }
+     }

How is this firmware file loading supposed to work? If I'm reading the
code right, the driver tries to load file "wfm_wf200_??.sec" but in
linux-firmware the file is silabs/wfm_wf200_C0.sec:

https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/silabs

That can't work automatically, unless I'm missing something of course.

The firmware are signed. "C0" is the key used to sign this firmware. This
key must match with the key burned into the chip. Fortunately, the driver
is able to read the key accepted by the chip and automatically choose the
right firmware.

We could imagine to add a attribute in the DT to choose the firmware to
load. However, it would be a pity to have to specify it manually whereas
the driver is able to detect it automatically.

Currently, the only possible key is C0. However, it exists some internal
parts with other keys. In addition, it is theoretically possible to ask
to Silabs to burn parts with a specific key in order to improve security
of a product.

Obviously, for now, this feature mainly exists for the Silabs firmware
developers who have to work with other keys.

My point above was about the directory "silabs". If I read the code
correctly, wfx driver tries to load "foo.bin" but in the linux-firmware
file is "silabs/foo.bin". So the should also include directory name in
the request and use "silabs/foo.bin".

Oh! Absolutely. I had never noticed my firmware was not in silabs/ on my
test setup.

[...]
-- 
Jérôme Pouiller