Hello Rob! Could you look at issue below to represent antenna (pds)
firmware file requirement for this driver in DTS file?

On Thursday 07 October 2021 11:16:29 Kalle Valo wrote:

Pali Rohár [off-list ref] writes:

quoted

On Friday 01 October 2021 17:09:41 Jérôme Pouiller wrote:

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.
 

quoted

Also I would prefer to use directory name as the driver name wfx, but I
guess silabs is also doable.

I have no opinion.

quoted

Also I'm not seeing the PDS files in linux-firmware. The idea is that
when user installs an upstream kernel and the linux-firmware everything
will work automatically, without any manual file installations.

WF200 is just a chip. Someone has to design an antenna before to be able
to use.

However, we have evaluation boards that have antennas and corresponding
PDS files[1]. Maybe linux-firmware should include the PDS for these boards

So chip vendor provides firmware and card vendor provides PDS files. In
my opinion all files should go into linux-firmware repository. If Silabs
has PDS files for its devel boards (which are basically cards) then I
think these files should go also into linux-firmware repository.

I agree, all files required for normal functionality should be in
linux-firmware. The upstream philosophy is that a user can just install
the latest kernel and latest linux-firmware, and everything should work
out of box (without any manual work).

quoted

And based on some parameter, driver should load correct PDS file. Seems
like DT can be a place where to put something which indicates which PDS
file should be used.

Again I agree.

quoted

But should be in DT directly name of PDS file? Or should be in DT just
additional compatible string with card vendor name and then in driver
itself should be mapping table from compatible string to filename? I do
not know what is better.

This is also what I was wondering, to me it sounds wrong to have a
filename in DT. I was more thinking about calling it "antenna name" (and
not the actually filename), but using compatible strings sounds good to
me as well. But of course DT maintainers know this better.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches